Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Calculation of vsize inside drm_sysfb_get_visible_size_si suffers from a 32-bit integer overflow before PAGE_ALIGN, leading to a truncated memory mapping. - [High] drm_sysfb_get_visible_size_si implicitly casts a negative errno to u64, bypassing the error checks in callers and causing resource registration corruption. -- --- Patch [1]: Patch "drm/sysfb: Avoid truncating maximum stride" has been added to the 6.18-stable tree --- commit afbe56da35160af0caa025f567da427b14e63119 Author: [email protected] <[email protected]> Patch "drm/sysfb: Avoid truncating maximum stride" has been added to the 6.18-stable tree Passing a maximum as 64-bit type to drm_sysfb_get_validated_int0() can truncate the value to 32 bits. Use drm_sysfb_get_validated_size0(), which uses 64-bit arithmetics. Then test the returned stride against the limits of int to avoid truncations in the returned value. A valid stride is in the range of [1, INT_MAX] inclusive. Link: https://patch.msgid.link/[email protected] > diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c > b/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c > index 885864168c54f..7c336360556d1 100644 > --- a/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c > +++ b/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c > @@ -56,11 +56,17 @@ int drm_sysfb_get_stride_si(struct drm_device *dev, const > struct screen_info *si > unsigned int width, unsigned int height, u64 size) > { > u64 lfb_linelength = si->lfb_linelength; > + s64 stride; > > if (!lfb_linelength) > lfb_linelength = drm_format_info_min_pitch(format, 0, width); > > - return drm_sysfb_get_validated_int0(dev, "stride", lfb_linelength, > div64_u64(size, height)); > + stride = drm_sysfb_get_validated_size0(dev, "stride", lfb_linelength, > + div64_u64(size, height)); > + if (stride < INT_MIN || stride > INT_MAX) > + return -EINVAL; > + > + return (int)stride; /* stride or negative errno code */ > } > EXPORT_SYMBOL(drm_sysfb_get_stride_si); [Severity: High] This isn't a regression introduced by this patch, but looking at the next function in this file, drm_sysfb_get_visible_size_si(), is there a 32-bit overflow risk here? drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c:drm_sysfb_get_visible_size_si() { u64 vsize = PAGE_ALIGN(height * stride); Since height and stride are both 32-bit unsigned integers, their product is calculated using 32-bit arithmetic before being passed to PAGE_ALIGN and widened to u64. If the bootloader supplies screen dimensions that cause the product to exceed 4GB, would this overflow wrap around to a small size and cause the driver to allocate and map an insufficient memory region? [Severity: High] This is also a pre-existing issue, but does this implicitly cast a negative errno to u64? drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c:drm_sysfb_get_visible_size_si() { ... return drm_sysfb_get_validated_size0(dev, "visible size", vsize, size); } drm_sysfb_get_validated_size0() returns s64, which can be an error code like -EINVAL. Since drm_sysfb_get_visible_size_si() returns u64, -EINVAL is cast to a massive positive number. If the framebuffer size is not page-aligned, drm_sysfb_get_validated_size0() returns -EINVAL. Will callers that only check for zero fail to catch this? For example, in drivers/gpu/drm/sysfb/efidrm.c:efidrm_device_create(): if (!vsize) return ERR_PTR(-EINVAL); Could this massive u64 value then be passed to devm_aperture_acquire_for_platform_device(), resulting in the driver registering a corrupt aperture that spans almost the entire physical address space and spuriously detaches the primary firmware framebuffer? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/2026072945-lather-carmaker-b8ca@gregkh?part=1
