[PATCH v3] fbdev: vfb: Validate framebuffer geometry and memory size

[PATCH v3] fbdev: vfb: Validate framebuffer geometry and memory size

[Weiming Shi]

FBIOPUT_VSCREENINFO can supply a virtual width large enough to
overflow get_line_length()'s int multiplication and bypass the
video-memory limit, allowing fbcon to write past the framebuffer. The
offset additions can also wrap, while an excessive videomemorysize
module parameter can be truncated when PAGE_ALIGN() is assigned to
unsigned int.

Cap framebuffer geometry at 8192 x 8192 and video memory at 512 MiB,
keeping the stride, framebuffer-size, and generic fbdev drawing
calculations within their destination types.

KASAN reports:

  BUG: KASAN: vmalloc-out-of-bounds in sys_fillrect
    drivers/video/fbdev/core/sysfillrect.c:24
  Write of size 8 at addr ffa0000001671000 by task poc/198
  sys_fillrect
    drivers/video/fbdev/core/sysfillrect.c:24
  bit_clear_margins
    drivers/video/fbdev/core/bitblit.c:247
  fbcon_clear_margins.constprop.0
    drivers/video/fbdev/core/fbcon.c:1410
  fbcon_switch
    drivers/video/fbdev/core/fbcon.c:2261
  redraw_screen
    drivers/tty/vt/vt.c:994
  fbcon_modechanged
    drivers/video/fbdev/core/fbcon.c:2693
  fbcon_update_vcs
    drivers/video/fbdev/core/fbcon.c:2732

Cc: stable@vger.kernel.org
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: <co+b0df61d49532b2ee@bugs.sh>
Suggested-by: Helge Deller <deller@gmx.de>
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
Changes in v3:
- Clamp visible and virtual resolutions and offsets instead of rejecting
  oversized requests.
- Clamp videomemorysize before PAGE_ALIGN() instead of rejecting it.
- Add unsigned suffixes to the limit constants.

Changes in v2:
- Limit virtual geometry and offsets to 8192 x 8192. This retains 8K mode
  support while bounding all generic fbdev drawing calculations.
- Cap videomemorysize at 512 MiB.
- Keep the existing arithmetic types now that all inputs are bounded.

 drivers/video/fbdev/vfb.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/vfb.c b/drivers/video/fbdev/vfb.c
index de137b2bdaed..deaa3e9c758d 100644
--- a/drivers/video/fbdev/vfb.c
+++ b/drivers/video/fbdev/vfb.c
@@ -31,6 +31,9 @@
      */
 
 #define VIDEOMEMSIZE	(1*1024*1024)	/* 1 MB */
+#define MAX_VIDEOMEMSIZE	(512UL * 1024 * 1024)
+#define MAX_XRES		8192U
+#define MAX_YRES		8192U
 
 static void *videomemory;
 static u_long videomemorysize = VIDEOMEMSIZE;
@@ -142,6 +145,10 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
 		var->xres = 1;
 	if (!var->yres)
 		var->yres = 1;
+	var->xres = min(var->xres, MAX_XRES);
+	var->yres = min(var->yres, MAX_YRES);
+	var->xres_virtual = min(var->xres_virtual, MAX_XRES);
+	var->yres_virtual = min(var->yres_virtual, MAX_YRES);
 	if (var->xres > var->xres_virtual)
 		var->xres_virtual = var->xres;
 	if (var->yres > var->yres_virtual)
@@ -159,6 +166,9 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
 	else
 		return -EINVAL;
 
+	var->xoffset = min(var->xoffset, MAX_XRES - var->xres);
+	var->yoffset = min(var->yoffset, MAX_YRES - var->yres);
+
 	if (var->xres_virtual < var->xoffset + var->xres)
 		var->xres_virtual = var->xoffset + var->xres;
 	if (var->yres_virtual < var->yoffset + var->yres)
