| Message ID | 20170912053930.18931-1-christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
[ added dri-devel ML to cc: ] On Tuesday, September 12, 2017 07:39:30 AM Christophe JAILLET wrote: > If 'dmam_alloc_attrs()' fails, we must go through the error handling code, > as done elsewhere in this function. Otherwise, there is a resource leak. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > I'm also puzzled by the 'framebuffer_alloc()' call a few lines above. > 'ret' is known to be 0 at this point. I guess that -ENOMEM should also be > returned. Yes, moreover the "failed:" error path is incomplete (please take a look at au1200fb_drv_remove() for comparison) and needs to be fixed. Could you please take care of it? > --- > drivers/video/fbdev/au1200fb.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c > index 5f04b4096c42..99d6cfb168b5 100644 > --- a/drivers/video/fbdev/au1200fb.c > +++ b/drivers/video/fbdev/au1200fb.c > @@ -1701,7 +1701,8 @@ static int au1200fb_drv_probe(struct platform_device *dev) > if (!fbdev->fb_mem) { > print_err("fail to allocate frambuffer (size: %dK))", > fbdev->fb_len / 1024); > - return -ENOMEM; > + ret = -ENOMEM; > + goto failed; > } > > /* Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 12/10/2017 à 18:25, Bartlomiej Zolnierkiewicz a écrit : > [ added dri-devel ML to cc: ] > > On Tuesday, September 12, 2017 07:39:30 AM Christophe JAILLET wrote: >> If 'dmam_alloc_attrs()' fails, we must go through the error handling code, >> as done elsewhere in this function. Otherwise, there is a resource leak. >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> I'm also puzzled by the 'framebuffer_alloc()' call a few lines above. >> 'ret' is known to be 0 at this point. I guess that -ENOMEM should also be >> returned. > Yes, moreover the "failed:" error path is incomplete (please take > a look at au1200fb_drv_remove() for comparison) and needs to be fixed. > > Could you please take care of it? I will try, but be indulgent :) My understanding of the code is that they are several issues all related to this error handling path (double free, missing memory release, invalid irq release...) I will try to sort it out, but my first tries will likely be incomplete and/or broken. CJ -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c index 5f04b4096c42..99d6cfb168b5 100644 --- a/drivers/video/fbdev/au1200fb.c +++ b/drivers/video/fbdev/au1200fb.c @@ -1701,7 +1701,8 @@ static int au1200fb_drv_probe(struct platform_device *dev) if (!fbdev->fb_mem) { print_err("fail to allocate frambuffer (size: %dK))", fbdev->fb_len / 1024); - return -ENOMEM; + ret = -ENOMEM; + goto failed; } /*
If 'dmam_alloc_attrs()' fails, we must go through the error handling code, as done elsewhere in this function. Otherwise, there is a resource leak. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- I'm also puzzled by the 'framebuffer_alloc()' call a few lines above. 'ret' is known to be 0 at this point. I guess that -ENOMEM should also be returned. --- drivers/video/fbdev/au1200fb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)