2019-07-28 17:12:18

by Colin King

Subject: [PATCH] media: vsp1: fix memory leak of dl on error return path

From: Colin Ian King <[email protected]>

Currently when the call vsp1_dl_body_get fails and returns null the
error return path leaks the allocation of dl. Fix this by kfree'ing
dl before returning.

Addresses-Coverity: ("Resource leak")
Fixes: 5d7936b8e27d ("media: vsp1: Convert display lists to use new body pool")
Signed-off-by: Colin Ian King <[email protected]>
---
drivers/media/platform/vsp1/vsp1_dl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index 104b6f514536..d7b43037e500 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -557,8 +557,10 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm)

/* Get a default body for our list. */
dl->body0 = vsp1_dl_body_get(dlm->pool);
- if (!dl->body0)
+ if (!dl->body0) {
+ kfree(dl);
return NULL;
+ }

header_offset = dl->body0->max_entries * sizeof(*dl->body0->entries);

--
2.20.1

2019-07-29 16:00:15

by Kieran Bingham

Subject: Re: [PATCH] media: vsp1: fix memory leak of dl on error return path

Hi Colin,

On 28/07/2019 18:11, Colin King wrote:
> From: Colin Ian King <[email protected]>
>
> Currently when the call vsp1_dl_body_get fails and returns null the
> error return path leaks the allocation of dl. Fix this by kfree'ing
> dl before returning.

Eeep. This does indeed look to be the case.

>
> Addresses-Coverity: ("Resource leak")
> Fixes: 5d7936b8e27d ("media: vsp1: Convert display lists to use new body pool")
> Signed-off-by: Colin Ian King <[email protected]>

Thank you!

Reviewed-by: Kieran Bingham <[email protected]>


> ---
> drivers/media/platform/vsp1/vsp1_dl.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
> index 104b6f514536..d7b43037e500 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -557,8 +557,10 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm)
>
> /* Get a default body for our list. */
> dl->body0 = vsp1_dl_body_get(dlm->pool);
> - if (!dl->body0)
> + if (!dl->body0) {
> + kfree(dl);
> return NULL;
> + }
>
> header_offset = dl->body0->max_entries * sizeof(*dl->body0->entries);
>
>

2019-07-29 16:00:27

by Colin King

Subject: Re: [PATCH] media: vsp1: fix memory leak of dl on error return path

On 29/07/2019 13:11, Kieran Bingham wrote:
> Hi Colin,
>
> On 28/07/2019 18:11, Colin King wrote:
>> From: Colin Ian King <[email protected]>
>>
>> Currently when the call vsp1_dl_body_get fails and returns null the
>> error return path leaks the allocation of dl. Fix this by kfree'ing
>> dl before returning.
>
> Eeep. This does indeed look to be the case.
>
>>
>> Addresses-Coverity: ("Resource leak")
>> Fixes: 5d7936b8e27d ("media: vsp1: Convert display lists to use new body pool")
>> Signed-off-by: Colin Ian King <[email protected]>
>
> Thank you!

Thank static analysis :-)

>
> Reviewed-by: Kieran Bingham <[email protected]>
>
>
>> ---
>> drivers/media/platform/vsp1/vsp1_dl.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
>> index 104b6f514536..d7b43037e500 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>> @@ -557,8 +557,10 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm)
>>
>> /* Get a default body for our list. */
>> dl->body0 = vsp1_dl_body_get(dlm->pool);
>> - if (!dl->body0)
>> + if (!dl->body0) {
>> + kfree(dl);
>> return NULL;
>> + }
>>
>> header_offset = dl->body0->max_entries * sizeof(*dl->body0->entries);
>>
>>
>

2019-07-29 17:20:50

by Kieran Bingham

Subject: Re: [PATCH] media: vsp1: fix memory leak of dl on error return path

On 29/07/2019 13:12, Colin Ian King wrote:
> On 29/07/2019 13:11, Kieran Bingham wrote:
>> Hi Colin,
>>
>> On 28/07/2019 18:11, Colin King wrote:
>>> From: Colin Ian King <[email protected]>
>>>
>>> Currently when the call vsp1_dl_body_get fails and returns null the
>>> error return path leaks the allocation of dl. Fix this by kfree'ing
>>> dl before returning.
>>
>> Eeep. This does indeed look to be the case.
>>
>>>
>>> Addresses-Coverity: ("Resource leak")
>>> Fixes: 5d7936b8e27d ("media: vsp1: Convert display lists to use new body pool")
>>> Signed-off-by: Colin Ian King <[email protected]>
>>
>> Thank you!
>
> Thank static analysis :-)

Bah, that's just the hammer - you're the one finding the nails :-D
--
Kieran


>
>>
>> Reviewed-by: Kieran Bingham <[email protected]>
>>
>>
>>> ---
>>> drivers/media/platform/vsp1/vsp1_dl.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
>>> index 104b6f514536..d7b43037e500 100644
>>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>>> @@ -557,8 +557,10 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm)
>>>
>>> /* Get a default body for our list. */
>>> dl->body0 = vsp1_dl_body_get(dlm->pool);
>>> - if (!dl->body0)
>>> + if (!dl->body0) {
>>> + kfree(dl);
>>> return NULL;
>>> + }
>>>
>>> header_offset = dl->body0->max_entries * sizeof(*dl->body0->entries);
>>>
>>>
>>
>