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 <[email protected]>

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: <[email protected]> # 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/[email protected]?part=1

Reply via email to