From 53a71cb186ff4f275d3c827abbfdbfda3670835f Mon Sep 17 00:00:00 2001 From: Alexandros Frantzis Date: Mon, 18 May 2020 15:26:01 +0300 Subject: [PATCH] drm: Reset associated universal plane states when finalizing a crtc 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 --- libweston/backend-drm/drm.c | 40 +++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/libweston/backend-drm/drm.c b/libweston/backend-drm/drm.c index 7c1c0b40..83993da5 100644 --- a/libweston/backend-drm/drm.c +++ b/libweston/backend-drm/drm.c @@ -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);