On 10/26/26 16:46, 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.
Limit virtual geometry and offsets to 8192 x 8192 and cap 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]>
---
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 | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vfb.c b/drivers/video/fbdev/vfb.c
index de137b2bd..de40ba8c3 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 (512 * 1024 * 1024)
+#define MAX_XRES 8192
+#define MAX_YRES 8192
static void *videomemory;
static u_long videomemorysize = VIDEOMEMSIZE;
@@ -159,6 +162,14 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
else
return -EINVAL;
+ if (var->xres_virtual > MAX_XRES ||
+ var->yres_virtual > MAX_YRES)
+ return -EINVAL;
+
+ if (var->xoffset > MAX_XRES - var->xres ||
+ var->yoffset > MAX_YRES - var->yres)
+ return -EINVAL;
Instead of returning EINVAL, I wonder if we could simply
cap the resolutions at MAX_XRES/MAX_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 +445,13 @@ 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;
and here maybe
videomemorysize = min(videomemorysize, MAX_VIDEOMEMSIZE);
that way the driver loads even if the provided too big value?
Helge
+ if (videomemorysize > MAX_VIDEOMEMSIZE)
+ return -EINVAL;
+ size = PAGE_ALIGN(videomemorysize);
