| Message ID | 20190728171124.14202-1-colin.king@canonical.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: vsp1: fix memory leak of dl on error return path | expand |
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); > >
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); >> >> >
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); >>> >>> >> >
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);