Re: [PATCH v3 1/7] drm/sysfb: simpledrm: Improve framebuffer-size validation

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing validation for device tree provided `stride` against `width * bpp` allows out-of-bounds memory writes.
--

commit daaa8ad78a371c66bd2324aa7a47d551084fe671
Author: Thomas Zimmermann <tzimmermann@suse.de>
drm/sysfb: simpledrm: Improve framebuffer-size validation

This commit validates the framebuffer dimensions provided by firmware against
the 16-bit limits of struct drm_display_mode, failing the probe on errors.

Fixes: 11e8f5fd223b ("drm: Add simpledrm driver")
Cc: <stable@vger.kernel.org> # v5.14+

[ ... ]
> @@ -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);
>  }

[Severity: High]
This is a pre-existing issue, but does this code need to ensure a provided
stride is at least large enough for the given width and pixel format?

In simpledrm_device_create(), if the device tree or platform data provides a
non-zero stride, it bypasses the minimum pitch calculation:

	if (!stride) {
		u64 pitch = drm_format_info_min_pitch(format, 0, width);
		...
		stride = pitch;
	}

The driver then calculates the required size using this stride:

	if (check_mul_overflow(height, stride, &size)) {
		...
		if (size > resource_size(mem)) {

If firmware provides a small but non-zero stride (for example, 1), the size
calculation yields a small value, which easily passes the resource size check.

However, rendering and clearing operations write width * bpp bytes per line.
For example, in drm_sysfb_plane_helper_atomic_disable():

	for (i = 0; i < lines; ++i) {
		memset_io(dst_vmap, 0, linepixels * dst_format->cpp[0]);
		dst_vmap += dst_pitch;
	}

Can this lead to out-of-bounds memory writes if the firmware provides a stride
that is smaller than width * bpp?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625094509.157581-1-tzimmermann@suse.de?part=1