From: Christopher James Halse Rogers <[email protected]>
The SwapBuffers request requires that we trigger the swap at some point in the future. Sane drivers implement this by passing this request to something that will trigger a callback with the buffer pointers at the appropriate time. The client can cause those buffers to be freed in X before the trigger occurs, most easily by quitting. This leads to a server crash in the driver when trying to do the swap. See http://bugs.freedesktop.org/show_bug.cgi?id=29065 for Radeon and https://bugs.freedesktop.org/show_bug.cgi?id=28080 for Intel. Signed-off-by: Christopher James Halse Rogers <[email protected]> Signed-off-by: Pauli Nieminen <[email protected]> --- hw/xfree86/dri2/dri2.c | 80 +++++++++++++++++++++++++++++++++++++++--------- hw/xfree86/dri2/dri2.h | 1 + 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 7746a1d..b50206e 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -108,12 +108,35 @@ typedef struct _DRI2Screen { ConfigNotifyProcPtr ConfigNotify; } DRI2ScreenRec; +typedef struct _DRI2SwapCompleteDataRec { + DRI2BufferPtr src; + DRI2BufferPtr dest; + void * data; +} DRI2SwapCompleteDataRec, *DRI2SwapCompleteDataPtr; + static DRI2ScreenPtr DRI2GetScreen(ScreenPtr pScreen) { return dixLookupPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey); } +static void +buffer_ref(DRI2BufferPtr buffer) +{ + buffer->refcnt++; +} + +static void +buffer_unref(DRI2DrawablePtr pPriv, DRI2BufferPtr buffer) +{ + DRI2ScreenPtr ds = pPriv->dri2_screen; + + buffer->refcnt--; + if (buffer->refcnt == 0) { + (*ds->DestroyBuffer)(pPriv, buffer); + } +} + DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw) { @@ -325,7 +348,6 @@ DRI2DestroyDrawable(ClientPtr client, DRI2DrawablePtr pPriv, XID id) static int DRI2DrawableGone(pointer p, XID id) { DRI2DrawablePtr pPriv = p; - DRI2ScreenPtr ds = pPriv->dri2_screen; DRI2DrawableRefPtr ref, next; DrawablePtr pDraw = pPriv->drawable; int i; @@ -355,7 +377,7 @@ static int DRI2DrawableGone(pointer p, XID id) if (pPriv->buffers != NULL) { for (i = 0; i < pPriv->bufferCount; i++) - (*ds->DestroyBuffer)(pPriv, pPriv->buffers[i]); + buffer_unref(pPriv, pPriv->buffers[i]); free(pPriv->buffers); } @@ -405,6 +427,7 @@ allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds, || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) { *buffer = (*ds->CreateBuffer)(pDraw, attachment, format); + buffer_ref(*buffer); pPriv->serialNumber = DRI2DrawableSerial(pDraw); return TRUE; @@ -419,14 +442,12 @@ static void update_dri2_drawable_buffers(DRI2DrawablePtr pPriv, DrawablePtr pDraw, DRI2BufferPtr *buffers, int *out_count, int *width, int *height) { - DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); int i; if (pPriv->buffers != NULL) { for (i = 0; i < pPriv->bufferCount; i++) { - if (pPriv->buffers[i] != NULL) { - (*ds->DestroyBuffer)(pPriv, pPriv->buffers[i]); - } + if (pPriv->buffers[i] != NULL) + buffer_unref(pPriv, pPriv->buffers[i]); } free(pPriv->buffers); @@ -562,7 +583,7 @@ err_out: for (i = 0; i < count; i++) { if (buffers[i] != NULL) - (*ds->DestroyBuffer)(pPriv, buffers[i]); + buffer_unref(pPriv, buffers[i]); } free(buffers); @@ -771,11 +792,20 @@ DRI2WakeClient(ClientPtr client, DRI2DrawablePtr pPriv, int frame, } } +static void +free_swap_complete_data (DRI2DrawablePtr pPriv, DRI2SwapCompleteDataPtr pSwapData) +{ + buffer_unref(pPriv, pSwapData->src); + buffer_unref(pPriv, pSwapData->dest); + free(pSwapData); +} + void DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame, unsigned int tv_sec, unsigned int tv_usec, int type, DRI2SwapEventPtr swap_complete, void *swap_data) { + DRI2SwapCompleteDataPtr pSwapData = swap_data; DrawablePtr pDraw = pPriv->drawable; CARD64 ust = 0; @@ -783,11 +813,6 @@ DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame, pPriv->swap_count++; pPriv->refcnt--; - if (pPriv->refcnt == 0) { - DRI2DrawableGone(pPriv, 0); - return; - } - if (pDraw) { BoxRec box; RegionRec region; @@ -803,12 +828,18 @@ DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame, ust = ((CARD64)tv_sec * 1000000) + tv_usec; if (swap_complete) - swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count); + swap_complete(client, pSwapData->data, type, ust, frame, + pPriv->swap_count); pPriv->last_swap_msc = frame; pPriv->last_swap_ust = ust; DRI2WakeClient(client, pPriv, frame, tv_sec, tv_usec); + + free_swap_complete_data(pPriv, pSwapData); + + if (pPriv->refcnt == 0) + DRI2DrawableGone(pPriv, 0); } Bool @@ -837,6 +868,7 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc, ScreenPtr pScreen = pPriv->dri2_screen->screen; DRI2ScreenPtr ds = pPriv->dri2_screen; DRI2BufferPtr pDestBuffer = NULL, pSrcBuffer = NULL; + DRI2SwapCompleteDataPtr pSwapData; int ret, i; CARD64 ust, current_msc; @@ -858,6 +890,23 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc, return BadDrawable; } + pSwapData = malloc(sizeof *pSwapData); + if (pSwapData == NULL) + return BadAlloc; + + /* + * Take a reference to the buffers we're exchanging. + * This ensures that these buffers will remain valid until the swap + * is completed. + * + * DRI2SwapComplete is required to unref these buffers. + */ + buffer_ref(pSrcBuffer); + buffer_ref(pDestBuffer); + pSwapData->src = pSrcBuffer; + pSwapData->dest = pDestBuffer; + pSwapData->data = data; + /* Old DDX or no swap interval, just blit */ if (!ds->ScheduleSwap || !pPriv->swap_interval) { BoxRec box; @@ -874,7 +923,7 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc, (*ds->CopyRegion)(pDraw, ®ion, pDestBuffer, pSrcBuffer); DRI2SwapComplete(client, pPriv, target_msc, 0, 0, DRI2_BLIT_COMPLETE, - func, data); + func, pSwapData); return Success; } @@ -914,12 +963,13 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc, pPriv->swapsPending++; pPriv->refcnt++; ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer, - swap_target, divisor, remainder, func, data); + swap_target, divisor, remainder, func, pSwapData); if (!ret) { pPriv->swapsPending--; /* didn't schedule */ pPriv->refcnt--; xf86DrvMsg(pScreen->myNum, X_ERROR, "[DRI2] %s: driver failed to schedule swap\n", __func__); + free_swap_complete_data(pPriv, pSwapData); return BadDrawable; } diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index c324b8f..6a5dd98 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -44,6 +44,7 @@ typedef struct { unsigned int flags; unsigned int format; void *driverPrivate; + unsigned int refcnt; } DRI2BufferRec, *DRI2BufferPtr; struct _DRI2Drawable; -- 1.7.0.4 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
