On 10/23/26 19:51, Weiming Shi wrote:
get_line_length() multiplies two int operands before assigning the
result to u_long. Setting xres_virtual to 0x8000000 at 32 bpp therefore
A xres_virtual value of 134217728 seems quite unrealistic IMHO...
wraps the product to zero and bypasses vfb_check_var()'s video-memory
limit. fbcon can then clear margins with a zero line stride and write
past the vmalloc'ed framebuffer.
The xoffset + xres and yoffset + yres calculations can likewise wrap,
leaving an invalid offset in info->var after fb_pan_display() rejects
it. Subsequent fbcon drawing can then access far beyond the framebuffer.
Finally, PAGE_ALIGN(videomemorysize) was narrowed from unsigned long to
unsigned int. Values near or above 4 GiB could therefore allocate a much
smaller buffer than the size used for mode validation.
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
Reject video-memory sizes whose aligned value is not representable by
the fbdev metadata. Check the offset additions for overflow. Compute the
line length and framebuffer size in u64, reject unrepresentable line
lengths, and check the total-size multiplication before comparing it
with the allocated video-memory size.
Cc: [email protected]
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: <[email protected]>
Assisted-by: LLM
Signed-off-by: Weiming Shi <[email protected]>
---
drivers/video/fbdev/vfb.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/video/fbdev/vfb.c b/drivers/video/fbdev/vfb.c
index de137b2bd..84b0a937c 100644
--- a/drivers/video/fbdev/vfb.c
+++ b/drivers/video/fbdev/vfb.c
@@ -101,11 +101,11 @@ static const struct fb_ops vfb_ops = {
* Internal routines
*/
-static u_long get_line_length(int xres_virtual, int bpp)
+static u64 get_line_length(u32 xres_virtual, u32 bpp)
Instead of trying to extend to 64bit, I think we should simply
limit the maximum allowed screen sizes to e.g. 4096 x 4096
(or some other realistic values).
{
- u_long length;
+ u64 length;
- length = xres_virtual * bpp;
+ length = (u64)xres_virtual * bpp;
length = (length + 31) & ~31;
length >>= 3;
return (length);
@@ -122,7 +122,8 @@ static u_long get_line_length(int xres_virtual, int bpp)
static int vfb_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
{
- u_long line_length;
+ u64 line_length, size;
+ u32 xres, yres;
/*
* FB_VMODE_CONUPDATE and FB_VMODE_SMOOTH_XPAN are equal!
@@ -159,17 +160,23 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
else
return -EINVAL;
- if (var->xres_virtual < var->xoffset + var->xres)
- var->xres_virtual = var->xoffset + var->xres;
- if (var->yres_virtual < var->yoffset + var->yres)
- var->yres_virtual = var->yoffset + var->yres;
+ if (check_add_overflow(var->xoffset, var->xres, &xres) ||
+ check_add_overflow(var->yoffset, var->yres, &yres))
+ return -EINVAL;
+
+ if (var->xres_virtual < xres)
+ var->xres_virtual = xres;
+ if (var->yres_virtual < yres)
+ var->yres_virtual = yres;
/*
* Memory limit
*/
line_length =
get_line_length(var->xres_virtual, var->bits_per_pixel);
- if (line_length * var->yres_virtual > videomemorysize)
+ if (line_length > U32_MAX ||
+ check_mul_overflow(line_length, (u64)var->yres_virtual, &size) ||
+ size > videomemorysize)
return -ENOMEM;
/*
@@ -434,9 +441,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;
+ if (videomemorysize > U32_MAX - (PAGE_SIZE - 1))
+ return -EINVAL;
+ size = PAGE_ALIGN(videomemorysize);
same here.
There doesn't seem to be a maximum limit for videomemorysize.
Maybe adding a check and allow a maximum of e.g. 512MB ?
Will you send a revised patch?
Helge
