[PATCH v2 1/6] drm/sysfb: simpledrm: Improve framebuffer-size validation

[PATCH v2 1/6] drm/sysfb: simpledrm: Improve framebuffer-size validation

[Thomas Zimmermann]

Validate the framebuffer size from the firmware against the
limitations of struct drm_display_mode. The type only stores sizes
in 16-bit fields. Fail probing on errors.

v2:
- remove unused function simplefb_get_validated_int0() (Sashiko)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 11e8f5fd223b ("drm: Add simpledrm driver")
Cc: <stable@vger.kernel.org> # v5.14+
---
 drivers/gpu/drm/sysfb/simpledrm.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
index fc168920f2c6..15dcafa9d524 100644
--- a/drivers/gpu/drm/sysfb/simpledrm.c
+++ b/drivers/gpu/drm/sysfb/simpledrm.c
@@ -48,13 +48,6 @@ simplefb_get_validated_int(struct drm_device *dev, const char *name,
 	return drm_sysfb_get_validated_int(dev, name, value, INT_MAX);
 }
 
-static int
-simplefb_get_validated_int0(struct drm_device *dev, const char *name,
-			    uint32_t value)
-{
-	return drm_sysfb_get_validated_int0(dev, name, value, INT_MAX);
-}
-
 static const struct drm_format_info *
 simplefb_get_validated_format(struct drm_device *dev, const char *format_name)
 {
@@ -88,14 +81,14 @@ static int
 simplefb_get_width_pd(struct drm_device *dev,
 		      const struct simplefb_platform_data *pd)
 {
-	return simplefb_get_validated_int0(dev, "width", pd->width);
+	return drm_sysfb_get_validated_int0(dev, "width", pd->width, U16_MAX);
 }
 
 static int
 simplefb_get_height_pd(struct drm_device *dev,
 		       const struct simplefb_platform_data *pd)
 {
-	return simplefb_get_validated_int0(dev, "height", pd->height);
+	return drm_sysfb_get_validated_int0(dev, "height", pd->height, U16_MAX);
 }
 
 static int
@@ -144,7 +137,7 @@ simplefb_get_width_of(struct drm_device *dev, struct device_node *of_node)
 
 	if (ret)
 		return ret;
-	return simplefb_get_validated_int0(dev, "width", width);
+	return drm_sysfb_get_validated_int0(dev, "width", width, U16_MAX);
 }
 
 static int
@@ -155,7 +148,7 @@ simplefb_get_height_of(struct drm_device *dev, struct device_node *of_node)
 
 	if (ret)
 		return ret;
-	return simplefb_get_validated_int0(dev, "height", height);
+	return drm_sysfb_get_validated_int0(dev, "height", height, U16_MAX);
 }
 
 static int
-- 
2.54.0

Re: [PATCH v2 1/6] drm/sysfb: simpledrm: Improve framebuffer-size validation

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

On Mon, Jun 22, 2026 at 03:19:35PM +0200, Thomas Zimmermann wrote:
> Validate the framebuffer size from the firmware against the
> limitations of struct drm_display_mode. The type only stores sizes
> in 16-bit fields. Fail probing on errors.
> 
> v2:
> - remove unused function simplefb_get_validated_int0() (Sashiko)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 11e8f5fd223b ("drm: Add simpledrm driver")
> Cc: <stable@vger.kernel.org> # v5.14+
> ---
>  drivers/gpu/drm/sysfb/simpledrm.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 2/6] drm/sysfb: simpledrm: Improve panel-size validation

[Thomas Zimmermann]

Validate the panel size from the device-tree node against the
limitations of struct drm_display_mode. The type only stores sizes
in 16-bit fields. Fail transparently on errors; do not warn.

v2:
- only use initialized values in debugging output (Sashiko)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 2a6d731a8f16 ("drm/simpledrm: Allow physical width and height configuration via panel node")
Cc: Rayyan Ansari <rayyan@ansari.sh>
Cc: <stable@vger.kernel.org> # v6.4+
---
 drivers/gpu/drm/sysfb/simpledrm.c | 40 ++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
index 15dcafa9d524..fa2121f81def 100644
--- a/drivers/gpu/drm/sysfb/simpledrm.c
+++ b/drivers/gpu/drm/sysfb/simpledrm.c
@@ -193,6 +193,40 @@ simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
 	return res;
 }
 
