[PATCH] mgag200: Fix a memory leak in mgag200fb_create()

[PATCH] mgag200: Fix a memory leak in mgag200fb_create()

[Jesper Juhl]

First we allocate memory for 'sysram' with vmalloc() and subsequently
we allocate for 'info' with framebuffer_alloc(). If the second
allocation fails we return -ENOMEM, but neglect to vfree() the memory
we previously allocated for 'sysram', thus leaking it.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/gpu/drm/mgag200/mgag200_fb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 880d336..3c837e5 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -156,8 +156,10 @@ static int mgag200fb_create(struct mga_fbdev *mfbdev,
 		return -ENOMEM;
 
 	info = framebuffer_alloc(0, device);
-	if (info == NULL)
+	if (info == NULL) {
+		vfree(sysram);
 		return -ENOMEM;
+	}
 
 	info->par = mfbdev;
 
-- 
1.7.11


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

Re: [PATCH] mgag200: Fix a memory leak in mgag200fb_create()

[devendra.aaru]

On Fri, Jun 22, 2012 at 3:43 AM, Jesper Juhl <jj@chaosbits.net> wrote:
> First we allocate memory for 'sysram' with vmalloc() and subsequently
> we allocate for 'info' with framebuffer_alloc(). If the second
> allocation fails we return -ENOMEM, but neglect to vfree() the memory
> we previously allocated for 'sysram', thus leaking it.
>
Hi Jesper,

> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/gpu/drm/mgag200/mgag200_fb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
> index 880d336..3c837e5 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_fb.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
> @@ -156,8 +156,10 @@ static int mgag200fb_create(struct mga_fbdev *mfbdev,
>                return -ENOMEM;
>
>        info = framebuffer_alloc(0, device);
> -       if (info == NULL)
> +       if (info == NULL) {
> +               vfree(sysram);
>                return -ENOMEM;
> +       }
>
>        info->par = mfbdev;
>
This looks ok. but what about the error path?
there are more leaks at error paths. mgag200_framebuffer_init failcase
and the functions below like,
fb_alloc_cmap,
alloc_apertures,
?

> --
> 1.7.11
>
>
> --
> Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] mgag200: Fix a memory leak in mgag200fb_create()

[Jesper Juhl]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1836 bytes --]

On Fri, 22 Jun 2012, devendra.aaru wrote:

> On Fri, Jun 22, 2012 at 3:43 AM, Jesper Juhl <jj@chaosbits.net> wrote:
> > First we allocate memory for 'sysram' with vmalloc() and subsequently
> > we allocate for 'info' with framebuffer_alloc(). If the second
> > allocation fails we return -ENOMEM, but neglect to vfree() the memory
> > we previously allocated for 'sysram', thus leaking it.
> >
> Hi Jesper,
> 
> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > ---
> >  drivers/gpu/drm/mgag200/mgag200_fb.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
> > index 880d336..3c837e5 100644
> > --- a/drivers/gpu/drm/mgag200/mgag200_fb.c
> > +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
> > @@ -156,8 +156,10 @@ static int mgag200fb_create(struct mga_fbdev *mfbdev,
> >                return -ENOMEM;
> >
> >        info = framebuffer_alloc(0, device);
> > -       if (info == NULL)
> > +       if (info == NULL) {
> > +               vfree(sysram);
> >                return -ENOMEM;
> > +       }
> >
> >        info->par = mfbdev;
> >
> This looks ok. but what about the error path?
> there are more leaks at error paths. mgag200_framebuffer_init failcase
> and the functions below like,
> fb_alloc_cmap,
> alloc_apertures,
> ?
> 
I didn't do a complete review. I just happened to notice this one and 
wanted to submit a patch.

If I can find the time I may look more closely for other problems, but no 
promises, so for now I'd suggest just merging this patch and then future 
patches from me, you or someone else, can be added on top of that at a 
later time.

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.