Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] 32-bit integer overflow in `height * stride` calculation leading to out-of-bounds I/O memory mapping and subsequent out-of-bounds writes. - [Medium] Truncation of `div64_u64` result when passed to the `u32 max` parameter of `drm_sysfb_get_validated_int0`. -- commit caeaafe04348540484a7b3b0bfa6ff4db7c32a32 Author: Thomas Zimmermann <[email protected]> drm/sysfb: Return errno code from drm_sysfb_get_visible_size() Change the return type of drm_sysfb_get_visible_size() to s64 so that it returns a possible errno code from _get_validated_size0(). Fix callers to handle the errno code. > diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c > b/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c > index 361b7233600c2..40b4f7883b9bc 100644 > --- a/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c > +++ b/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c > @@ -64,7 +64,7 @@ int drm_sysfb_get_stride_si(struct drm_device *dev, const > struct screen_info *si > } > EXPORT_SYMBOL(drm_sysfb_get_stride_si); > > -u64 drm_sysfb_get_visible_size_si(struct drm_device *dev, const struct > screen_info *si, > +s64 drm_sysfb_get_visible_size_si(struct drm_device *dev, const struct > screen_info *si, > unsigned int height, unsigned int stride, u64 > size) > { > u64 vsize = height * stride; [Severity: High] This is a pre-existing issue, but does this calculation overflow if height and stride are large? Both height and stride are passed as 32-bit unsigned integers to drm_sysfb_get_visible_size_si(), so their product is computed as a 32-bit unsigned integer before being implicitly extended to the 64-bit vsize. If they overflow (e.g., height up to U16_MAX and stride up to U16_MAX * 4), the truncated 32-bit value could bypass the bounds check in drm_sysfb_get_validated_size0(). This could result in requesting and mapping a falsely small I/O memory region for the framebuffer, potentially leading to an out-of-bounds memory write later. Could explicit 64-bit casting or mul_u32_u32() be used to prevent this? [Severity: Medium] This isn't a bug introduced by this patch, but looking at drm_sysfb_get_stride_si() located just above this in drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c: int drm_sysfb_get_stride_si(...) { ... return drm_sysfb_get_validated_int0(dev, "stride", lfb_linelength, div64_u64(size, height)); } Could the result of div64_u64(size, height) be truncated when passed to drm_sysfb_get_validated_int0()? The max parameter of drm_sysfb_get_validated_int0() is typed as u32. If the total resource size is large and the height is small (for example, size = 4GB and height = 1), the division result would overflow the u32 type and truncate to 0. This would cause drm_sysfb_get_validated_int0() to use a max value of 0, falsely rejecting a valid configuration with -EINVAL. > return drm_sysfb_get_validated_size0(dev, "visible size", vsize, size); > } [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
