-----Original Message----- From: Jani Nikula <[email protected]> Sent: Wednesday, January 7, 2026 3:39 AM To: Cavitt, Jonathan <[email protected]>; [email protected] Cc: Gupta, Saurabhg <[email protected]>; Zuo, Alex <[email protected]>; Cavitt, Jonathan <[email protected]>; Zanoni, Paulo R <[email protected]>; [email protected]; [email protected] Subject: Re: [PATCH] drm/i915/display: Prevent u64 underflow in intel_fbc_stolen_end > > On Fri, 19 Dec 2025, Jonathan Cavitt <[email protected]> wrote: > > Static analysis reveals a potential integer underflow in > > intel_fbc_stolen_end. This can apparently occur if > > intel_parent_stolen_area_size returns zero (or, theoretically, any value > > less than 2^23), as 2^23 is subtracted from the return value and stored > > in a u64. While this doesn't appear to cause any issues due to the use > > of the min() function to clamp the return values from the > > intel_fbc_stolen_end function, it would be best practice to avoid > > undeflowing values like this on principle. So, rework the function to > > prevent the underflow from occurring. Note that the underflow at > > present would result in the value of intel_fbc_cfb_base_max being > > returned at the end of intel_fbc_stolen_end, so just return that if the > > value of intel_parent_stolen_area_size is too small. > > > > While we're here, create a macro for the 2^23 value and modify the > > execution path for readability. > > > > Fixes: a9da512b3ed7 ("drm/i915: avoid the last 8mb of stolen on BDW/SKL") > > Signed-off-by: Jonathan Cavitt <[email protected]> > > Cc: Paulo Zanoni <[email protected]> > > Cc: Ville Syrjälä <[email protected]> > > Cc: Daniel Vetter <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_fbc.c | 20 ++++++++++++++------ > > 1 file changed, 14 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > index fef2f35ff1e9..00c32df50933 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > @@ -807,21 +807,29 @@ static u64 intel_fbc_cfb_base_max(struct > > intel_display *display) > > return BIT_ULL(32); > > } > > > > +#define STOLEN_RESERVE_MAX SZ_8M > > static u64 intel_fbc_stolen_end(struct intel_display *display) > > { > > - u64 end; > > + u64 end = intel_fbc_cfb_base_max(display); > > > > /* The FBC hardware for BDW/SKL doesn't have access to the stolen > > * reserved range size, so it always assumes the maximum (8mb) is used. > > * If we enable FBC using a CFB on that memory range we'll get FIFO > > * underruns, even if that range is not reserved by the BIOS. */ > > if (display->platform.broadwell || > > - (DISPLAY_VER(display) == 9 && !display->platform.broxton)) > > - end = intel_parent_stolen_area_size(display) - 8 * 1024 * 1024; > > - else > > - end = U64_MAX; > > + (DISPLAY_VER(display) == 9 && !display->platform.broxton)) { > > + u64 stolen_area_size = intel_parent_stolen_area_size(display); > > + > > + /* If stolen_area_size is less than STOLEN_RESERVE_MAX, > > + * use intel_fbc_cfb_base_max instead. */ > > Please use the proper multi-line comment style.
Should I also fix the comment about FBC hardware while I'm here? > > > + if (stolen_area_size < STOLEN_RESERVE_MAX) > > + return end; > > check_sub_overflow(), perhaps with a drm_WARN_ON(), would be the way to > go I think. You can get rid of the extra macro too. So, instead of STOLEN_RESERVE_MAX, just directly reference SZ_8M here? -Jonathan Cavitt > > > + > > + stolen_area_size -= STOLEN_RESERVE_MAX; > > A blank line is preferred before return. > > > + return min(end, stolen_area_size); > > + } > > > > - return min(end, intel_fbc_cfb_base_max(display)); > > + return end; > > } > > > > static int intel_fbc_min_limit(const struct intel_plane_state *plane_state) > > -- > Jani Nikula, Intel >
