Re: [PATCH] staging: sm750fb: free cmap on framebuffer release

[Dan Carpenter]

On Tue, Sep 22, 2026 at 12:53:02AM -0300, Lucas Costa wrote:
> The driver never calls fb_dealloc_cmap() after it has been allocated,
> so when the buffer is torn down we leak.
> 
> This happens on every unbind, and when an error is detected in
> sm750fb_framebuffer_alloc().
> 
> fb_dealloc_cmap() gets called before framebuffer_release() in both places.
> 
> Fixes: 81dee67e215b ("staging: sm750fb: add sm750 to staging")
> 
> Signed-off-by: Lucas Costa <2000.costalucas@xxxxxxxxx>
> ---
>  drivers/staging/sm750fb/sm750.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 8b93bfeb217b..92412534239c 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -910,6 +910,7 @@ static void sm750fb_framebuffer_release(struct sm750_dev *sm750_dev)
>  	while (sm750_dev->fb_count) {
>  		fb_info = sm750_dev->fbinfo[sm750_dev->fb_count - 1];
>  		unregister_framebuffer(fb_info);
> +		fb_dealloc_cmap(&fb_info->cmap);
>  		framebuffer_release(fb_info);
>  		sm750_dev->fb_count--;
>  	}
> @@ -943,6 +944,7 @@ static int sm750fb_framebuffer_alloc(struct sm750_dev *sm750_dev, int fbidx)
>  	return 0;
>  
>  release_fb:
> +	fb_dealloc_cmap(&fb_info->cmap);
>  	framebuffer_release(fb_info);
>  	return err;

This works, but I don't love One Err label style cleanup.  The label
name is no longer accurate.

The cmap is allocated by:

	err = lynxfb_set_fbinfo(fb_info, fbidx);

So ideally there would be a cleanup function which is called something
like:

	lynxfb_unset_fbinfo(fb_info);

Then we would only call it if lynxfb_set_fbinfo() succeeds instead of
if it succeeds or fails.

unset_fb:
	lynxfb_unset_fbinfo(fb_info);
release_fb:
	framebuffer_release(fb_info);

	return err;

See my blog for more details.
staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/

regards,
dan carpenter