mirror of
https://github.com/X11Libre/xserver.git
synced 2026-10-26 01:59:57 +00:00
modesetting: fix crash on flip queue retry and use-after-free in drm event queue
сheck for non-retryable errors (errno != EBUSY/EAGAIN) in do_queue_flip_on_crtc to prevent invalid retry and event flush loops I find in drm events and prevent use-after-free in ms_drm_sequence_handler and ms_drm_abort_scrn by safely restarting queue iteration when event callbacks modify DRM queue Marks page flip sequences with kernel_queued upon successful submission to prevent premature freeing of queue entries Affected issues: - https://github.com/X11Libre/xserver/issues/1200 - https://github.com/X11Libre/xserver/issues/2174 - https://github.com/X11Libre/xserver/issues/3432 Signed-off-by: Herman Semenoff <GermanAizek@yandex.ru>
This commit is contained in:
parent
71f28aec8b
commit
08619070f8
4 changed files with 34 additions and 8 deletions
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
|
|
@ -1166,6 +1166,8 @@ drmmode_SharedPixmapFlip(PixmapPtr frontTarget, xf86CrtcPtr crtc,
|
|||
return FALSE;
|
||||
}
|
||||
|
||||
ms_drm_set_seq_queued(ppriv_front->flip_seq, UINT64_MAX);
|
||||
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue