[PATCH libdrm] etnaviv: Fix double-free in etna_bo_cache_free()

[Christian Gmeiner]

Am So., 2. Juni 2019 um 01:37 Uhr schrieb Marek Vasut <marex at denx.de>:
>
> The following situation can happen in a multithreaded OpenGL application.
> A BO is submitted from etna_cmd_stream #1 with flags set for read.
> A BO is submitted from etna_cmd_stream #2 with flags set for write.
> This triggers a flush on stream #1 and clears the BO's current_stream
> pointer. If at this point, stream #2 attempts to queue BO again, which
> does happen, the BO will be added to the submit list twice. The Linux
> kernel driver correctly detects this and warns about it with "BO at
> index %u already on submit list" kernel message.
>
> However, when cleaning the BO cache in etna_bo_cache_free(), the BO
> which was submitted twice will also be free()d twice, this triggering
> a glibc double free detector.
>
> The fix is easy, even if the BO does not have current_stream set,
> iterate over current streams' list of BOs before adding the BO to it
> and verify that the BO is not yet there.
>
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Christian Gmeiner <christian.gmeiner at gmail.com>
> Cc: Lucas Stach <l.stach at pengutronix.de>

Reviewed-by: Christian Gmeiner <christian.gmeiner at gmail.com>

Will land this patch even libdrm parts got moved to mesa as it fixes
a serious issue and somebody might use newest libdrm with mesa < 19.1.

> ---
>  etnaviv/etnaviv_cmd_stream.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/etnaviv/etnaviv_cmd_stream.c b/etnaviv/etnaviv_cmd_stream.c
> index 7139c324..261777b0 100644
> --- a/etnaviv/etnaviv_cmd_stream.c
> +++ b/etnaviv/etnaviv_cmd_stream.c
> @@ -150,11 +150,7 @@ static uint32_t bo2idx(struct etna_cmd_stream *stream, struct etna_bo *bo,
>
>         pthread_mutex_lock(&idx_lock);
>
> -       if (!bo->current_stream) {
> -               idx = append_bo(stream, bo);
> -               bo->current_stream = stream;
> -               bo->idx = idx;
> -       } else if (bo->current_stream == stream) {
> +       if (bo->current_stream == stream) {
>                 idx = bo->idx;
>         } else {
>                 /* slow-path: */
> @@ -165,6 +161,8 @@ static uint32_t bo2idx(struct etna_cmd_stream *stream, struct etna_bo *bo,
>                         /* not found */
>                         idx = append_bo(stream, bo);
>                 }
> +               bo->current_stream = stream;
> +               bo->idx = idx;
>         }
>         pthread_mutex_unlock(&idx_lock);
>
> --
> 2.20.1
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy