* Re: [PATCH] video: fbdev: remove skeletonfb example driver with no remaining purpose
From: Helge Deller @ 2026-06-09 14:19 UTC (permalink / raw)
To: Geert Uytterhoeven, Ethan Nelson-Moore; +Cc: linux-fbdev
In-Reply-To: <CAMuHMdXiToNLeQqW54+tOm6-eh9Xefxe3QaMC2Zg7r-3pBOx8A@mail.gmail.com>
On 6/8/26 10:03, Geert Uytterhoeven wrote:
> On Sun, 7 Jun 2026 at 03:58, Ethan Nelson-Moore <enelsonmoore@gmail.com> wrote:
>> The skeletonfb driver is intended to serve as an example for writing
>> new framebuffer drivers. However, new framebuffer drivers are no longer
>> accepted into the kernel because DRM has obsoleted fbdev, so it no
>> longer has a purpose. In spite of this, it continues to be updated to
>> reflect fbdev API changes, wasting maintainers' time.
I don't want to argue, but the last update was 3 years ago, so
I don't see that much wasted time.
> Remove it.
>>
>> Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
>
> Thanks for your patch!
>
> Makes sense, as we still have vfb.c, which can actually be built.
> Perhaps some of the comments and/or kerneldoc should be moved
> elsewhere, so it is preserved?
That's what I'm mostly concerned about.
It has so much (still) useful info, that it would be sad to loose that.
Looking up the info in an already deleted file isn't a good alternative.
So, either someone should move comments/info over, or I'd like to keep it
for some more time as it's not actually hurting anybody.
Helge
^ permalink raw reply
* [PATCH] fbdev: modedb: keep mode option buffer until parsing completes
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
* Re: [PATCH] fbdev: modedb: keep mode option buffer until parsing completes
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
In-Reply-To: <20260609160028.5-1-ruoyuw560@gmail.com>
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
page: | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox