From aebcf26d8219cee79da89313124c2147595a660c Mon Sep 17 00:00:00 2001 From: Chad Versace Date: Tue, 18 Nov 2014 21:11:24 -0800 Subject: [PATCH] i965: Fix intel_miptree_map() signature to be more 64-bit safe 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 Signed-off-by: Chad Versace --- src/mesa/drivers/dri/i965/intel_copy_image.c | 4 ++-- src/mesa/drivers/dri/i965/intel_fbo.c | 4 ++-- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 17 ++++++++++++++--- src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 2 +- src/mesa/drivers/dri/i965/intel_tex.c | 7 +++++-- 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_copy_image.c b/src/mesa/drivers/dri/i965/intel_copy_image.c index cb44474a31c..f4c7eff2904 100644 --- a/src/mesa/drivers/dri/i965/intel_copy_image.c +++ b/src/mesa/drivers/dri/i965/intel_copy_image.c @@ -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; diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c index d56fc5bcefd..37ac613fa34 100644 --- a/src/mesa/drivers/dri/i965/intel_fbo.c +++ b/src/mesa/drivers/dri/i965/intel_fbo.c @@ -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); diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 7081f1d3557..f815fbe1e5d 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -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; diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h index f0f6814a599..44ddc6070b6 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h @@ -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, diff --git a/src/mesa/drivers/dri/i965/intel_tex.c b/src/mesa/drivers/dri/i965/intel_tex.c index 549d9b833c9..3be72d5faa1 100644 --- a/src/mesa/drivers/dri/i965/intel_tex.c +++ b/src/mesa/drivers/dri/i965/intel_tex.c @@ -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 -- 2.30.2