* [PATCH] fbdev: modedb: keep mode option buffer until parsing completes
@ 2026-06-09 16:00 Ruoyu Wang
2026-06-09 16:07 ` Helge Deller
0 siblings, 1 reply; 2+ messages in thread
From: Ruoyu Wang @ 2026-06-09 16:00 UTC (permalink / raw)
To: Simona Vetter, Helge Deller, Javier Martinez Canillas,
Thomas Zimmermann
Cc: linux-fbdev, dri-devel, linux-kernel, Ruoyu Wang
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.
Fixes: 089d924d03d5 ("fbdev: Read video= option with fb_get_option() in modedb")
Signed-off-by: Ruoyu Wang <ruoyuw560@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;
}
/**
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] fbdev: modedb: keep mode option buffer until parsing completes
2026-06-09 16:00 [PATCH] fbdev: modedb: keep mode option buffer until parsing completes Ruoyu Wang
@ 2026-06-09 16:07 ` Helge Deller
0 siblings, 0 replies; 2+ messages in thread
From: Helge Deller @ 2026-06-09 16:07 UTC (permalink / raw)
To: Ruoyu Wang, Simona Vetter, Javier Martinez Canillas,
Thomas Zimmermann
Cc: linux-fbdev, dri-devel, linux-kernel
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
>
> Fixes: 089d924d03d5 ("fbdev: Read video= option with fb_get_option() in modedb")
> Signed-off-by: Ruoyu Wang <ruoyuw560@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;
> }
>
> /**
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-09 16:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-09 16:00 [PATCH] fbdev: modedb: keep mode option buffer until parsing completes Ruoyu Wang
2026-06-09 16:07 ` Helge Deller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox