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

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

[Weiming Shi]

get_line_length() multiplies two int operands before assigning the
result to u_long. Setting xres_virtual to 0x8000000 at 32 bpp therefore
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: stable@vger.kernel.org
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: <co+b0df61d49532b2ee@bugs.sh>
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 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)
 {
-	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);
+
 	/*
 	 * For real video cards we use ioremap.
 	 */
-- 
2.56.0

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

[Helge Deller]

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: stable@vger.kernel.org
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: <co+b0df61d49532b2ee@bugs.sh>
> Assisted-by: LLM
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>   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

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

[Weiming Shi]

Helge Deller <deller@gmx.de> 于2026年10月25日周日 12:38写道：
>
> 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: stable@vger.kernel.org
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: <co+b0df61d49532b2ee@bugs.sh>
> > Assisted-by: LLM
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> >   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

Thanks for your review. v2 sent.