Re: [PATCH] fbdev: modedb: keep mode option buffer until parsing completes
From: Helge Deller <deller@gmx.de>
Date: 2026-06-09 16:07:57
Also in:
dri-devel, lkml
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-islituo@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
quoted hunk ↗ jump to hunk
Fixes: 089d924d03d5 ("fbdev: Read video= option with fb_get_option() in modedb") Signed-off-by: Ruoyu Wang <redacted> --- 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; } /**