+static u16
+__simplefb_get_panel_size_mm_of(struct drm_device *dev, struct device_node *of_panel_node,
+				const char *name)
+{
+	int ret;
+	u32 value;
+
+	ret = of_property_read_u32(of_panel_node, name, &value);
+	if (ret) {
+		drm_dbg(dev, "simplefb: cannot parse panel %s: error %d\n",
+			name, ret);
+		return 0; /* not an error, simply ignore */
+	}
+	if (value > U16_MAX) {
+		drm_dbg(dev, "simplefb: panel %s of %u exceeds maximum value\n",
+			name, value);
+		return 0; /* not an error, simply ignore */
+	}
+
+	return value;
+}
+
+static u16
+simplefb_get_panel_width_mm_of(struct drm_device *dev, struct device_node *of_panel_node)
+{
+	return __simplefb_get_panel_size_mm_of(dev, of_panel_node, "width-mm");
+}
+
+static u16
+simplefb_get_panel_height_mm_of(struct drm_device *dev, struct device_node *of_panel_node)
+{
+	return __simplefb_get_panel_size_mm_of(dev, of_panel_node, "height-mm");
+}
+
 /*
  * Simple Framebuffer device
  */
@@ -594,7 +628,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	struct drm_sysfb_device *sysfb;
 	struct drm_device *dev;
 	int width, height, stride;
-	int width_mm = 0, height_mm = 0;
+	u16 width_mm = 0, height_mm = 0;
 	struct device_node *panel_node;
 	const struct drm_format_info *format;
 	struct resource *res, *mem = NULL;
@@ -658,8 +692,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 			return ERR_CAST(mem);
 		panel_node = of_parse_phandle(of_node, "panel", 0);
 		if (panel_node) {
-			simplefb_read_u32_of(dev, panel_node, "width-mm", &width_mm);
-			simplefb_read_u32_of(dev, panel_node, "height-mm", &height_mm);
+			width_mm = simplefb_get_panel_width_mm_of(dev, panel_node);
+			height_mm = simplefb_get_panel_height_mm_of(dev, panel_node);
 			of_node_put(panel_node);
 		}
 	} else {
-- 
2.54.0

Re: [PATCH v2 2/6] drm/sysfb: simpledrm: Improve panel-size validation

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3698 bytes --]

On Mon, Jun 22, 2026 at 03:19:36PM +0200, Thomas Zimmermann wrote:
> Validate the panel size from the device-tree node against the
> limitations of struct drm_display_mode. The type only stores sizes
> in 16-bit fields. Fail transparently on errors; do not warn.
> 
> v2:
> - only use initialized values in debugging output (Sashiko)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 2a6d731a8f16 ("drm/simpledrm: Allow physical width and height configuration via panel node")
> Cc: Rayyan Ansari <rayyan@ansari.sh>
> Cc: <stable@vger.kernel.org> # v6.4+
> ---
>  drivers/gpu/drm/sysfb/simpledrm.c | 40 ++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
> index 15dcafa9d524..fa2121f81def 100644
> --- a/drivers/gpu/drm/sysfb/simpledrm.c
> +++ b/drivers/gpu/drm/sysfb/simpledrm.c
> @@ -193,6 +193,40 @@ simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
>  	return res;
>  }
>  
> +static u16
> +__simplefb_get_panel_size_mm_of(struct drm_device *dev, struct device_node *of_panel_node,
> +				const char *name)
> +{
> +	int ret;
> +	u32 value;
> +
> +	ret = of_property_read_u32(of_panel_node, name, &value);
> +	if (ret) {
> +		drm_dbg(dev, "simplefb: cannot parse panel %s: error %d\n",
> +			name, ret);
> +		return 0; /* not an error, simply ignore */
> +	}
> +	if (value > U16_MAX) {
> +		drm_dbg(dev, "simplefb: panel %s of %u exceeds maximum value\n",
> +			name, value);
> +		return 0; /* not an error, simply ignore */

I wonder if it's perhaps better to move this comment to the function
scope and explain why this can be ignored. I didn't know and had to go
look at drm_sysfb_mode() to see that if these are 0, it'll compute the
physical dimensions based on a default of 96 DPI.

> +	}
> +
> +	return value;
> +}
> +
> +static u16
> +simplefb_get_panel_width_mm_of(struct drm_device *dev, struct device_node *of_panel_node)
> +{
> +	return __simplefb_get_panel_size_mm_of(dev, of_panel_node, "width-mm");
> +}
> +
> +static u16
> +simplefb_get_panel_height_mm_of(struct drm_device *dev, struct device_node *of_panel_node)
> +{
> +	return __simplefb_get_panel_size_mm_of(dev, of_panel_node, "height-mm");
> +}
> +
>  /*
>   * Simple Framebuffer device
>   */
> @@ -594,7 +628,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>  	struct drm_sysfb_device *sysfb;
>  	struct drm_device *dev;
>  	int width, height, stride;
> -	int width_mm = 0, height_mm = 0;
> +	u16 width_mm = 0, height_mm = 0;
>  	struct device_node *panel_node;
>  	const struct drm_format_info *format;
>  	struct resource *res, *mem = NULL;
> @@ -658,8 +692,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>  			return ERR_CAST(mem);
>  		panel_node = of_parse_phandle(of_node, "panel", 0);
>  		if (panel_node) {
> -			simplefb_read_u32_of(dev, panel_node, "width-mm", &width_mm);
> -			simplefb_read_u32_of(dev, panel_node, "height-mm", &height_mm);
> +			width_mm = simplefb_get_panel_width_mm_of(dev, panel_node);
> +			height_mm = simplefb_get_panel_height_mm_of(dev, panel_node);
>  			of_node_put(panel_node);
>  		}
>  	} else {

The drm_sysfb_mode() function that width_mm and height_mm get passed
into accepts them as unsigned int, so maybe that should be changed as
well for more consistency?

In either case, since they all end up in the u16 in the struct, it's
obviously correct to check for the range when parsing, so:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 2/6] drm/sysfb: simpledrm: Improve panel-size validation

