Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20211206104315.12516-1-amhamza.mgc@gmail.com>
References: <163878547435.2211244.3536763956780138208@Monstersaurus> <20211206104315.12516-1-amhamza.mgc@gmail.com>
Subject: Re: [PATCH v3] media: venus: vdec: fixed possible memory leak issue
From:   Kieran Bingham <kieran.bingham@ideasonboard.com>
Cc:     agross@kernel.org, bjorn.andersson@linaro.org, mchehab@kernel.org,
        stanimir.varbanov@linaro.org, linux-media@vger.kernel.org,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        amhamza.mgc@gmail.com
To:     Ameer Hamza <amhamza.mgc@gmail.com>
Date:   Mon, 06 Dec 2021 11:32:21 +0000
Message-ID: <163879034159.2211244.5962318772307516005@Monstersaurus>
User-Agent: alot/0.10
Precedence: bulk

Quoting Ameer Hamza (2021-12-06 10:43:15)
> The venus_helper_alloc_dpb_bufs() implementation allows an early return
> on an error path when checking the id from ida_alloc_min() which would
> not release the earlier buffer allocation.
>=20
> Move the direct kfree() from the error checking of dma_alloc_attrs() to
> the common fail path to ensure that allocations are released on all
> error paths in this function.
>=20
> Addresses-Coverity: 1494120 ("Resource leak")
>=20
> Fixes: 40d87aafee29 ("media: venus: vdec: decoded picture buffer handling=
 during reconfig sequence")
>=20

No need for blank lines between those tags, and when someone provides a
Reviewed-by tag, you can collect it into your patch for future versions
unless you feel you've modified the patch so much that it doesn't apply
anymore.

So this can still be added (no need to repost to add to this patch, I
believe the integration scripts likely pick up tags added to a patch,
but won't pick up ones added to previous versions).

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>


> Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com>
>=20
> ---
> Changes in v3:
> Updated description and added fix tag
> ---
>  drivers/media/platform/qcom/venus/helpers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>=20
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/=
platform/qcom/venus/helpers.c
> index 84c3a511ec31..0bca95d01650 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -189,7 +189,6 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *in=
st)
>                 buf->va =3D dma_alloc_attrs(dev, buf->size, &buf->da, GFP=
_KERNEL,
>                                           buf->attrs);
>                 if (!buf->va) {
> -                       kfree(buf);
>                         ret =3D -ENOMEM;
>                         goto fail;
>                 }
> @@ -209,6 +208,7 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *in=
st)
>         return 0;
> =20
>  fail:
> +       kfree(buf);
>         venus_helper_free_dpb_bufs(inst);
>         return ret;
>  }
> --=20
> 2.25.1
>