diff --git a/hw/xfree86/drivers/video/modesetting/driver.h b/hw/xfree86/drivers/video/modesetting/driver.h index 79e68b339a..d79d87cf78 100644 --- a/hw/xfree86/drivers/video/modesetting/driver.h +++ b/hw/xfree86/drivers/video/modesetting/driver.h @@ -215,6 +215,7 @@ void ms_drm_abort(ScrnInfoPtr scrn, Bool (*match)(void *data, void *match_data), void *match_data); void ms_drm_abort_seq(ScrnInfoPtr scrn, uint32_t seq); +void ms_drm_set_seq_queued(uint32_t seq, uint64_t msc); Bool ms_drm_queue_is_empty(void); diff --git a/hw/xfree86/drivers/video/modesetting/drmmode_display.c b/hw/xfree86/drivers/video/modesetting/drmmode_display.c index 368a2b769f..743db1c8ae 100644 --- a/hw/xfree86/drivers/video/modesetting/drmmode_display.c +++ b/hw/xfree86/drivers/video/modesetting/drmmode_display.c @@ -1162,6 +1162,8 @@ drmmode_SharedPixmapFlip(PixmapPtr frontTarget, xf86CrtcPtr crtc, return FALSE; } + ms_drm_set_seq_queued(ppriv_front->flip_seq, UINT64_MAX); + return TRUE; } diff --git a/hw/xfree86/drivers/video/modesetting/pageflip.c b/hw/xfree86/drivers/video/modesetting/pageflip.c index e50a722dfc..1d7654b53d 100644 --- a/hw/xfree86/drivers/video/modesetting/pageflip.c +++ b/hw/xfree86/drivers/video/modesetting/pageflip.c @@ -186,6 +186,12 @@ do_queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc, uint32_t flags, drmmode_tearfree_ptr trf = &drmmode_crtc->tearfree; while (drmmode_crtc_flip(crtc, fb_id, x, y, flags, (void *)(long)seq)) { + /* If we failed for a reason other than a busy queue, abort and return error. */ + if (errno != EBUSY && errno != EAGAIN) { + ms_drm_abort_seq(crtc->scrn, seq); + return TRUE; + } + /* We may have failed because the event queue was full. Flush it * and retry. If there was nothing to flush, then we failed for * some other reason and should just return an error. @@ -248,6 +254,8 @@ queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc, crtc->x, crtc->y)) return QUEUE_FLIP_DRM_FLUSH_FAILED; + ms_drm_set_seq_queued(seq, UINT64_MAX); + /* The page flip succeeded. */ return QUEUE_FLIP_SUCCESS; } @@ -675,6 +683,7 @@ ms_do_tearfree_flip(ScreenPtr screen, xf86CrtcPtr crtc) seq, trf->buf[idx].fb_id, 0, 0)) goto no_flip; + ms_drm_set_seq_queued(seq, UINT64_MAX); trf->flip_seq = seq; return FALSE; diff --git a/hw/xfree86/drivers/video/modesetting/vblank.c b/hw/xfree86/drivers/video/modesetting/vblank.c index e0de33f59f..695ce2baaf 100644 --- a/hw/xfree86/drivers/video/modesetting/vblank.c +++ b/hw/xfree86/drivers/video/modesetting/vblank.c @@ -273,7 +273,7 @@ ms_drm_set_seq_msc(uint32_t seq, uint64_t msc) } } -static void +void ms_drm_set_seq_queued(uint32_t seq, uint64_t msc) { drmmode_crtc_private_ptr drmmode_crtc; @@ -522,11 +522,18 @@ ms_drm_abort_one(struct ms_drm_queue *q) static void ms_drm_abort_scrn(ScrnInfoPtr scrn) { - struct ms_drm_queue *q, *tmp; + struct ms_drm_queue *q; - xorg_list_for_each_entry_safe(q, tmp, &ms_drm_queue, list) { - if (q->scrn == scrn) + /* Restart traversal from the head when an entry is aborted, because + * ms_drm_abort_one() may trigger callbacks or re-entrant list modifications + * which invalidate cached list pointers. + */ +restart: + xorg_list_for_each_entry(q, &ms_drm_queue, list) { + if (q->scrn == scrn && !q->aborted) { ms_drm_abort_one(q); + goto restart; + } } } @@ -536,9 +543,9 @@ ms_drm_abort_scrn(ScrnInfoPtr scrn) void ms_drm_abort_seq(ScrnInfoPtr scrn, uint32_t seq) { - struct ms_drm_queue *q, *tmp; + struct ms_drm_queue *q; - xorg_list_for_each_entry_safe(q, tmp, &ms_drm_queue, list) { + xorg_list_for_each_entry(q, &ms_drm_queue, list) { if (q->seq == seq) { ms_drm_abort_one(q); break; @@ -598,13 +605,20 @@ ms_drm_sequence_handler(int fd, uint64_t frame, uint64_t ns, Bool is64bit, uint6 if (!crtc) return; - /* Now run all of the vblank events for this CRTC with an expired MSC */ - xorg_list_for_each_entry_safe(q, tmp, &ms_drm_queue, list) { + /* Now run all of the vblank events for this CRTC with an expired MSC. + * Restart traversal from the head whenever a handler is executed, because + * q->handler() invokes external callbacks (e.g. Present, DRI2, TearFree) + * which can abort or modify other entries in ms_drm_queue, invalidating + * any cached next pointer and causing use-after-free. + */ +restart: + xorg_list_for_each_entry(q, &ms_drm_queue, list) { if (q->crtc == crtc && q->msc <= msc) { xorg_list_del(&q->list); if (!q->aborted) q->handler(msc, ns / 1000, q->data); free(q); + goto restart; } }