[Thomas Zimmermann]

Hi

Am 22.06.26 um 18:05 schrieb Thierry Reding:
> On Mon, Jun 22, 2026 at 03:19:36PM +0200, Thomas Zimmermann wrote:
>> Validate the panel size from the device-tree node against the
>> limitations of struct drm_display_mode. The type only stores sizes
>> in 16-bit fields. Fail transparently on errors; do not warn.
>>
>> v2:
>> - only use initialized values in debugging output (Sashiko)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: 2a6d731a8f16 ("drm/simpledrm: Allow physical width and height configuration via panel node")
>> Cc: Rayyan Ansari <rayyan@ansari.sh>
>> Cc: <stable@vger.kernel.org> # v6.4+
>> ---
>>   drivers/gpu/drm/sysfb/simpledrm.c | 40 ++++++++++++++++++++++++++++---
>>   1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
>> index 15dcafa9d524..fa2121f81def 100644
>> --- a/drivers/gpu/drm/sysfb/simpledrm.c
>> +++ b/drivers/gpu/drm/sysfb/simpledrm.c
>> @@ -193,6 +193,40 @@ simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
>>   	return res;
>>   }
>>   
>> +static u16
>> +__simplefb_get_panel_size_mm_of(struct drm_device *dev, struct device_node *of_panel_node,
>> +				const char *name)
>> +{
>> +	int ret;
>> +	u32 value;
>> +
>> +	ret = of_property_read_u32(of_panel_node, name, &value);
>> +	if (ret) {
>> +		drm_dbg(dev, "simplefb: cannot parse panel %s: error %d\n",
>> +			name, ret);
>> +		return 0; /* not an error, simply ignore */
>> +	}
>> +	if (value > U16_MAX) {
>> +		drm_dbg(dev, "simplefb: panel %s of %u exceeds maximum value\n",
>> +			name, value);
>> +		return 0; /* not an error, simply ignore */
> I wonder if it's perhaps better to move this comment to the function
> scope and explain why this can be ignored. I didn't know and had to go
> look at drm_sysfb_mode() to see that if these are 0, it'll compute the
> physical dimensions based on a default of 96 DPI.

Makes sense.

>
>> +	}
>> +
>> +	return value;
>> +}
>> +
>> +static u16
>> +simplefb_get_panel_width_mm_of(struct drm_device *dev, struct device_node *of_panel_node)
>> +{
>> +	return __simplefb_get_panel_size_mm_of(dev, of_panel_node, "width-mm");
>> +}
>> +
>> +static u16
>> +simplefb_get_panel_height_mm_of(struct drm_device *dev, struct device_node *of_panel_node)
>> +{
>> +	return __simplefb_get_panel_size_mm_of(dev, of_panel_node, "height-mm");
>> +}
>> +
>>   /*
>>    * Simple Framebuffer device
>>    */
>> @@ -594,7 +628,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>   	struct drm_sysfb_device *sysfb;
>>   	struct drm_device *dev;
>>   	int width, height, stride;
>> -	int width_mm = 0, height_mm = 0;
>> +	u16 width_mm = 0, height_mm = 0;
>>   	struct device_node *panel_node;
>>   	const struct drm_format_info *format;
>>   	struct resource *res, *mem = NULL;
>> @@ -658,8 +692,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>   			return ERR_CAST(mem);
>>   		panel_node = of_parse_phandle(of_node, "panel", 0);
>>   		if (panel_node) {
>> -			simplefb_read_u32_of(dev, panel_node, "width-mm", &width_mm);
>> -			simplefb_read_u32_of(dev, panel_node, "height-mm", &height_mm);
>> +			width_mm = simplefb_get_panel_width_mm_of(dev, panel_node);
>> +			height_mm = simplefb_get_panel_height_mm_of(dev, panel_node);
>>   			of_node_put(panel_node);
>>   		}
>>   	} else {
> The drm_sysfb_mode() function that width_mm and height_mm get passed
> into accepts them as unsigned int, so maybe that should be changed as
> well for more consistency?

Also makes sense.

Best regards
Thomas

>
> In either case, since they all end up in the u16 in the struct, it's
> obviously correct to check for the range when parsing, so:
>
> Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v2 4/6] drm/sysfb: simpledrm: Improve stride validation

