The error handling in fbtft_framebuffer_alloc() mixes managed allocation
and plain allocation, and performs error handling in an order different
from the order in fbtft_framebuffer_release().
Fix them by moving vmem allocation closer to where it is used, and using
plain kzalloc() for txbuf allocation.
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Suggested-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com>
---
v2->v3:
- Remove the if check before kfree of txbuf.buf, because it is zero
initialized on allocation, and kfree is NULL aware.
Newly added in v2
drivers/staging/fbtft/fbtft-core.c | 31 +++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8538b6bab6a5..9e7b84071174 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -568,18 +568,13 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
height = display->height;
}
- vmem_size = display->width * display->height * bpp / 8;
- vmem = vzalloc(vmem_size);
- if (!vmem)
- goto alloc_fail;
-
fbdefio = devm_kzalloc(dev, sizeof(struct fb_deferred_io), GFP_KERNEL);
if (!fbdefio)
- goto alloc_fail;
+ return NULL;
buf = devm_kzalloc(dev, 128, GFP_KERNEL);
if (!buf)
- goto alloc_fail;
+ return NULL;
if (display->gamma_num && display->gamma_len) {
gamma_curves = devm_kcalloc(dev,
@@ -588,12 +583,17 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
sizeof(gamma_curves[0]),
GFP_KERNEL);
if (!gamma_curves)
- goto alloc_fail;
+ return NULL;
}
info = framebuffer_alloc(sizeof(struct fbtft_par), dev);
if (!info)
- goto alloc_fail;
+ return NULL;
+
+ vmem_size = display->width * display->height * bpp / 8;
+ vmem = vzalloc(vmem_size);
+ if (!vmem)
+ goto release_framebuf;
info->screen_buffer = vmem;
info->fbops = &fbtft_ops;
@@ -613,7 +613,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
info->fix.accel = FB_ACCEL_NONE;
info->fix.smem_len = vmem_size;
if (fb_deferred_io_init(info))
- goto release_framebuf;
+ goto release_screen_buffer;
info->var.rotate = pdata->rotate;
info->var.xres = width;
@@ -668,7 +668,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
#endif
if (txbuflen > 0) {
- txbuf = devm_kzalloc(par->info->device, txbuflen, GFP_KERNEL);
+ txbuf = kzalloc(txbuflen, GFP_KERNEL);
if (!txbuf)
goto cleanup_deferred;
par->txbuf.buf = txbuf;
@@ -694,12 +694,10 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
cleanup_deferred:
fb_deferred_io_cleanup(info);
+release_screen_buffer:
+ vfree(info->screen_buffer);
release_framebuf:
framebuffer_release(info);
-
-alloc_fail:
- vfree(vmem);
-
return NULL;
}
EXPORT_SYMBOL(fbtft_framebuffer_alloc);
@@ -712,6 +710,9 @@ EXPORT_SYMBOL(fbtft_framebuffer_alloc);
*/
void fbtft_framebuffer_release(struct fb_info *info)
{
+ struct fbtft_par *par = info->par;
+
+ kfree(par->txbuf.buf);
fb_deferred_io_cleanup(info);
vfree(info->screen_buffer);
framebuffer_release(info);
--
2.43.0On Sun, Jun 29, 2025 at 08:10:11PM +0530, Abdun Nihaal wrote: > The error handling in fbtft_framebuffer_alloc() mixes managed allocation > and plain allocation, and performs error handling in an order different > from the order in fbtft_framebuffer_release(). > > Fix them by moving vmem allocation closer to where it is used, and using > plain kzalloc() for txbuf allocation. > > Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com> > Suggested-by: Dan Carpenter <dan.carpenter@linaro.org> > Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com> > --- > v2->v3: > - Remove the if check before kfree of txbuf.buf, because it is zero > initialized on allocation, and kfree is NULL aware. This patch does not apply to my tree, can you rebase and resend? thanks, greg k-h
Hello Greg,
On Mon, Jun 30, 2025 at 07:16:38PM +0200, Greg KH wrote:
> This patch does not apply to my tree, can you rebase and resend?
I think you have added both the V1 patch and this current V3 patchset to
your tree, that's why this patch does not apply.
Commit eb2cb7dab60f ("staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()")
on staging-testing is an older version of this patchset, and so it has to be dropped.
Regards,
Nihaal
On Tue, Jul 01, 2025 at 12:47:22AM +0530, Abdun Nihaal wrote:
> Hello Greg,
>
> On Mon, Jun 30, 2025 at 07:16:38PM +0200, Greg KH wrote:
> > This patch does not apply to my tree, can you rebase and resend?
>
> I think you have added both the V1 patch and this current V3 patchset to
> your tree, that's why this patch does not apply.
>
> Commit eb2cb7dab60f ("staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()")
> on staging-testing is an older version of this patchset, and so it has to be dropped.
I can't "drop" patches as my tree can not be rebased. Can you send a
fix-up patch instead, OR a revert?
thanks,
greg k-h
On Tue, Jul 1, 2025 at 8:14 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Jul 01, 2025 at 12:47:22AM +0530, Abdun Nihaal wrote:
> > On Mon, Jun 30, 2025 at 07:16:38PM +0200, Greg KH wrote:
> > > This patch does not apply to my tree, can you rebase and resend?
> >
> > I think you have added both the V1 patch and this current V3 patchset to
> > your tree, that's why this patch does not apply.
> >
> > Commit eb2cb7dab60f ("staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()")
> > on staging-testing is an older version of this patchset, and so it has to be dropped.
>
> I can't "drop" patches as my tree can not be rebased. Can you send a
> fix-up patch instead, OR a revert?
I think the cleaner solution will be revert and v3 patches together as
v4. Abdun, can you do that?
--
With Best Regards,
Andy Shevchenko
On Tue, Jul 01, 2025 at 10:03:50AM +0300, Andy Shevchenko wrote:
> On Tue, Jul 1, 2025 at 8:14 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Jul 01, 2025 at 12:47:22AM +0530, Abdun Nihaal wrote:
> > > On Mon, Jun 30, 2025 at 07:16:38PM +0200, Greg KH wrote:
> > > > This patch does not apply to my tree, can you rebase and resend?
> > >
> > > I think you have added both the V1 patch and this current V3 patchset to
> > > your tree, that's why this patch does not apply.
> > >
> > > Commit eb2cb7dab60f ("staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()")
> > > on staging-testing is an older version of this patchset, and so it has to be dropped.
> >
> > I can't "drop" patches as my tree can not be rebased. Can you send a
> > fix-up patch instead, OR a revert?
>
> I think the cleaner solution will be revert and v3 patches together as
> v4. Abdun, can you do that?
>
I'm reading my email in the wrong order today. I thought Abdun came
up with the revert idea on his own instead of you and Greg suggesting
it...
This isn't a case where we revert. The patch we applied was acceptable
quality and it worked fine. Just do the additional cleanup on the top.
regards,
dan carpenter
On Tue, Jul 01, 2025 at 10:03:50AM +0300, Andy Shevchenko wrote: > On Tue, Jul 1, 2025 at 8:14 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > I can't "drop" patches as my tree can not be rebased. Can you send a > > fix-up patch instead, OR a revert? > > I think the cleaner solution will be revert and v3 patches together as > v4. Abdun, can you do that? Sure, I'll send a revert in v4. Regards, Nihaal
© 2016 - 2025 Red Hat, Inc.