[Mesa-dev] [PATCH] mesa: add bounds checking to eliminate buffer overrun

Courtney Goeltzenleuchter courtney at lunarg.com
Fri Feb 28 12:55:28 PST 2014


No, I do not believe this commit has made it.

Courtney


On Wed, Feb 26, 2014 at 11:44 AM, Ian Romanick <idr at freedesktop.org> wrote:

> On 02/17/2014 09:43 AM, Courtney Goeltzenleuchter wrote:
>
>> Thanks for the feedback.
>>
>> I've updated the patch to integrate Ian's comments.
>>
>
> Did the updated patch ever land?
>
>  Courtney
>>
>>
>> On Fri, Feb 14, 2014 at 2:00 PM, Ian Romanick <idr at freedesktop.org
>> <mailto:idr at freedesktop.org>> wrote:
>>
>>     On 02/14/2014 07:52 AM, Courtney Goeltzenleuchter wrote:
>>      > Decompressing ETC2 textures was causing intermitent segfault
>>      > by copying resulting 4x4 texel block to the destination texture
>>      > regardless of the size of the destination texture. Issue found
>>      > via application crash in GLBenchmark 3.0's Manhattan test.
>>
>>     So... the problem is that every ETC texture is (physically) a multiple
>>     of 4 width, but we may have allocated a decompression buffer that was
>>     the logical width of the texture.  I think the code needs more of a
>>     comment explaining that.  Otherwise, when someone comes back to it in
>> a
>>     year (or uses it as a model for some other texture compression code),
>>     they won't understand why the code is the way it is.
>>
>>     With that, this patch is
>>
>>     Reviewed-by: Ian Romanick <ian.d.romanick at intel.com
>>     <mailto:ian.d.romanick at intel.com>>
>>
>>
>>     One additional suggestion below that you can take or leave.
>>
>>      > Signed-off-by: Courtney Goeltzenleuchter <courtney at LunarG.com>
>>
>>     Also add:
>>
>>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74988
>>     Cc: "9.2 10.0 10.1" <mesa-stable at lists.freedesktop.org
>>     <mailto:mesa-stable at lists.freedesktop.org>>
>>
>>
>>      > ---
>>      >  src/mesa/main/texcompress_etc.c | 49
>>     +++++++++++++++++++++--------------------
>>      >  1 file changed, 25 insertions(+), 24 deletions(-)
>>      >
>>      > diff --git a/src/mesa/main/texcompress_etc.c
>>     b/src/mesa/main/texcompress_etc.c
>>      > index e3862be..f9234b0 100644
>>      > --- a/src/mesa/main/texcompress_etc.c
>>      > +++ b/src/mesa/main/texcompress_etc.c
>>      > @@ -684,9 +684,10 @@ etc2_unpack_rgb8(uint8_t *dst_row,
>>      >           etc2_rgb8_parse_block(&block, src,
>>      >                                 false /* punchthrough_alpha */);
>>      >
>>
>>     Alternately, the code below could be:
>>
>>          const unsigned h = MIN2(bh, height - y);
>>          const unsigned w = MIN2(bw, width - x);
>>
>>          ...
>>
>>          for (j = 0; j < h; j++) {
>>
>>          ...
>>
>>             for (i = 0; i < w; i++) {
>>
>>
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         /* be sure to stay within the bounds of the texture */
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride + x *
>>     comps;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_rgb8_fetch_texel(&block, i, j, dst,
>>      >                                       false /* punchthrough_alpha
>>     */);
>>      >                 dst[3] = 255;
>>      > @@ -721,9 +722,9 @@ etc2_unpack_srgb8(uint8_t *dst_row,
>>      >           etc2_rgb8_parse_block(&block, src,
>>      >                                 false /* punchthrough_alpha */);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride + x *
>>     comps;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_rgb8_fetch_texel(&block, i, j, dst,
>>      >                                       false /* punchthrough_alpha
>>     */);
>>      >                 /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */
>>      > @@ -764,9 +765,9 @@ etc2_unpack_rgba8(uint8_t *dst_row,
>>      >        for (x = 0; x < width; x+= bw) {
>>      >           etc2_rgba8_parse_block(&block, src);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride + x *
>>     comps;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_rgba8_fetch_texel(&block, i, j, dst);
>>      >                 dst += comps;
>>      >              }
>>      > @@ -801,9 +802,9 @@ etc2_unpack_srgb8_alpha8(uint8_t *dst_row,
>>      >        for (x = 0; x < width; x+= bw) {
>>      >           etc2_rgba8_parse_block(&block, src);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride + x *
>>     comps;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_rgba8_fetch_texel(&block, i, j, dst);
>>      >
>>      >                 /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */
>>      > @@ -843,9 +844,9 @@ etc2_unpack_r11(uint8_t *dst_row,
>>      >        for (x = 0; x < width; x+= bw) {
>>      >           etc2_r11_parse_block(&block, src);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride + x *
>>     comps * comp_size;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_r11_fetch_texel(&block, i, j, dst);
>>      >                 dst += comps * comp_size;
>>      >              }
>>      > @@ -879,10 +880,10 @@ etc2_unpack_rg11(uint8_t *dst_row,
>>      >           /* red component */
>>      >           etc2_r11_parse_block(&block, src);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride +
>>      >                             x * comps * comp_size;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_r11_fetch_texel(&block, i, j, dst);
>>      >                 dst += comps * comp_size;
>>      >              }
>>      > @@ -890,10 +891,10 @@ etc2_unpack_rg11(uint8_t *dst_row,
>>      >           /* green component */
>>      >           etc2_r11_parse_block(&block, src + 8);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride +
>>      >                             x * comps * comp_size;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_r11_fetch_texel(&block, i, j, dst +
>> comp_size);
>>      >                 dst += comps * comp_size;
>>      >              }
>>      > @@ -926,10 +927,10 @@ etc2_unpack_signed_r11(uint8_t *dst_row,
>>      >        for (x = 0; x < width; x+= bw) {
>>      >           etc2_r11_parse_block(&block, src);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride +
>>      >                             x * comps * comp_size;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_signed_r11_fetch_texel(&block, i, j, dst);
>>      >                 dst += comps * comp_size;
>>      >              }
>>      > @@ -963,10 +964,10 @@ etc2_unpack_signed_rg11(uint8_t *dst_row,
>>      >           /* red component */
>>      >           etc2_r11_parse_block(&block, src);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride +
>>      >                            x * comps * comp_size;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_signed_r11_fetch_texel(&block, i, j, dst);
>>      >                 dst += comps * comp_size;
>>      >              }
>>      > @@ -974,10 +975,10 @@ etc2_unpack_signed_rg11(uint8_t *dst_row,
>>      >           /* green component */
>>      >           etc2_r11_parse_block(&block, src + 8);
>>      >
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride +
>>      >                             x * comps * comp_size;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_signed_r11_fetch_texel(&block, i, j, dst +
>>     comp_size);
>>      >                 dst += comps * comp_size;
>>      >              }
>>      > @@ -1007,9 +1008,9 @@
>>     etc2_unpack_rgb8_punchthrough_alpha1(uint8_t *dst_row,
>>      >        for (x = 0; x < width; x+= bw) {
>>      >           etc2_rgb8_parse_block(&block, src,
>>      >                                 true /* punchthrough_alpha */);
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride + x *
>>     comps;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_rgb8_fetch_texel(&block, i, j, dst,
>>      >                                       true /* punchthrough_alpha
>> */);
>>      >                 dst += comps;
>>      > @@ -1042,9 +1043,9 @@
>>     etc2_unpack_srgb8_punchthrough_alpha1(uint8_t *dst_row,
>>      >        for (x = 0; x < width; x+= bw) {
>>      >           etc2_rgb8_parse_block(&block, src,
>>      >                                 true /* punchthrough_alpha */);
>>      > -         for (j = 0; j < bh; j++) {
>>      > +         for (j = 0; j < bh && (j+y) < height; j++) {
>>      >              uint8_t *dst = dst_row + (y + j) * dst_stride + x *
>>     comps;
>>      > -            for (i = 0; i < bw; i++) {
>>      > +            for (i = 0; i < bw && (i+x) < width; i++) {
>>      >                 etc2_rgb8_fetch_texel(&block, i, j, dst,
>>      >                                       true /* punchthrough_alpha
>> */);
>>      >                 /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */
>>      >
>>
>>
>>
>>
>> --
>> Courtney Goeltzenleuchter
>> LunarG
>>
>>
>


-- 
Courtney Goeltzenleuchter
LunarG
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140228/86f38df9/attachment-0001.html>


More information about the mesa-dev mailing list