Hi Helge, On Mon, Jun 8, 2026 at 12:12 AM Helge Deller <[email protected]> wrote: > > On 5/26/26 11:15, Tuo Li wrote: > > 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