@@ -434,9 +444,12 @@ static int __init vfb_setup(char *options)
 static int vfb_probe(struct platform_device *dev)
 {
 	struct fb_info *info;
-	unsigned int size = PAGE_ALIGN(videomemorysize);
+	unsigned int size;
 	int retval = -ENOMEM;
 
+	videomemorysize = min(videomemorysize, MAX_VIDEOMEMSIZE);
+	size = PAGE_ALIGN(videomemorysize);
+
 	/*
 	 * For real video cards we use ioremap.
 	 */
-- 
2.56.0

Re: [PATCH v3] fbdev: vfb: Validate framebuffer geometry and memory size

[Helge Deller]

On 10/27/26 17:09, Weiming Shi wrote:
> FBIOPUT_VSCREENINFO can supply a virtual width large enough to
> overflow get_line_length()'s int multiplication and bypass the
> video-memory limit, allowing fbcon to write past the framebuffer. The
> offset additions can also wrap, while an excessive videomemorysize
> module parameter can be truncated when PAGE_ALIGN() is assigned to
> unsigned int.
> 
> Cap framebuffer geometry at 8192 x 8192 and video memory at 512 MiB,
> keeping the stride, framebuffer-size, and generic fbdev drawing
> calculations within their destination types.
> 
> KASAN reports:
> 
>    BUG: KASAN: vmalloc-out-of-bounds in sys_fillrect
>      drivers/video/fbdev/core/sysfillrect.c:24
>    Write of size 8 at addr ffa0000001671000 by task poc/198
>    sys_fillrect
>      drivers/video/fbdev/core/sysfillrect.c:24
>    bit_clear_margins
>      drivers/video/fbdev/core/bitblit.c:247
>    fbcon_clear_margins.constprop.0
>      drivers/video/fbdev/core/fbcon.c:1410
>    fbcon_switch
>      drivers/video/fbdev/core/fbcon.c:2261
>    redraw_screen
>      drivers/tty/vt/vt.c:994
>    fbcon_modechanged
>      drivers/video/fbdev/core/fbcon.c:2693
>    fbcon_update_vcs
>      drivers/video/fbdev/core/fbcon.c:2732
> 
> Cc: stable@vger.kernel.org
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: <co+b0df61d49532b2ee@bugs.sh>
> Suggested-by: Helge Deller <deller@gmx.de>
> Assisted-by: LLM
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> Changes in v3:
> - Clamp visible and virtual resolutions and offsets instead of rejecting
>    oversized requests.
> - Clamp videomemorysize before PAGE_ALIGN() instead of rejecting it.
> - Add unsigned suffixes to the limit constants.
> 
> Changes in v2:
> - Limit virtual geometry and offsets to 8192 x 8192. This retains 8K mode
>    support while bounding all generic fbdev drawing calculations.
> - Cap videomemorysize at 512 MiB.
> - Keep the existing arithmetic types now that all inputs are bounded.
> 
>   drivers/video/fbdev/vfb.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
applied.

Thanks!
Helge

Re: [PATCH v3] fbdev: vfb: Validate framebuffer geometry and memory size

[Geert Uytterhoeven]

Hi Weiming,

On Tue, 27 Oct 2026 at 17:16, Weiming Shi <bestswngs@gmail.com> wrote:
> FBIOPUT_VSCREENINFO can supply a virtual width large enough to
> overflow get_line_length()'s int multiplication and bypass the
> video-memory limit, allowing fbcon to write past the framebuffer. The
> offset additions can also wrap, while an excessive videomemorysize
> module parameter can be truncated when PAGE_ALIGN() is assigned to
> unsigned int.
>
> Cap framebuffer geometry at 8192 x 8192 and video memory at 512 MiB,
> keeping the stride, framebuffer-size, and generic fbdev drawing
> calculations within their destination types.
>
> KASAN reports:
>
>   BUG: KASAN: vmalloc-out-of-bounds in sys_fillrect
>     drivers/video/fbdev/core/sysfillrect.c:24
>   Write of size 8 at addr ffa0000001671000 by task poc/198
>   sys_fillrect
>     drivers/video/fbdev/core/sysfillrect.c:24
>   bit_clear_margins
>     drivers/video/fbdev/core/bitblit.c:247
>   fbcon_clear_margins.constprop.0
>     drivers/video/fbdev/core/fbcon.c:1410
>   fbcon_switch
>     drivers/video/fbdev/core/fbcon.c:2261
>   redraw_screen
>     drivers/tty/vt/vt.c:994
>   fbcon_modechanged
>     drivers/video/fbdev/core/fbcon.c:2693
>   fbcon_update_vcs
>     drivers/video/fbdev/core/fbcon.c:2732
>
> Cc: stable@vger.kernel.org
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: <co+b0df61d49532b2ee@bugs.sh>
> Suggested-by: Helge Deller <deller@gmx.de>
> Assisted-by: LLM
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>

Thanks for your patch, which is now commit f93dd31db3e23e61 ("fbdev:
vfb: Validate framebuffer geometry and memory size") in fbdev/for-next.

> --- a/drivers/video/fbdev/vfb.c
> +++ b/drivers/video/fbdev/vfb.c
> @@ -31,6 +31,9 @@
>       */
>
>  #define VIDEOMEMSIZE   (1*1024*1024)   /* 1 MB */
> +#define MAX_VIDEOMEMSIZE       (512UL * 1024 * 1024)

Is there any point in having a larger limit than 256 MiB
(i.e. MAX_XRES * MAX_YRES * 32 bpp)?

> +#define MAX_XRES               8192U
> +#define MAX_YRES               8192U
>
>  static void *videomemory;
>  static u_long videomemorysize = VIDEOMEMSIZE;
> @@ -142,6 +145,10 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
>                 var->xres = 1;
>         if (!var->yres)
>                 var->yres = 1;
> +       var->xres = min(var->xres, MAX_XRES);
> +       var->yres = min(var->yres, MAX_YRES);
> +       var->xres_virtual = min(var->xres_virtual, MAX_XRES);
> +       var->yres_virtual = min(var->yres_virtual, MAX_YRES);

This violates fbdev API rounding rules:
  1. If a value doesn't fit, it must be rounded up by the driver,
  2. If a value is too large, -EINVAL must be returned.

>         if (var->xres > var->xres_virtual)
>                 var->xres_virtual = var->xres;
>         if (var->yres > var->yres_virtual)
> @@ -159,6 +166,9 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
>         else
>                 return -EINVAL;
>
> +       var->xoffset = min(var->xoffset, MAX_XRES - var->xres);
> +       var->yoffset = min(var->yoffset, MAX_YRES - var->yres);

Likewise.

> +
>         if (var->xres_virtual < var->xoffset + var->xres)
>                 var->xres_virtual = var->xoffset + var->xres;
>         if (var->yres_virtual < var->yoffset + var->yres)
> @@ -434,9 +444,12 @@ static int __init vfb_setup(char *options)
>  static int vfb_probe(struct platform_device *dev)
>  {
>         struct fb_info *info;
> -       unsigned int size = PAGE_ALIGN(videomemorysize);
> +       unsigned int size;
>         int retval = -ENOMEM;
>
> +       videomemorysize = min(videomemorysize, MAX_VIDEOMEMSIZE);
> +       size = PAGE_ALIGN(videomemorysize);

Perhaps return -EINVAL, too?

> +
>         /*
>          * For real video cards we use ioremap.
>          */

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 v3] fbdev: vfb: Validate framebuffer geometry and memory size

[Helge Deller]

On 10/29/26 09:09, Geert Uytterhoeven wrote:
> Hi Weiming,
> 
> On Tue, 27 Oct 2026 at 17:16, Weiming Shi <bestswngs@gmail.com> wrote:
>> FBIOPUT_VSCREENINFO can supply a virtual width large enough to
>> overflow get_line_length()'s int multiplication and bypass the
>> video-memory limit, allowing fbcon to write past the framebuffer. The
>> offset additions can also wrap, while an excessive videomemorysize
>> module parameter can be truncated when PAGE_ALIGN() is assigned to
>> unsigned int.
>>
>> Cap framebuffer geometry at 8192 x 8192 and video memory at 512 MiB,
>> keeping the stride, framebuffer-size, and generic fbdev drawing
>> calculations within their destination types.
>>
>> KASAN reports:
>>
>>    BUG: KASAN: vmalloc-out-of-bounds in sys_fillrect
>>      drivers/video/fbdev/core/sysfillrect.c:24
>>    Write of size 8 at addr ffa0000001671000 by task poc/198
>>    sys_fillrect
>>      drivers/video/fbdev/core/sysfillrect.c:24
>>    bit_clear_margins
>>      drivers/video/fbdev/core/bitblit.c:247
>>    fbcon_clear_margins.constprop.0
>>      drivers/video/fbdev/core/fbcon.c:1410
>>    fbcon_switch
>>      drivers/video/fbdev/core/fbcon.c:2261
>>    redraw_screen
>>      drivers/tty/vt/vt.c:994
>>    fbcon_modechanged
>>      drivers/video/fbdev/core/fbcon.c:2693
>>    fbcon_update_vcs
>>      drivers/video/fbdev/core/fbcon.c:2732
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Reported-by: <co+b0df61d49532b2ee@bugs.sh>
>> Suggested-by: Helge Deller <deller@gmx.de>
>> Assisted-by: LLM
>> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> 
> Thanks for your patch, which is now commit f93dd31db3e23e61 ("fbdev:
> vfb: Validate framebuffer geometry and memory size") in fbdev/for-next.
> 
>> --- a/drivers/video/fbdev/vfb.c
>> +++ b/drivers/video/fbdev/vfb.c
>> @@ -31,6 +31,9 @@
>>        */
>>
>>   #define VIDEOMEMSIZE   (1*1024*1024)   /* 1 MB */
>> +#define MAX_VIDEOMEMSIZE       (512UL * 1024 * 1024)
> 
> Is there any point in having a larger limit than 256 MiB
> (i.e. MAX_XRES * MAX_YRES * 32 bpp)?
> 
>> +#define MAX_XRES               8192U
>> +#define MAX_YRES               8192U
>>
>>   static void *videomemory;
>>   static u_long videomemorysize = VIDEOMEMSIZE;
>> @@ -142,6 +145,10 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
>>                  var->xres = 1;
>>          if (!var->yres)
>>                  var->yres = 1;
>> +       var->xres = min(var->xres, MAX_XRES);
>> +       var->yres = min(var->yres, MAX_YRES);
>> +       var->xres_virtual = min(var->xres_virtual, MAX_XRES);
>> +       var->yres_virtual = min(var->yres_virtual, MAX_YRES);
> 
> This violates fbdev API rounding rules:
>    1. If a value doesn't fit, it must be rounded up by the driver,
>    2. If a value is too large, -EINVAL must be returned.

Thanks for your review, Geert!
You are right. It's still documented like that in skeletonfb.c
Weiming changed it based on my feedback.
Weiming, no need to resend the patch.
I'll pick up one of your former patches.

Thanks!
Helge

> 
>>          if (var->xres > var->xres_virtual)
>>                  var->xres_virtual = var->xres;
>>          if (var->yres > var->yres_virtual)
>> @@ -159,6 +166,9 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
>>          else
>>                  return -EINVAL;
>>
>> +       var->xoffset = min(var->xoffset, MAX_XRES - var->xres);
>> +       var->yoffset = min(var->yoffset, MAX_YRES - var->yres);
> 
> Likewise.
> 
>> +
>>          if (var->xres_virtual < var->xoffset + var->xres)
>>                  var->xres_virtual = var->xoffset + var->xres;
>>          if (var->yres_virtual < var->yoffset + var->yres)
>> @@ -434,9 +444,12 @@ static int __init vfb_setup(char *options)
>>   static int vfb_probe(struct platform_device *dev)
>>   {
>>          struct fb_info *info;
>> -       unsigned int size = PAGE_ALIGN(videomemorysize);
>> +       unsigned int size;
>>          int retval = -ENOMEM;
>>
>> +       videomemorysize = min(videomemorysize, MAX_VIDEOMEMSIZE);
>> +       size = PAGE_ALIGN(videomemorysize);
> 
> Perhaps return -EINVAL, too?
> 
>> +
>>          /*
>>           * For real video cards we use ioremap.
>>           */
> 
> 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