[Intel-gfx] [PATCH] drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB

[Chris Wilson]

Quoting Ville Syrjala (2018-09-11 17:54:57)
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> If we have framebuffers that are >= 4GiB in size we will overflow
> the fb size check in intel_fill_fb_info().
> 
> Currently that is only possible with NV12 and CCS as offsets[1]
> may be anything between 0 and 0xffffffff. ofsets[0] is currently
> required to be 0 so we can't hit the overflow with any single
> plane format (thanks to max fb size of 8kx8k and max stride of
> 32 KiB).
> 
> In the future we may allow almost any framebuffer to exceed 4GiB
> in size so we really should fix the overflow. Not that the overflow
> is particularly dangerous. It's mostly just a sanity check against
> insane userspace. The display engine can't write to memory anyway
> so I suppose in the worst case we might anger the hw by attempting
> scanout past the end of the ggtt, or we might scan out some data
> that we're not supposed to see from other parts of the ggtt.
> 
> Note that triggering this overflow depends on the driver
> aligning the fb height to the next tile boundary to push the
> calculated size above 4GiB. With linear buffers the effective
> tile height is one so that never happens, and the core already
> has a check for 32bit overflow of offsets[]+pitches[]*height.
> 
> Testcase: igt/kms_big_fb/x-tiled-addfb-size-offset-overflow
> Testcase: igt/kms_big_fb/y-tiled-addfb-size-offset-overflow
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2b77d9350a3a..2b474d049074 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2636,9 +2636,10 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
>                 max_size = max(max_size, offset + size);
>         }
>  
> -       if (max_size * tile_size > obj->base.size) {
> -               DRM_DEBUG_KMS("fb too big for bo (need %u bytes, have %zu bytes)\n",
> -                             max_size * tile_size, obj->base.size);
> +       if (mul_u32_u32(max_size, tile_size) > obj->base.size) {
> +               DRM_DEBUG_KMS("fb too big for bo (need %llu bytes, have %zu bytes)\n",
> +                             (unsigned long long) mul_u32_u32(max_size, tile_size),

mul_u32_u32 returns u64 i.e. unsigned long long; %llu is the one true
format specifier for u64 (Linus decree #103789)

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris