drm: Reset associated universal plane states when finalizing a crtc
authorAlexandros Frantzis <alexandros.frantzis@collabora.com>
Mon, 18 May 2020 12:26:01 +0000 (15:26 +0300)
committerDaniel Stone <daniels@collabora.com>
Mon, 17 Aug 2020 09:44:45 +0000 (09:44 +0000)
When dissociating a universal plane from a crtc, we currently don't
reset the current state of the plane (plane->state_cur). When attempting
to use this plane in the future, we can run into invalid memory accesses
due to left over associations with potentially freed drm backend
objects. This commit resets the state of the scanout and cursor
universal planes associated with a crtc.

The following scenario exhibits the problem:

1. Start a (fullscreen) client that is suitable for and assigned to
   the scanout plane. The plane's state_cur->output value is set.
2. Unplug the monitor: the scanout plane is "released" but still
   maintains the state_cur->output association.
3. Replug the monitor: the plane is deemed unavailable due to an
   existing, albeit invalid, state_cur->output value. Note the memory
   errors trying to access the drm_output which was freed at step (2).

Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
libweston/backend-drm/drm.c

index 7c1c0b4024137a251a0d05207f0a79ebf0097559..83993da5a98712c1cbe2e900aa9e00ca5a309b97 100644 (file)
@@ -1679,23 +1679,29 @@ drm_output_fini_crtc(struct drm_output *output)
        struct drm_backend *b = to_drm_backend(output->base.compositor);
        uint32_t *unused;
 
-       if (!b->universal_planes && !b->shutting_down) {
-               /* With universal planes, the 'special' planes are allocated at
-                * startup, freed at shutdown, and live on the plane list in
-                * between. We want the planes to continue to exist and be freed
-                * up for other outputs.
-                *
-                * Without universal planes, our special planes are
-                * pseudo-planes allocated at output creation, freed at output
-                * destruction, and not usable by other outputs.
-                *
-                * On the other hand, if the compositor is already shutting down,
-                * the plane has already been destroyed.
-                */
-               if (output->cursor_plane)
-                       drm_plane_destroy(output->cursor_plane);
-               if (output->scanout_plane)
-                       drm_plane_destroy(output->scanout_plane);
+       /* If the compositor is already shutting down, the planes have already
+        * been destroyed. */
+       if (!b->shutting_down) {
+               if (!b->universal_planes) {
+                       /* Without universal planes, our special planes are
+                        * pseudo-planes allocated at output creation, freed at
+                        * output destruction, and not usable by other outputs.
+                        */
+                       if (output->cursor_plane)
+                               drm_plane_destroy(output->cursor_plane);
+                       if (output->scanout_plane)
+                               drm_plane_destroy(output->scanout_plane);
+               } else {
+                       /* With universal planes, the 'special' planes are
+                        * allocated at startup, freed at shutdown, and live on
+                        * the plane list in between. We want the planes to
+                        * continue to exist and be freed up for other outputs.
+                        */
+                       if (output->cursor_plane)
+                               drm_plane_reset_state(output->cursor_plane);
+                       if (output->scanout_plane)
+                               drm_plane_reset_state(output->scanout_plane);
+               }
        }
 
        drm_property_info_free(output->props_crtc, WDRM_CRTC__COUNT);