[Intel-gfx] [PATCH 4/5] drm/i915: Fix overflows in fixed16_16

[Chris Wilson]

Quoting Michal Wajdeczko (2017-12-22 12:25:55)
> If someone provides too large number for fixed16 type
> we will WARN but we will not correctly clamp values
> and that may lead to fully wrong calculations.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/fixed16_16.h | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/fixed16_16.h b/drivers/gpu/drm/i915/fixed16_16.h
> index af23997..43fe0037 100644
> --- a/drivers/gpu/drm/i915/fixed16_16.h
> +++ b/drivers/gpu/drm/i915/fixed16_16.h
> @@ -57,7 +57,8 @@ static inline fixed16_16_t u32_to_fixed16(u32 val)
>  {
>         fixed16_16_t fp;
>  
> -       WARN_ON(val > U16_MAX);
> +       if (WARN_ON(val > U16_MAX))
> +               val = U16_MAX;

return FIXED16_16_MAX;

If val is too large, the closest we can get to val in our representation
is 16_16_MAX.

>  
>         fp.val = val << 16;
>         return fp;

return TO_FIXED16_16(val << 16);

> @@ -93,7 +94,9 @@ static inline fixed16_16_t clamp_u64_to_fixed16(u64 val)
>  {
>         fixed16_16_t fp;
>  
> -       WARN_ON(val > U32_MAX);
> +       if (WARN_ON(val > U32_MAX))
> +               val = U32_MAX;

I would do the early return. Perhaps check with gcc if prefers returning
a constant than the jump back?
-Chris