On Wed, 20 Dec 2017 12:26:23 +0000 Daniel Stone <dani...@collabora.com> wrote:
> Use a real drm_plane to back the scanout plane, displacing
> output->fb_{last,cur,pending} to their plane-tracked equivalents.
>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> ---
> libweston/compositor-drm.c | 222
> ++++++++++++++++++++++++++++++---------------
> 1 file changed, 149 insertions(+), 73 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index a600ef40b..6ad616ebd 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -3355,6 +3398,19 @@ drm_output_init_egl(struct drm_output *output, struct
> drm_backend *b)
> static void
> drm_output_fini_egl(struct drm_output *output)
> {
> + struct drm_backend *b = to_drm_backend(output->base.compositor);
> +
> + /* Destroying the GBM surface will destroy all our GBM buffers,
> + * regardless of refcount. Ensure we destroy them here. */
I suppose that happens inside eglDestroySurface().
> + if (!b->shutting_down &&
> + output->scanout_plane->state_cur->fb &&
> + output->scanout_plane->state_cur->fb->type == BUFFER_GBM_SURFACE) {
> + drm_plane_state_free(output->scanout_plane->state_cur, true);
> + output->scanout_plane->state_cur =
> + drm_plane_state_alloc(NULL, output->scanout_plane);
> + output->scanout_plane->state_cur->complete = true;
> + }
> +
> gl_renderer->output_destroy(&output->base);
> gbm_surface_destroy(output->gbm_surface);
> drm_output_fini_cursor_egl(output);
> @@ -3420,8 +3476,20 @@ err:
> static void
> drm_output_fini_pixman(struct drm_output *output)
> {
> + struct drm_backend *b = to_drm_backend(output->base.compositor);
> unsigned int i;
>
> + /* Destroying the Pixman surface will destroy all our buffers,
> + * regardless of refcount. Ensure we destroy them here. */
I think this is only half-true. The pixman_image_t does get destroyed,
but drm_fb is still refcounted and recount honoured it seems. Only
drm_output_render_pixman() uses the image, and I assume that won't
called anymore. drm_fb is the one that maintains the backing storage in
this case.
Therefore this workaround here is not necessary for avoiding
use-after-free, but maybe we want this to ensure the drm_fb really does
get destroyed anyway?
At most, the comment here could be adjusted.
> + if (!b->shutting_down &&
> + output->scanout_plane->state_cur->fb &&
> + output->scanout_plane->state_cur->fb->type == BUFFER_PIXMAN_DUMB) {
> + drm_plane_state_free(output->scanout_plane->state_cur, true);
> + output->scanout_plane->state_cur =
> + drm_plane_state_alloc(NULL, output->scanout_plane);
> + output->scanout_plane->state_cur->complete = true;
> + }
> +
> pixman_renderer_output_destroy(&output->base);
> pixman_region32_fini(&output->previous_damage);
>
Anyway,
Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
Thanks,
pq
pgpxjnV6LJb81.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
