[Mesa-dev] [Mesa-stable] [PATCH 16/18] i965: Prevent coordinate overflow in intel_emit_linear_blit

[Anuj Phogat]

On Mon, Jul 6, 2015 at 3:33 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> Fixes regression from
> commit 8c17d53823c77ac1c56b0548e4e54f69a33285f1
> Author: Kenneth Graunke <kenneth at whitecape.org>
> Date:   Wed Apr 15 03:04:33 2015 -0700
>
>     i965: Make intel_emit_linear_blit handle Gen8+ alignment restrictions.
>
> which adjusted the coordinates to be relative to the nearest cacheline.
> However, this then offsets the coordinates by up to 63 and this may then
> cause them to overflow the BLT limits. For the well aligned large
> transfer case, we can use 32bpp pixels and so reduce the coordinates by
> 4 (versus the current 8bpp pixels). We also have to be more careful
> doing the last line just in case it may exceed the coordinate limit.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90734
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Anuj Phogat <anuj.phogat at gmail.com>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/mesa/drivers/dri/i965/intel_blit.c | 62 ++++++++++++++++------------------
>  1 file changed, 29 insertions(+), 33 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c b/src/mesa/drivers/dri/i965/intel_blit.c
> index 7e81685..dbb53d8 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -762,35 +762,20 @@ intel_emit_linear_blit(struct brw_context *brw,
>     int16_t src_x, dst_x;
>     bool ok;
>
> -   /* The pitch given to the GPU must be DWORD aligned, and
> -    * we want width to match pitch. Max width is (1 << 15 - 1),
> -    * rounding that down to the nearest DWORD is 1 << 15 - 4
> -    */
> -   pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 1), 4);
> -   height = (pitch == 0) ? 1 : size / pitch;
> -   src_x = src_offset % 64;
> -   dst_x = dst_offset % 64;
> -   ok = intelEmitCopyBlit(brw, 1,
> -                         pitch, src_bo, src_offset - src_x, I915_TILING_NONE,
> -                          INTEL_MIPTREE_TRMODE_NONE,
> -                         pitch, dst_bo, dst_offset - dst_x, I915_TILING_NONE,
> -                          INTEL_MIPTREE_TRMODE_NONE,
> -                         src_x, 0, /* src x/y */
> -                         dst_x, 0, /* dst x/y */
> -                         pitch, height, /* w, h */
> -                         GL_COPY);
> -   if (!ok)
> -      _mesa_problem(ctx, "Failed to linear blit %dx%d\n", pitch, height);
> -
> -   src_offset += pitch * height;
> -   dst_offset += pitch * height;
> -   src_x = src_offset % 64;
> -   dst_x = dst_offset % 64;
> -   size -= pitch * height;
> -   assert (size < (1 << 15));
> -   pitch = ALIGN(size, 4);
> -
> -   if (size != 0) {
> +   do {
> +      /* The pitch given to the GPU must be DWORD aligned, and
> +       * we want width to match pitch. Max width is (1 << 15 - 1),
> +       * rounding that down to the nearest DWORD is 1 << 15 - 4
> +       */
> +      pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 64), 4);
I understand why you are subtracting 64 in above statement, it'll
be nice to update above comment explaining the reason.

> +      height = (size < pitch || pitch == 0) ? 1 : size / pitch;
> +
> +      src_x = src_offset % 64;
> +      dst_x = dst_offset % 64;
> +      pitch = ALIGN(MIN2(size, (1 << 15) - 64), 4);
> +      assert(src_x + pitch < 1 << 15);
> +      assert(dst_x + pitch < 1 << 15);
> +
>        ok = intelEmitCopyBlit(brw, 1,
>                              pitch, src_bo, src_offset - src_x, I915_TILING_NONE,
>                               INTEL_MIPTREE_TRMODE_NONE,
> @@ -798,11 +783,22 @@ intel_emit_linear_blit(struct brw_context *brw,
>                               INTEL_MIPTREE_TRMODE_NONE,
>                              src_x, 0, /* src x/y */
>                              dst_x, 0, /* dst x/y */
> -                            size, 1, /* w, h */
> +                            MIN2(size, pitch), height, /* w, h */
>                              GL_COPY);
> -      if (!ok)
> -         _mesa_problem(ctx, "Failed to linear blit %dx%d\n", size, 1);
> -   }
> +      if (!ok) {
> +        _mesa_problem(ctx, "Failed to linear blit %dx%d\n",
> +                      MIN2(size, pitch), height);
> +        return;
> +      }
> +
> +      pitch *= height;
> +      if (size <= pitch)
I think size < pitch will never be true. How about:
assert(size < pitch);
 if (size == pitch)
> +        return;
> +
> +      src_offset += pitch;
> +      dst_offset += pitch;
> +      size -= pitch;
> +   } while (1);
>  }
>
>  /**
> --
> 2.1.4
>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable