i965: Fix intel_miptree_map() signature to be more 64-bit safe
authorChad Versace <chad.versace@linux.intel.com>
Wed, 19 Nov 2014 05:11:24 +0000 (21:11 -0800)
committerChad Versace <chad.versace@intel.com>
Mon, 22 Dec 2014 21:47:07 +0000 (15:47 -0600)
This patch should diminish the likelihood of pointer arithmetic overflow
bugs, like the one fixed by b69c7c5dac.

Change the type of parameter 'out_stride' from int to ptrdiff_t. The
logic is that if you call intel_miptree_map() and use the value of
'out_stride', then you must be doing pointer arithmetic on 'out_ptr'.
Using ptrdiff_t instead of int should make a little bit harder to hit
overflow bugs.

As a side-effect, some function-scope variables needed to be retyped to
avoid compilation errors.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Chad Versace <chad.versace@linux.intel.com>
src/mesa/drivers/dri/i965/intel_copy_image.c
src/mesa/drivers/dri/i965/intel_fbo.c
src/mesa/drivers/dri/i965/intel_mipmap_tree.c
src/mesa/drivers/dri/i965/intel_mipmap_tree.h
src/mesa/drivers/dri/i965/intel_tex.c

index cb44474a31cec8021f6a1eb02c6db813f44539e1..f4c7eff2904d4f00fba8d15495a4e14f7d68631f 100644 (file)
@@ -145,7 +145,7 @@ copy_image_with_memcpy(struct brw_context *brw,
 {
    bool same_slice;
    void *mapped, *src_mapped, *dst_mapped;
-   int src_stride, dst_stride, i, cpp;
+   ptrdiff_t src_stride, dst_stride, cpp;
    int map_x1, map_y1, map_x2, map_y2;
    GLuint src_bw, src_bh;
 
@@ -197,7 +197,7 @@ copy_image_with_memcpy(struct brw_context *brw,
    src_width /= (int)src_bw;
    src_height /= (int)src_bh;
 
-   for (i = 0; i < src_height; ++i) {
+   for (int i = 0; i < src_height; ++i) {
       memcpy(dst_mapped, src_mapped, src_width * cpp);
       src_mapped += src_stride;
       dst_mapped += dst_stride;
index d56fc5bcefd3af1080a551a951b952097f6c2a5c..37ac613fa34db09421add1e3e4ab2407190fe524 100644 (file)
@@ -127,7 +127,7 @@ intel_map_renderbuffer(struct gl_context *ctx,
    struct intel_renderbuffer *irb = intel_renderbuffer(rb);
    struct intel_mipmap_tree *mt;
    void *map;
-   int stride;
+   ptrdiff_t stride;
 
    if (srb->Buffer) {
       /* this is a malloc'd renderbuffer (accum buffer), not an irb */
@@ -189,7 +189,7 @@ intel_map_renderbuffer(struct gl_context *ctx,
       stride = -stride;
    }
 
-   DBG("%s: rb %d (%s) mt mapped: (%d, %d) (%dx%d) -> %p/%d\n",
+   DBG("%s: rb %d (%s) mt mapped: (%d, %d) (%dx%d) -> %p/%"PRIdPTR"\n",
        __FUNCTION__, rb->Name, _mesa_get_format_name(rb->Format),
        x, y, w, h, map, stride);
 
index 7081f1d35578af08914667c5b374af0b63fdc6e2..f815fbe1e5d3370613d433aecfd226748abbabd5 100644 (file)
@@ -1111,7 +1111,7 @@ intel_miptree_copy_slice_sw(struct brw_context *brw,
                             int height)
 {
    void *src, *dst;
-   int src_stride, dst_stride;
+   ptrdiff_t src_stride, dst_stride;
    int cpp = dst_mt->cpp;
 
    intel_miptree_map(brw, src_mt,
@@ -1129,7 +1129,7 @@ intel_miptree_copy_slice_sw(struct brw_context *brw,
                      BRW_MAP_DIRECT_BIT,
                      &dst, &dst_stride);
 
-   DBG("sw blit %s mt %p %p/%d -> %s mt %p %p/%d (%dx%d)\n",
+   DBG("sw blit %s mt %p %p/%"PRIdPTR" -> %s mt %p %p/%"PRIdPTR" (%dx%d)\n",
        _mesa_get_format_name(src_mt->format),
        src_mt, src, src_stride,
        _mesa_get_format_name(dst_mt->format),
@@ -2259,6 +2259,17 @@ can_blit_slice(struct intel_mipmap_tree *mt,
    return true;
 }
 
+/**
+ * Parameter \a out_stride has type ptrdiff_t not because the buffer stride may
+ * exceed 32 bits but to diminish the likelihood subtle bugs in pointer
+ * arithmetic overflow.
+ *
+ * If you call this function and use \a out_stride, then you're doing pointer
+ * arithmetic on \a out_ptr. The type of \a out_stride doesn't prevent all
+ * bugs.  The caller must still take care to avoid 32-bit overflow errors in
+ * all arithmetic expressions that contain buffer offsets and pixel sizes,
+ * which usually have type uint32_t or GLuint.
+ */
 void
 intel_miptree_map(struct brw_context *brw,
                   struct intel_mipmap_tree *mt,
@@ -2270,7 +2281,7 @@ intel_miptree_map(struct brw_context *brw,
                   unsigned int h,
                   GLbitfield mode,
                   void **out_ptr,
-                  int *out_stride)
+                  ptrdiff_t *out_stride)
 {
    struct intel_miptree_map *map;
 
index f0f6814a5998018d730b3ece118227a357db7f22..44ddc6070b69d9da17596b2b6b5bb860fb2cc013 100644 (file)
@@ -733,7 +733,7 @@ intel_miptree_map(struct brw_context *brw,
                  unsigned int h,
                  GLbitfield mode,
                  void **out_ptr,
-                 int *out_stride);
+                 ptrdiff_t *out_stride);
 
 void
 intel_miptree_unmap(struct brw_context *brw,
index 549d9b833c9d987230f6c8ac71802e2be8c77754..3be72d5faa12f86564d03d1ef4c92792aeae0619 100644 (file)
@@ -205,11 +205,12 @@ intel_map_texture_image(struct gl_context *ctx,
                        GLuint x, GLuint y, GLuint w, GLuint h,
                        GLbitfield mode,
                        GLubyte **map,
-                       GLint *stride)
+                       GLint *out_stride)
 {
    struct brw_context *brw = brw_context(ctx);
    struct intel_texture_image *intel_image = intel_texture_image(tex_image);
    struct intel_mipmap_tree *mt = intel_image->mt;
+   ptrdiff_t stride;
 
    /* Our texture data is always stored in a miptree. */
    assert(mt);
@@ -228,7 +229,9 @@ intel_map_texture_image(struct gl_context *ctx,
                      tex_image->Level + tex_image->TexObject->MinLevel,
                      slice + tex_image->TexObject->MinLayer,
                      x, y, w, h, mode,
-                     (void **)map, stride);
+                     (void **)map, &stride);
+
+   *out_stride = stride;
 }
 
 static void