Thread (3 messages) 3 messages, 2 authors, 12d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help