[Mesa-dev] [PATCH v6 1/2] gallium/winsys/kms: Fix possible leak in map/unmap.

[Emil Velikov]

Hi Lepton,

On 19 March 2018 at 17:33, Lepton Wu <lepton at chromium.org> wrote:
> If user calls map twice for kms_sw_displaytarget, the first mapped
> buffer could get leaked. Instead of calling mmap every time, just
> reuse previous mapping. Since user could map same displaytarget with
> different flags, we have to keep two different pointers, one for rw
> mapping and one for ro mapping. Also introduce reference count for
> mapped buffer so we can unmap them at right time.
>
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> Reviewed-by: Tomasz Figa <tfiga at chromium.org>
> Signed-off-by: Lepton Wu <lepton at chromium.org>

Nit: normally it's a good idea to have brief revision log when sending
new version:
v2:
 - split from larger patch (Emil)
v3:
 - remove munmap w/a from dt_destory(Emil)
...

> @@ -170,6 +172,14 @@ kms_sw_displaytarget_destroy(struct sw_winsys *ws,
>     if (kms_sw_dt->ref_count > 0)
>        return;
>
> +   if (kms_sw_dt->map_count > 0) {
> +      DEBUG_PRINT("KMS-DEBUG: fix leaked map buffer %u\n", kms_sw_dt->handle);
> +      munmap(kms_sw_dt->mapped, kms_sw_dt->size);
> +      kms_sw_dt->mapped = NULL;
> +      munmap(kms_sw_dt->ro_mapped, kms_sw_dt->size);
> +      kms_sw_dt->ro_mapped = NULL;
> +   }
> +
I could swear this workaround was missing in earlier revisions. I
don't see anything in Tomasz' reply that suggesting we should bring it
back?
AFAICT the added refcounting makes no difference - the driver isn't
cleaning up after itself.

Am I missing something?

-Emil