[PATCH v2] fbdev:modedb: fix a possible UAF in fb_find_mode()

[PATCH v2] fbdev:modedb: fix a possible UAF in fb_find_mode()

[Tuo Li]

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 extending the lifetime of mode_option_buf until the end of the 
function and using scope-based resource management for cleanup.

Signed-off-by: Tuo Li <islituo@gmail.com>
---
v2:
* Use scope-based resource management instead of manual kfree() calls.
  Thanks to Helge Deller for the helpful advice.
---
 drivers/video/fbdev/core/modedb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

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;
-- 
2.43.0

Re: [PATCH v2] fbdev:modedb: fix a possible UAF in fb_find_mode()

[Geert Uytterhoeven]

Hi Tuo, Helge,

On Wed, 10 Jun 2026 at 04:50, Tuo Li <islituo@gmail.com> 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 extending the lifetime of mode_option_buf until the end of the
> function and using scope-based resource management for cleanup.
>
> Signed-off-by: Tuo Li <islituo@gmail.com>
> ---
> v2:
> * Use scope-based resource management instead of manual kfree() calls.
>   Thanks to Helge Deller for the helpful advice.

Thanks for your patch, which is now commit 85b6256469cebdac ("fbdev:
modedb: fix a possible UAF in fb_find_mode()") in fbdev/for-next, and has:

    Cc: stable@vger.kernel.org # v6.5+

I believe it needs:
Fixes: 089d924d03d5c17b ("fbdev: Read video= option with
fb_get_option() in modedb")

and that commit entered v6.4-rc1, i.e. not v6.5?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] fbdev:modedb: fix a possible UAF in fb_find_mode()

[Helge Deller]

On 6/11/26 10:10, Geert Uytterhoeven wrote:
> Hi Tuo, Helge,
> 
> On Wed, 10 Jun 2026 at 04:50, Tuo Li <islituo@gmail.com> 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 extending the lifetime of mode_option_buf until the end of the
>> function and using scope-based resource management for cleanup.
>>
>> Signed-off-by: Tuo Li <islituo@gmail.com>
>> ---
>> v2:
>> * Use scope-based resource management instead of manual kfree() calls.
>>    Thanks to Helge Deller for the helpful advice.
> 
> Thanks for your patch, which is now commit 85b6256469cebdac ("fbdev:
> modedb: fix a possible UAF in fb_find_mode()") in fbdev/for-next, and has:
> 
>      Cc: stable@vger.kernel.org # v6.5+
> 
> I believe it needs:
> Fixes: 089d924d03d5c17b ("fbdev: Read video= option with
> fb_get_option() in modedb")
> 
> and that commit entered v6.4-rc1, i.e. not v6.5?

Right, but I added the v6.5+ tag, because this patch uses the "__cleanup() based infrastructure",
which I think was introduced with v6.5.

Helge