Re: [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, Jun 26, 2025 at 10:54:10PM +0530, Abdun Nihaal wrote:
> In the error paths after fb_info structure is successfully allocated,
> the memory allocated in fb_deferred_io_init() for info->pagerefs is not
> freed. Fix that by adding the cleanup function on the error path.

Thanks for the report and the fix! My comments below.

...

>  release_framebuf:
> +	fb_deferred_io_cleanup(info);
>  	framebuffer_release(info);

While the fix sounds good, there are still problems in the driver in this area:

1) managed resources allocation is mixed up with plain allocations
(as you discovery hints);

2) the order in fbtft_framebuffer_release() is asymmetrical to what
we have in fbtft_framebuffer_alloc().

I would recommend to study this code a bit more and provide the following
patches as a result:

1) fixing the order in fbtft_framebuffer_release();

2) moving vmem allocation closer to when it's needed, i.e. just after
successful allocation of the info; at the same time move txbuf allocation
from managed to unmanaged (drop devm, add respective kfree() calls where
it's required);

3) this patch.

All three should have the respective Fixes tags and hence may be backported.


-- 
With Best Regards,
Andy Shevchenko