Re: [PATCH] fbdev:modedb: fix a possible UAF in fb_find_mode()
From: Tuo Li <hidden>
Date: 2026-06-10 02:21:53
Also in:
dri-devel, lkml
Hi Helge, On Mon, Jun 8, 2026 at 12:12 AM Helge Deller [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On 5/26/26 11:15, Tuo Li wrote:quoted
If mode_option is NULL, it is assigned from mode_option_buf: if (!mode_option) { fb_get_options(NULL, &mode_option_buf); mode_option = mode_option_buf; } Later, name is assigned from mode_option: const char *name = mode_option; However, mode_option_buf is freed before name is no longer used: kfree(mode_option_buf); while name is still accessed by: if ((name_matches(db[i], name, namelen) || Since name aliases mode_option_buf, this may result in a use-after-free. Fix this by moving the kfree(mode_option_buf) call behind the access, and add corresponding cleanup before early returns.I wonder if this isn't a typical good use-case for the new "Scope-based resource management for the kernel" [1] feature. Instead of adding kfree() at various places, we could do:diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c index 703d0b7aec32..b6926764a99c 100644 --- a/drivers/video/fbdev/core/modedb.c +++ b/drivers/video/fbdev/core/modedb.c@@ -626,7 +626,7 @@ int fb_find_mode(struct fb_var_screeninfo *var, const struct fb_videomode *default_mode, unsigned int default_bpp) { - char *mode_option_buf = NULL; + char *mode_option_buf __free(kfree) = NULL; int i; /* Set up defaults */@@ -724,7 +724,6 @@ int fb_find_mode(struct fb_var_screeninfo *var, res_specified = 1; } done: - kfree(mode_option_buf); if (cvt) { struct fb_videomode cvt_mode; int ret;[1] https://lwn.net/Articles/934679/ Do you want to check if that's correct, and if yes resend a patch? Helge
Thanks for the suggestion. I think that makes sense. Using __free(kfree) can simplify the cleanup logic and avoid manually managing multiple kfree() calls on different return paths. I'll prepare a v2 based on this approach and send it soon. Sincerely, Tuo