Re: [PATCH v2 1/6] 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] The simpledrm driver fails to correctly validate that the framebuffer dimensions fit within the mapped memory resource size due to an inverted condition.
--

commit 1c3adcb617c4a3add398f3f24ada5131889b5d6a
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/sysfb: simpledrm: Improve framebuffer-size validation

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.

Cc: <stable@vger.kernel.org> # v5.14+

> diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
> index fc168920f2c64..15dcafa9d524a 100644
> --- a/drivers/gpu/drm/sysfb/simpledrm.c
> +++ b/drivers/gpu/drm/sysfb/simpledrm.c

[ ... ]

> @@ -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

[Severity: High]
This isn't a bug introduced by this patch, but commit 'drm/sysfb:
simpledrm: Validate mmap size against framebuffer size' in this series
attempted to fix the missing validation and failed due to an inverted
condition.

Does simpledrm_device_create() fail to correctly validate that the
framebuffer dimensions fit within the mapped memory resource size?

Looking at simpledrm_device_create(), the check appears to be inverted in
both the system memory and I/O memory fallback paths:

drivers/gpu/drm/sysfb/simpledrm.c:simpledrm_device_create() {
        ...
		if (resource_size(mem) > size) {
			drm_err(dev,
				"framebuffer size of %llu exceeds memory range %pr\n",
				size, mem);
			return ERR_PTR(-EINVAL);
		}
        ...
}

Because this check returns -EINVAL when resource_size(mem) > size, it
incorrectly rejects valid configurations where the physical memory region
is larger than the required framebuffer.

More importantly, it allows invalid configurations where size >
resource_size(mem). In this case, devm_memremap() maps the smaller
resource_size(mem), but drm_fb_memcpy() will access size bytes during
plane updates, leading to an out-of-bounds kernel memory access past the
mapped area.

Should this check be size > resource_size(mem) instead?

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