Blacklist bad GPU drivers: currenly we disable all gpu related features if th...

chrome/renderer/render_thread.cc

Index: chrome/renderer/render_thread.cc
===================================================================
--- chrome/renderer/render_thread.cc	(revision 68066)
+++ chrome/renderer/render_thread.cc	(working copy)
@@ -29,6 +29,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/db_message_filter.h"
 #include "chrome/common/dom_storage_common.h"
+#include "chrome/common/gpu_feature_flags.h"
 #include "chrome/common/plugin_messages.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/render_messages_params.h"
@@ -1079,18 +1080,27 @@
 }
 
 void RenderThread::OnGpuChannelEstablished(
-    const IPC::ChannelHandle& channel_handle, const GPUInfo& gpu_info) {
+    const IPC::ChannelHandle& channel_handle,
+    const GPUInfo& gpu_info,
+    const GpuFeatureFlags& gpu_feature_flags) {
 #if defined(OS_POSIX)
   // If we received a ChannelHandle, register it now.
   if (channel_handle.socket.fd >= 0)
     IPC::AddChannelSocket(channel_handle.name, channel_handle.socket.fd);
 #endif
 
-  gpu_channel_->set_gpu_info(gpu_info);
-
   if (channel_handle.name.size() != 0) {
-    // Connect to the GPU process if a channel name was received.
-    gpu_channel_->Connect(channel_handle.name);
+    if (gpu_feature_flags.is_accelerated_2d_canvas_blacklisted() ||

[vangelis, 2010/12/08 00:17:38]

It would be worth putting a comment here mentioning that for now we don't
establish a gpu channel if any of the features is blacklisted.

[Zhenyao Mo, 2010/12/09 00:06:06]

Done.

+        gpu_feature_flags.is_accelerated_compositing_blacklisted() ||
+        gpu_feature_flags.is_webgl_blacklisted()) {

[Ken Russell (switch to Gerrit), 2010/12/08 05:04:16]

There's an architectural problem with doing these checks here, which is that the
renderer process is untrusted. A compromised renderer might cause these bits to
be set to false even though the browser process said that it isn't supposed to
be allowed to establish a channel to the GPU process.

I think this CL should be restructured to send the blacklist information from
the browser to the GPU process in GpuProcessHost::EstablishGpuChannel. The GPU
process should then determine whether to create a channel or not. If not, then
send back an invalid channel handle. GpuProcessHost::OnChannelEstablished and
RenderThread::OnGpuChannelEstablished then shouldn't need to be modified at all.

We won't be able to support blacklisting of individual features with this new
structure, at least not by preventing a GPU channel from being created for this
renderer process. There is one GPU channel per renderer, and when a new "OpenGL"
context is created for an API like WebGL or accelerated canvas 2D, the channel
is used to create a new command buffer. The creation of those command buffers is
eventually going to have to check these blacklist flags, but since we're
starting off by disabling GPU support if any of these flags are set, the safest
initial solution is what you've done here, to just prevent the GPU channel from
being established. It's only where we make the choice to not establish the
channel that's the concern.

+      // If any GPU feature is blacklisted, cancel the connection.
+      gpu_channel_ = NULL;
+    } else {
+      // Connect to the GPU process if a channel name was received.
+      gpu_channel_->set_gpu_info(gpu_info);
+      gpu_channel_->set_gpu_feature_flags(gpu_feature_flags);
+      gpu_channel_->Connect(channel_handle.name);
+    }
   } else {
     // Otherwise cancel the connection.
     gpu_channel_ = NULL;