Re: Patch "drm/sysfb: Avoid truncating maximum stride" has been added to the 6.18-stable tree

[sashiko-bot@kernel.org]

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: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>

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/20260618084327.46567-5-tzimmermann@suse.de

> 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