On 10/29/26 09:09, Geert Uytterhoeven wrote:
Hi Weiming,
On Tue, 27 Oct 2026 at 17:16, Weiming Shi <[email protected]> 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: [email protected]
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: <[email protected]>
Suggested-by: Helge Deller <[email protected]>
Assisted-by: LLM
Signed-off-by: Weiming Shi <[email protected]>
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 -- [email protected]
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
