From 9940fb42058e0b0815e0edb202ca69b7853aead5 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Mon, 18 Sep 2017 15:17:31 -0700 Subject: [PATCH] broadcom/vc4: Fix use-after-free trying to mix a quad and tile clear. The blitter will bind just the depth buffer, which flushes the current job if we had both a color and depth/stencil. If the clear was doing partial depth/stencil (quad-based) and color (tile-based), we'd go on to try to set up the rest of the tile clear in the now flushed job. Instead, move the partial clear up before we start setting up the job for the current FBO state, and re-fetch the job if we're continuing on to a tile-based clear. Fixes valgrind failures in fbo-depthtex. Fixes: 9421a6065c4e ("vc4: Fix fallback to quad clears of depth in GLX.") --- src/gallium/drivers/vc4/vc4_draw.c | 56 ++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/src/gallium/drivers/vc4/vc4_draw.c b/src/gallium/drivers/vc4/vc4_draw.c index 2074931..fdf983d 100644 --- a/src/gallium/drivers/vc4/vc4_draw.c +++ b/src/gallium/drivers/vc4/vc4_draw.c @@ -502,6 +502,37 @@ vc4_clear(struct pipe_context *pctx, unsigned buffers, struct vc4_context *vc4 = vc4_context(pctx); struct vc4_job *job = vc4_get_job_for_fbo(vc4); + if (buffers & PIPE_CLEAR_DEPTHSTENCIL) { + struct vc4_resource *rsc = + vc4_resource(vc4->framebuffer.zsbuf->texture); + unsigned zsclear = buffers & PIPE_CLEAR_DEPTHSTENCIL; + + /* Clearing ZS will clear both Z and stencil, so if we're + * trying to clear just one then we need to draw a quad to do + * it instead. We need to do this before setting up + * tile-based clears in vc4->job, because the blitter may + * submit the current job. + */ + if ((zsclear == PIPE_CLEAR_DEPTH || + zsclear == PIPE_CLEAR_STENCIL) && + (rsc->initialized_buffers & ~(zsclear | job->cleared)) && + util_format_is_depth_and_stencil(vc4->framebuffer.zsbuf->format)) { + perf_debug("Partial clear of Z+stencil buffer, " + "drawing a quad instead of fast clearing\n"); + vc4_blitter_save(vc4); + util_blitter_clear(vc4->blitter, + vc4->framebuffer.width, + vc4->framebuffer.height, + 1, + zsclear, + NULL, depth, stencil); + buffers &= ~zsclear; + if (!buffers) + return; + job = vc4_get_job_for_fbo(vc4); + } + } + /* We can't flag new buffers for clearing once we've queued draws. We * could avoid this by using the 3d engine to clear. */ @@ -537,29 +568,6 @@ vc4_clear(struct pipe_context *pctx, unsigned buffers, if (buffers & PIPE_CLEAR_DEPTHSTENCIL) { struct vc4_resource *rsc = vc4_resource(vc4->framebuffer.zsbuf->texture); - unsigned zsclear = buffers & PIPE_CLEAR_DEPTHSTENCIL; - - /* Clearing ZS will clear both Z and stencil, so if we're - * trying to clear just one then we need to draw a quad to do - * it instead. - */ - if ((zsclear == PIPE_CLEAR_DEPTH || - zsclear == PIPE_CLEAR_STENCIL) && - (rsc->initialized_buffers & ~(zsclear | job->cleared)) && - util_format_is_depth_and_stencil(vc4->framebuffer.zsbuf->format)) { - perf_debug("Partial clear of Z+stencil buffer, " - "drawing a quad instead of fast clearing\n"); - vc4_blitter_save(vc4); - util_blitter_clear(vc4->blitter, - vc4->framebuffer.width, - vc4->framebuffer.height, - 1, - zsclear, - NULL, depth, stencil); - buffers &= ~zsclear; - if (!buffers) - return; - } /* Though the depth buffer is stored with Z in the high 24, * for this field we just need to store it in the low 24. @@ -571,7 +579,7 @@ vc4_clear(struct pipe_context *pctx, unsigned buffers, if (buffers & PIPE_CLEAR_STENCIL) job->clear_stencil = stencil; - rsc->initialized_buffers |= zsclear; + rsc->initialized_buffers |= (buffers & PIPE_CLEAR_DEPTHSTENCIL); } job->draw_min_x = 0; -- 2.7.4