| Message ID | CA+yXCZD=HHZrhbG068nLB2xmmSFsAaBMsFxZHcQ=+a8af-BVKw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 09, 2014 at 10:00:38PM +0800, Kito Cheng wrote: > `q` will out of bound access if `*q` already reach the end of > multilib_options, so check it before increment to prevent condition > check part out of bound access. > > btw, this bug is detected by address sanitizer. Can you please expand on which target it is and what multilib_options contains? Perhaps some target just has invalid string in there. > 2014-04-09 Kito Cheng <kito@0xlab.org> > * gcc.c (used_arg): Prevent out of bound access for multilib_options. > > diff --git a/gcc/gcc.c b/gcc/gcc.c > index 5cb485a..c8ab7d6 100644 > --- a/gcc/gcc.c > +++ b/gcc/gcc.c > @@ -7490,7 +7490,7 @@ used_arg (const char *p, int len) > { > const char *r; > > - for (q = multilib_options; *q != '\0'; q++) > + for (q = multilib_options; *q != '\0'; *q && q++) > { > while (*q == ' ') > q++; Jakub
for example: arm-elf-eabi in trunk, multilib_options = "marm/mthumb
mfloat-abi=hard"
and it's my configure options:
/home/kito/gcc/gcc-src/configure
--prefix=/home/kito/gcc-workspace/arm-eabi --target=arm-elf-eabi
CFLAGS="-fsanitize=address -g" CXXFLAGS="-fsanitize=address -g"
LDFLAGS="-fsanitize=address -g"
$ bin/arm-elf-eabi-gcc -v
Using built-in specs.
=================================================================
==26436== ERROR: AddressSanitizer: global-buffer-overflow on address
0x00000051f7dc at pc 0x425b42 bp 0x7fffbb84f890 sp 0x7fffbb84f888
READ of size 1 at 0x00000051f7dc thread T0
#0 0x425b41
(/home/kito/gcc-workspace/arm-eabi/bin/arm-elf-eabi-gcc+0x425b41)
#1 0x426d28
(/home/kito/gcc-workspace/arm-eabi/bin/arm-elf-eabi-gcc+0x426d28)
#2 0x420b5e
(/home/kito/gcc-workspace/arm-eabi/bin/arm-elf-eabi-gcc+0x420b5e)
#3 0x31b3421b44 (/usr/lib64/libc-2.17.so+0x21b44)
#4 0x4032b8
(/home/kito/gcc-workspace/arm-eabi/bin/arm-elf-eabi-gcc+0x4032b8)
0x00000051f7dc is located 36 bytes to the left of global variable
'*.LC2 (/home/kito/gcc/gcc-src/gcc/gcc.c)' (0x51f800) of size 13
'*.LC2 (/home/kito/gcc/gcc-src/gcc/gcc.c)' is ascii string 'arm-elf-eabi'
0x00000051f7dc is located 0 bytes to the right of global variable
'*.LC1 (/home/kito/gcc/gcc-src/gcc/gcc.c)' (0x51f7c0) of size 28
'*.LC1 (/home/kito/gcc/gcc-src/gcc/gcc.c)' is ascii string
'marm/mthumb mfloat-abi=hard'
Shadow bytes around the buggy address:
0x00008009bea0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00008009beb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00008009bec0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00008009bed0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00008009bee0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x00008009bef0: 01 f9 f9 f9 f9 f9 f9 f9 00 00 00[04]f9 f9 f9 f9
0x00008009bf00: 00 05 f9 f9 f9 f9 f9 f9 02 f9 f9 f9 f9 f9 f9 f9
0x00008009bf10: 00 00 00 00 00 00 00 00 00 00 00 00 03 f9 f9 f9
0x00008009bf20: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
0x00008009bf30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00008009bf40: 00 03 f9 f9 f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap righ redzone: fb
Freed Heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
ASan internal: fe
==26436== ABORTING
On Wed, Apr 9, 2014 at 10:03 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Apr 09, 2014 at 10:00:38PM +0800, Kito Cheng wrote:
>> `q` will out of bound access if `*q` already reach the end of
>> multilib_options, so check it before increment to prevent condition
>> check part out of bound access.
>>
>> btw, this bug is detected by address sanitizer.
>
> Can you please expand on which target it is and what multilib_options
> contains? Perhaps some target just has invalid string in there.
>
>> 2014-04-09 Kito Cheng <kito@0xlab.org>
>> * gcc.c (used_arg): Prevent out of bound access for multilib_options.
>>
>> diff --git a/gcc/gcc.c b/gcc/gcc.c
>> index 5cb485a..c8ab7d6 100644
>> --- a/gcc/gcc.c
>> +++ b/gcc/gcc.c
>> @@ -7490,7 +7490,7 @@ used_arg (const char *p, int len)
>> {
>> const char *r;
>>
>> - for (q = multilib_options; *q != '\0'; q++)
>> + for (q = multilib_options; *q != '\0'; *q && q++)
>> {
>> while (*q == ' ')
>> q++;
>
> Jakub
More detail for arm-elf-eabi :) After first iteration at gcc.c:7493-7534, r = q = "mfloat-abi=hard" at gcc.c:7498 then continue scan multilib_options at gcc.c:7499-7507, and then `q` already reach the end of `multilib_options` which mean `q` == multilib_options + strlen(multilib_options) so the `if` is not taken at gcc.c:7509 next, `q++` at gcc.c:7493, it's now `q` == multilib_options + strlen(multilib_options) + 1!!! and finally access `*q` for check `*q` != '\0', out of bound access. On Wed, Apr 9, 2014 at 10:21 PM, Kito Cheng <kito.cheng@gmail.com> wrote: > for example: arm-elf-eabi in trunk, multilib_options = "marm/mthumb > mfloat-abi=hard" > > and it's my configure options: > /home/kito/gcc/gcc-src/configure > --prefix=/home/kito/gcc-workspace/arm-eabi --target=arm-elf-eabi > CFLAGS="-fsanitize=address -g" CXXFLAGS="-fsanitize=address -g" > LDFLAGS="-fsanitize=address -g" > > $ bin/arm-elf-eabi-gcc -v > Using built-in specs. > ================================================================= > ==26436== ERROR: AddressSanitizer: global-buffer-overflow on address > 0x00000051f7dc at pc 0x425b42 bp 0x7fffbb84f890 sp 0x7fffbb84f888 > READ of size 1 at 0x00000051f7dc thread T0 > #0 0x425b41 > (/home/kito/gcc-workspace/arm-eabi/bin/arm-elf-eabi-gcc+0x425b41) > #1 0x426d28 > (/home/kito/gcc-workspace/arm-eabi/bin/arm-elf-eabi-gcc+0x426d28) > #2 0x420b5e > (/home/kito/gcc-workspace/arm-eabi/bin/arm-elf-eabi-gcc+0x420b5e) > #3 0x31b3421b44 (/usr/lib64/libc-2.17.so+0x21b44) > #4 0x4032b8 > (/home/kito/gcc-workspace/arm-eabi/bin/arm-elf-eabi-gcc+0x4032b8) > 0x00000051f7dc is located 36 bytes to the left of global variable > '*.LC2 (/home/kito/gcc/gcc-src/gcc/gcc.c)' (0x51f800) of size 13 > '*.LC2 (/home/kito/gcc/gcc-src/gcc/gcc.c)' is ascii string 'arm-elf-eabi' > 0x00000051f7dc is located 0 bytes to the right of global variable > '*.LC1 (/home/kito/gcc/gcc-src/gcc/gcc.c)' (0x51f7c0) of size 28 > '*.LC1 (/home/kito/gcc/gcc-src/gcc/gcc.c)' is ascii string > 'marm/mthumb mfloat-abi=hard' > Shadow bytes around the buggy address: > 0x00008009bea0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x00008009beb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x00008009bec0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x00008009bed0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x00008009bee0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > =>0x00008009bef0: 01 f9 f9 f9 f9 f9 f9 f9 00 00 00[04]f9 f9 f9 f9 > 0x00008009bf00: 00 05 f9 f9 f9 f9 f9 f9 02 f9 f9 f9 f9 f9 f9 f9 > 0x00008009bf10: 00 00 00 00 00 00 00 00 00 00 00 00 03 f9 f9 f9 > 0x00008009bf20: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00 > 0x00008009bf30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x00008009bf40: 00 03 f9 f9 f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 > Shadow byte legend (one shadow byte represents 8 application bytes): > Addressable: 00 > Partially addressable: 01 02 03 04 05 06 07 > Heap left redzone: fa > Heap righ redzone: fb > Freed Heap region: fd > Stack left redzone: f1 > Stack mid redzone: f2 > Stack right redzone: f3 > Stack partial redzone: f4 > Stack after return: f5 > Stack use after scope: f8 > Global redzone: f9 > Global init order: f6 > Poisoned by user: f7 > ASan internal: fe > ==26436== ABORTING > > On Wed, Apr 9, 2014 at 10:03 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> On Wed, Apr 09, 2014 at 10:00:38PM +0800, Kito Cheng wrote: >>> `q` will out of bound access if `*q` already reach the end of >>> multilib_options, so check it before increment to prevent condition >>> check part out of bound access. >>> >>> btw, this bug is detected by address sanitizer. >> >> Can you please expand on which target it is and what multilib_options >> contains? Perhaps some target just has invalid string in there. >> >>> 2014-04-09 Kito Cheng <kito@0xlab.org> >>> * gcc.c (used_arg): Prevent out of bound access for multilib_options. >>> >>> diff --git a/gcc/gcc.c b/gcc/gcc.c >>> index 5cb485a..c8ab7d6 100644 >>> --- a/gcc/gcc.c >>> +++ b/gcc/gcc.c >>> @@ -7490,7 +7490,7 @@ used_arg (const char *p, int len) >>> { >>> const char *r; >>> >>> - for (q = multilib_options; *q != '\0'; q++) >>> + for (q = multilib_options; *q != '\0'; *q && q++) >>> { >>> while (*q == ' ') >>> q++; >> >> Jakub
On Wed, Apr 09, 2014 at 10:00:38PM +0800, Kito Cheng wrote: > `q` will out of bound access if `*q` already reach the end of > multilib_options, so check it before increment to prevent condition > check part out of bound access. > > btw, this bug is detected by address sanitizer. > > > 2014-04-09 Kito Cheng <kito@0xlab.org> > * gcc.c (used_arg): Prevent out of bound access for multilib_options. There should be a newline between date/name/email line and * gcc.c ... and the * gcc.c line should be indented by tab. > diff --git a/gcc/gcc.c b/gcc/gcc.c > index 5cb485a..c8ab7d6 100644 > --- a/gcc/gcc.c > +++ b/gcc/gcc.c > @@ -7490,7 +7490,7 @@ used_arg (const char *p, int len) > { > const char *r; > > - for (q = multilib_options; *q != '\0'; q++) > + for (q = multilib_options; *q != '\0'; *q && q++) > { > while (*q == ' ') > q++; Ok for trunk and 4.9.1. Jakub
diff --git a/gcc/gcc.c b/gcc/gcc.c index 5cb485a..c8ab7d6 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -7490,7 +7490,7 @@ used_arg (const char *p, int len) { const char *r; - for (q = multilib_options; *q != '\0'; q++) + for (q = multilib_options; *q != '\0'; *q && q++) { while (*q == ' ') q++;
`q` will out of bound access if `*q` already reach the end of multilib_options, so check it before increment to prevent condition check part out of bound access. btw, this bug is detected by address sanitizer. 2014-04-09 Kito Cheng <kito@0xlab.org> * gcc.c (used_arg): Prevent out of bound access for multilib_options.