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

[Emil Velikov]

On 6 March 2018 at 22:43, 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.
>
> Change-Id: I65308f0ff2640bd57b2577c6a3469540c9722859
> Signed-off-by: Lepton Wu <lepton at chromium.org>
> ---
>  .../winsys/sw/kms-dri/kms_dri_sw_winsys.c     | 26 ++++++++++++++-----
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
> index 22e1c936ac5..30343222470 100644
> --- a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
> +++ b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
> @@ -70,6 +70,7 @@ struct kms_sw_displaytarget
>
>     uint32_t handle;
>     void *mapped;
> +   void *ro_mapped;
>
>     int ref_count;
>     struct list_head link;
> @@ -170,6 +171,11 @@ kms_sw_displaytarget_destroy(struct sw_winsys *ws,
>     if (kms_sw_dt->ref_count > 0)
>        return;
>
> +   if (kms_sw_dt->ro_mapped)
> +      munmap(kms_sw_dt->ro_mapped, kms_sw_dt->size);
> +   if (kms_sw_dt->mapped)
> +      munmap(kms_sw_dt->mapped, kms_sw_dt->size);
> +
I'm not 100% sure about this. There's a reason why dt_unmap exists.

Skimming through the existing code [1] - there's a handful of cases
that indicate the leaks you're hitting.
I'd look into addressing those properly because as-is this looks alike
a duck-taping the problem.

With the hunk gone the patch is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil

[1] $ git grep -20 -w displaytarget_[a-z]*map