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

Kenneth Graunke kenneth at whitecape.org
Tue Apr 8 00:28:45 PDT 2014


On 04/07/2014 09:20 PM, 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.
> 
> v2: add more explanatory comments
> v3: add bugzilla reference
> v4: Correct cc syntax in commit log
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74988
> Cc: "9.2 10.0 10.1" <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 cbda689..39775e2 100644
> --- a/src/mesa/main/texcompress_etc.c
> +++ b/src/mesa/main/texcompress_etc.c
> @@ -683,9 +683,10 @@ etc2_unpack_rgb8(uint8_t *dst_row,
>           etc2_rgb8_parse_block(&block, src,
>                                 false /* punchthrough_alpha */);
>  
> -         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;
> @@ -720,9 +721,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 */
> @@ -763,9 +764,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;
>              }
> @@ -800,9 +801,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 */
> @@ -842,9 +843,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;
>              }
> @@ -878,10 +879,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;
>              }
> @@ -889,10 +890,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;
>              }
> @@ -925,10 +926,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;
>              }
> @@ -962,10 +963,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;
>              }
> @@ -973,10 +974,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;
>              }
> @@ -1006,9 +1007,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;
> @@ -1041,9 +1042,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 */
> 

As is,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

I do like Ian's suggestion (from 2014-02-14), where he suggested doing:

    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++) {

That keeps the loop termination conditions as a simple comparison,
rather than two comparisons, an add, and a logical-and.

--Ken

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-stable/attachments/20140408/1dd12981/attachment-0001.sig>


More information about the mesa-stable mailing list