[Thomas Zimmermann]

Validate the computed stride against the maximum value INT_MAX.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 7bfa5c7b28d6 ("drm/simpledrm: Compute linestride with drm_format_info_min_pitch()")
Cc: <stable@vger.kernel.org> # v6.1+
---
 drivers/gpu/drm/sysfb/simpledrm.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
index 709922ea656c..76b9a3f5c4ef 100644
--- a/drivers/gpu/drm/sysfb/simpledrm.c
+++ b/drivers/gpu/drm/sysfb/simpledrm.c
@@ -694,9 +694,15 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 		return ERR_PTR(-ENODEV);
 	}
 	if (!stride) {
-		stride = drm_format_info_min_pitch(format, 0, width);
-		if (drm_WARN_ON(dev, !stride))
+		u64 pitch = drm_format_info_min_pitch(format, 0, width);
+
+		if (drm_WARN_ON(dev, !pitch)) {
+			return ERR_PTR(-EINVAL); /* driver bug */
+		} else if (pitch > INT_MAX) {
+			drm_warn(dev, "stride of %llu exceeds maximum\n", pitch);
 			return ERR_PTR(-EINVAL);
+		}
+		stride = pitch;
 	}
 
 	sysfb->fb_mode = drm_sysfb_mode(width, height, width_mm, height_mm);
-- 
2.54.0

Re: [PATCH v2 4/6] drm/sysfb: simpledrm: Improve stride validation

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

On Mon, Jun 22, 2026 at 03:19:38PM +0200, Thomas Zimmermann wrote:
> Validate the computed stride against the maximum value INT_MAX.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 7bfa5c7b28d6 ("drm/simpledrm: Compute linestride with drm_format_info_min_pitch()")
> Cc: <stable@vger.kernel.org> # v6.1+
> ---
>  drivers/gpu/drm/sysfb/simpledrm.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Ugh... I have a hard time imagining that we'd ever exceed INT_MAX here,
but I guess better safe than sorry?

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 4/6] drm/sysfb: simpledrm: Improve stride validation

[Thomas Zimmermann]

Hi

Am 22.06.26 um 18:10 schrieb Thierry Reding:
> On Mon, Jun 22, 2026 at 03:19:38PM +0200, Thomas Zimmermann wrote:
>> Validate the computed stride against the maximum value INT_MAX.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: 7bfa5c7b28d6 ("drm/simpledrm: Compute linestride with drm_format_info_min_pitch()")
>> Cc: <stable@vger.kernel.org> # v6.1+
>> ---
>>   drivers/gpu/drm/sysfb/simpledrm.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
> Ugh... I have a hard time imagining that we'd ever exceed INT_MAX here,
> but I guess better safe than sorry?

Yeah. The idea of these patches is to harden the driver against all kind 
of nonsense the firmware might give it. With patch 1 that limits the 
width to 2^16, I don't think it's possible to exceed a pitch of INT_MAX, 
but let's check anyway.

Best regards
Thomas

>
> Reviewed-by: Thierry Reding <treding@nvidia.com>

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