linux-kernel - Re: [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aF2Ic8BP0zWS6R19@smile.fi.intel.com>
Date: Thu, 26 Jun 2025 20:50:43 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Abdun Nihaal <abdun.nihaal@...il.com>
Cc: andy@...nel.org, gregkh@...uxfoundation.org, lorenzo.stoakes@...cle.com,
	tzimmermann@...e.de, riyandhiman14@...il.com, willy@...radead.org,
	notro@...nnes.org, thomas.petazzoni@...e-electrons.com,
	dri-devel@...ts.freedesktop.org, linux-fbdev@...r.kernel.org,
	linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: fbtft: fix potential memory leak in
 fbtft_framebuffer_alloc()

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