Re: [PATCH] fbdev: modedb: keep mode option buffer until parsing completes

[Helge Deller]

On 6/9/26 18:00, Ruoyu Wang wrote:

When fb_find_mode() obtains the mode option from fb_get_options(),
mode_option_buf owns the returned string and name points into that
buffer. The done label frees mode_option_buf before the database
fallback has finished using name in name_matches(), so the fallback can
read freed memory.

Move the free to a common exit path and convert the successful returns
that can use mode_option_buf into jumps to that exit path.

There was another similiar patch already posted:
https://patchwork.kernel.org/project/linux-fbdev/patch/20260526091507.421730-1-isli...@gmail.com/

Do you want to check if the Scope-based kfree can be used here,
as suggested by me in that thread? It's at least much smaller than your patch...

Helge

Fixes: 089d924d03d5 ("fbdev: Read video= option with fb_get_option() in modedb")
Signed-off-by: Ruoyu Wang <ruoyuw...@gmail.com>
---
  drivers/video/fbdev/core/modedb.c | 41 ++++++++++++++++++++-----------
  1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/video/fbdev/core/modedb.c 
b/drivers/video/fbdev/core/modedb.c
index 703d0b7aec322..82f6ea38e1fb8 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -627,7 +627,7 @@ int fb_find_mode(struct fb_var_screeninfo *var,
                 unsigned int default_bpp)
  {
        char *mode_option_buf = NULL;
-       int i;
+       int i, ret;

/* Set up defaults */

        if (!db) {
@@ -724,10 +724,9 @@ 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;
+                       int cvt_ret;

DPRINTK("CVT mode %dx%d@%dHz%s%s%s\n", xres, yres,

                                (refresh) ? refresh : 60,
@@ -745,11 +744,12 @@ int fb_find_mode(struct fb_var_screeninfo *var,
                        else
                                cvt_mode.vmode &= ~FB_VMODE_INTERLACED;

- ret = fb_find_mode_cvt(&cvt_mode, margins, rb);

+                       cvt_ret = fb_find_mode_cvt(&cvt_mode, margins, rb);

- if (!ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {

+                       if (!cvt_ret && !fb_try_mode(var, info, &cvt_mode, 
bpp)) {
                                DPRINTK("modedb CVT: CVT mode ok\n");
-                               return 1;
+                               ret = 1;
+                               goto out;
                        }

DPRINTK("CVT mode invalid, getting mode from database\n");

@@ -793,8 +793,10 @@ int fb_find_mode(struct fb_var_screeninfo *var,
                                if (!interlace_specified ||
                                    db_interlace == interlace)
                                        if (refresh_specified &&
-                                           db[i].refresh == refresh)
-                                               return 1;
+                                           db[i].refresh == refresh) {
+                                               ret = 1;
+                                               goto out;
+                                       }

if (score < diff) {

                                        diff = score;
@@ -804,7 +806,8 @@ int fb_find_mode(struct fb_var_screeninfo *var,
                }
                if (best != -1) {
                        fb_try_mode(var, info, &db[best], bpp);
-                       return (refresh_specified) ? 2 : 1;
+                       ret = (refresh_specified) ? 2 : 1;
+                       goto out;
                }

diff = 2 * (xres + yres);

@@ -831,21 +834,29 @@ int fb_find_mode(struct fb_var_screeninfo *var,
                }
                if (best != -1) {
                        fb_try_mode(var, info, &db[best], bpp);
-                       return 5;
+                       ret = 5;
+                       goto out;
                }
        }

DPRINTK("Trying default video mode\n");

-       if (!fb_try_mode(var, info, default_mode, default_bpp))
-               return 3;
+       if (!fb_try_mode(var, info, default_mode, default_bpp)) {
+               ret = 3;
+               goto out;
+       }

DPRINTK("Trying all modes\n");

        for (i = 0; i < dbsize; i++)
-               if (!fb_try_mode(var, info, &db[i], default_bpp))
-                       return 4;
+               if (!fb_try_mode(var, info, &db[i], default_bpp)) {
+                       ret = 4;
+                       goto out;
+               }

DPRINTK("No valid mode found\n");

-       return 0;
+       ret = 0;
+out:
+       kfree(mode_option_buf);
+       return ret;
  }

/**