media: vsp1: fix memory leak of dl on error return path
diff mbox series

Message ID 20190728171124.14202-1-colin.king@canonical.com
State New
Delegated to: Kieran Bingham
Headers show
Series
  • media: vsp1: fix memory leak of dl on error return path
Related show

Commit Message

Colin King July 28, 2019, 5:11 p.m. UTC
From: Colin Ian King <colin.king@canonical.com>

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 <colin.king@canonical.com>
---
 drivers/media/platform/vsp1/vsp1_dl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Kieran Bingham July 29, 2019, 12:11 p.m. UTC | #1
Hi Colin,

On 28/07/2019 18:11, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> 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 <colin.king@canonical.com>

Thank you!

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


> ---
>  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);
>  
>
Colin King July 29, 2019, 12:12 p.m. UTC | #2
On 29/07/2019 13:11, Kieran Bingham wrote:
> Hi Colin,
> 
> On 28/07/2019 18:11, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> 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 <colin.king@canonical.com>
> 
> Thank you!

Thank static analysis :-)

> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> 
>> ---
>>  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);
>>  
>>
>
Kieran Bingham July 29, 2019, 12:14 p.m. UTC | #3
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 <colin.king@canonical.com>
>>>
>>> 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 <colin.king@canonical.com>
>>
>> Thank you!
> 
> Thank static analysis :-)

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


> 
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>>
>>> ---
>>>  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);
>>>  
>>>
>>
>

Patch
diff mbox series

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);