ozone: drm: Fix HardwareDisplayController use-after-free

ui/ozone/platform/drm/gpu/screen_manager.cc

Index: ui/ozone/platform/drm/gpu/screen_manager.cc
diff --git a/ui/ozone/platform/drm/gpu/screen_manager.cc b/ui/ozone/platform/drm/gpu/screen_manager.cc
index b8b1fd3747952ce505d3de2dfeb99c54048bcc29..cf812d3f5edb17d9e50b33a0ec74af4a87332e46 100644
--- a/ui/ozone/platform/drm/gpu/screen_manager.cc
+++ b/ui/ozone/platform/drm/gpu/screen_manager.cc
@@ -98,10 +98,8 @@ void ScreenManager::RemoveDisplayController(const scoped_refptr<DrmDevice>& drm,
   if (it != controllers_.end()) {
     bool is_mirrored = (*it)->IsMirrored();
     (*it)->RemoveCrtc(drm, crtc);
-    if (!is_mirrored) {
-      UpdateControllerToWindowMapping();
-      controllers_.erase(it);

[alexst (slow to review), 2015/04/15 00:50:02]

I want to say there was a reason why this was removed after the mapping was
updated. The controller was disabled so didn't affect the window assignment, but
still a valid ptr for the duration of  UpdateControllerToWindowMapping();

Maybe Danny remembers.

[spang, 2015/04/15 02:12:11]

I don't think we are intentionally invoking UB and crashing. The order just
seems mixed up to me.

[dnicoara, 2015/04/15 13:43:41]

Yeah, order is messed up.

-    }
+    if (!is_mirrored)
+      RemoveController(it);
   }
 }
 
@@ -284,7 +282,7 @@ bool ScreenManager::HandleMirrorMode(
     uint32_t connector) {
   (*mirror)->AddCrtc((*original)->RemoveCrtc(drm, crtc));
   if ((*mirror)->Enable()) {
-    controllers_.erase(original);
+    RemoveController(original);

[dnicoara, 2015/04/15 13:43:41]

This will cause UpdateControllerToWindowMapping() to be called twice since
HandleMirrorMode is just a helper function. The update happens in
ConfigureDisplayController().

[spang, 2015/04/15 13:48:24]

Done.

     return true;
   }
 
@@ -333,4 +331,10 @@ DrmWindow* ScreenManager::FindWindowAt(const gfx::Rect& bounds) const {
   return nullptr;
 }
 
+void ScreenManager::RemoveController(
+    HardwareDisplayControllers::iterator controller) {
+  controllers_.erase(controller);
+  UpdateControllerToWindowMapping();
+}
+
 }  // namespace ui