[PATCH] drm/sysfb: ofdrm: Fix integer overflow in fb_size calculation

[PATCH] drm/sysfb: ofdrm: Fix integer overflow in fb_size calculation

[oushixiong1025]

From: Shixiong Ou <oushixiong@kylinos.cn>

The framebuffer size calculation `fb_size = linebytes * height` can
overflow when both values are large (e.g., 46341 * 46341 > INT_MAX).
Since linebytes and height are both int types, the multiplication is
performed as int * int, which results in undefined behavior on overflow.

Use check_mul_overflow() to detect and prevent this overflow, consistent
with the approach used in simpledrm.c and corebootdrm.c.

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/ofdrm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
index 819aed466727..a6dc34b9ec0f 100644
--- a/drivers/gpu/drm/sysfb/ofdrm.c
+++ b/drivers/gpu/drm/sysfb/ofdrm.c
@@ -2,6 +2,7 @@
 
 #include <linux/aperture.h>
 #include <linux/of_address.h>
+#include <linux/overflow.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
@@ -913,7 +914,10 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
 			return ERR_PTR(-EINVAL);
 	}
 
-	fb_size = linebytes * height;
+	if (check_mul_overflow(linebytes, height, &fb_size)) {
+		drm_err(dev, "framebuffer size exceeds maximum\n");
+		return ERR_PTR(-EINVAL);
+	}
 
 	/*
 	 * Try to figure out the address of the framebuffer. Unfortunately, Open
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH] drm/sysfb: ofdrm: Fix integer overflow in fb_size calculation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver accepts any non-zero `linebytes` from the Device Tree without ensuring it is at least as large as `width * bytes_per_pixel`.
- [High] The 64-bit result of `drm_format_info_min_pitch()` is silently truncated when assigned to the 32-bit `int linebytes` variable.
- [High] The `round_up(fb_size, PAGE_SIZE)` calculation can overflow to 0 on 32-bit systems, leading to an undersized memory mapping.
- [High] On 32-bit architectures with 64-bit physical addresses (PAE/LPAE), passing a 64-bit size to `ioremap` via `devm_ioremap` truncates it to a 32-bit `size_t`.
--

--- Patch [1]: [PATCH] drm/sysfb: ofdrm: Fix integer overflow in fb_size calculation ---
commit ad44f8947b62f451f5098acf1a574e6b16699aa6
Author: Shixiong Ou <oushixiong@kylinos.cn>

drm/sysfb: ofdrm: Fix integer overflow in fb_size calculation

The framebuffer size calculation fb_size = linebytes * height can
overflow when both values are large (e.g., 46341 * 46341 > INT_MAX).
Since linebytes and height are both int types, the multiplication is
performed as int * int, which results in undefined behavior on overflow.

Use check_mul_overflow() to detect and prevent this overflow, consistent
with the approach used in simpledrm.c and corebootdrm.c.

> diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
> index 819aed466727e..a6dc34b9ec0f3 100644
> --- a/drivers/gpu/drm/sysfb/ofdrm.c
> +++ b/drivers/gpu/drm/sysfb/ofdrm.c
> @@ -913,7 +914,10 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
>  			return ERR_PTR(-EINVAL);
>  	}
>  
> -	fb_size = linebytes * height;
> +	if (check_mul_overflow(linebytes, height, &fb_size)) {

[Severity: High]
This isn't a bug introduced by this patch, but does drm_format_info_min_pitch()
called earlier in this function return a 64-bit value that is silently truncated
when assigned to the 32-bit int linebytes variable?

	if (!linebytes) {
		linebytes = drm_format_info_min_pitch(format, 0, width);
		if (drm_WARN_ON(dev, !linebytes))
			return ERR_PTR(-EINVAL);
	}

If width is maliciously large (e.g., 0x40000001), the 64-bit result could be
truncated to 4, bypassing the !linebytes check and the new check_mul_overflow()
check.

[Severity: High]
This is a pre-existing issue, but is there a missing lower bound check for
linebytes in the same block above? 

If the Device Tree provides a maliciously small but non-zero linebytes (e.g., 4),
it bypasses the !linebytes check. It would also pass the new
check_mul_overflow() check but could cause out-of-bounds memory writes
downstream when DRM plane updates write data based on the logical width.

[Severity: High]
This is a pre-existing issue, but on 32-bit systems where resource_size_t is
32-bit, could round_up(fb_size, PAGE_SIZE) computed later in this function
overflow and wrap around to 0?

	fb_pgbase = round_down(fb_base, PAGE_SIZE);
	fb_pgsize = fb_base - fb_pgbase + round_up(fb_size, PAGE_SIZE);

If fb_size is near U32_MAX, the overflow check succeeds, but this calculation
would result in an undersized memory mapping.

[Severity: High]
This is a pre-existing issue, but on 32-bit architectures with 64-bit physical
addresses (PAE/LPAE), does devm_ioremap() silently truncate the 64-bit size to a
32-bit size_t? 

	screen_base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));

Since check_mul_overflow() checks against the 64-bit resource_size_t fb_size, a
massive size wouldn't be caught there, and would be truncated here, leading to a
smaller mapping than the DRM core expects.

> +		drm_err(dev, "framebuffer size exceeds maximum\n");
> +		return ERR_PTR(-EINVAL);
> +	}
>  
>  	/*
>  	 * Try to figure out the address of the framebuffer. Unfortunately, Open

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825104134.669676-1-oushixiong1025@163.com?part=1

Re: [PATCH] drm/sysfb: ofdrm: Fix integer overflow in fb_size calculation

[Thomas Zimmermann]

Merged into drm-misc-fixes

Am 25.08.26 um 12:41 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> The framebuffer size calculation `fb_size = linebytes * height` can
> overflow when both values are large (e.g., 46341 * 46341 > INT_MAX).
> Since linebytes and height are both int types, the multiplication is
> performed as int * int, which results in undefined behavior on overflow.
>
> Use check_mul_overflow() to detect and prevent this overflow, consistent
> with the approach used in simpledrm.c and corebootdrm.c.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/sysfb/ofdrm.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
> index 819aed466727..a6dc34b9ec0f 100644
> --- a/drivers/gpu/drm/sysfb/ofdrm.c
> +++ b/drivers/gpu/drm/sysfb/ofdrm.c
> @@ -2,6 +2,7 @@
>   
>   #include <linux/aperture.h>
>   #include <linux/of_address.h>
> +#include <linux/overflow.h>
>   #include <linux/pci.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm.h>
> @@ -913,7 +914,10 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
>   			return ERR_PTR(-EINVAL);
>   	}
>   
> -	fb_size = linebytes * height;
> +	if (check_mul_overflow(linebytes, height, &fb_size)) {
> +		drm_err(dev, "framebuffer size exceeds maximum\n");
> +		return ERR_PTR(-EINVAL);
> +	}
>   
>   	/*
>   	 * Try to figure out the address of the framebuffer. Unfortunately, Open

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)