Re: [PATCH v2] drm/i915/display: Prevent u64 underflow in intel_fbc_stolen_end

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 07 Jan 2026, Jonathan Cavitt <jonathan.cavitt@intel.com> 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, fix the other comments here and modify the execution
> path for readability.
>
> v2: (Jani)
> - Fix the comments in intel_fbc_stolen_end
> - Use check_sub_overflow
> - Remove macro that mirrors SZ_8M, as it is now only referenced once
> - Misc. formatting fixes
>
> Fixes: a9da512b3ed7 ("drm/i915: avoid the last 8mb of stolen on BDW/SKL")
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 29 +++++++++++++++++-------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index fef2f35ff1e9..1f3f5237a1c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -809,19 +809,32 @@ static u64 intel_fbc_cfb_base_max(struct intel_display *display)
>  
>  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
> +	/*
> +	 * 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. */
> +	 * 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 SZ_8M, use
> +		 * intel_fbc_cfb_base_max instead.  This should not happen,
> +		 * so warn if it does.
> +		 */
> +		if (drm_WARN_ON(display->drm,
> +				check_sub_overflow(stolen_area_size,
> +						   SZ_8M, &stolen_area_size)))
> +			return end;
> +
> +		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