* [PATCH] iproute2: Conforming to -D_FORTIFY_SOURCE=2 restrictions
@ 2011-10-17 7:35 Bin Li
2011-10-17 15:23 ` Stephen Hemminger
0 siblings, 1 reply; 5+ messages in thread
From: Bin Li @ 2011-10-17 7:35 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]
Hi,
The issue is from below link.
https://bugzilla.novell.com/show_bug.cgi?id=719537
The issue is debug at below.
(gdb) bt
#0 0x00007ffff7697945 in raise (sig=<optimized out>)
at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x00007ffff7698f21 in abort () at abort.c:92
#2 0x00007ffff76d48ef in __libc_message (do_abort=2,
fmt=0x7ffff7789541 "*** %s ***: %s terminated\n")
at ../sysdeps/unix/sysv/linux/libc_fatal.c:186
#3 0x00007ffff7750177 in __fortify_fail (
msg=0x7ffff77894d8 "buffer overflow detected") at fortify_fail.c:32
#4 0x00007ffff774de10 in __chk_fail () at chk_fail.c:29
#5 0x00007ffff774cf8d in __strncpy_chk (
s1=0x640c <Address 0x640c out of bounds>,
s2=0x640c <Address 0x640c out of bounds>, n=6, s1len=18446744073709551615)
at strncpy_chk.c:34
#6 0x000000000041e9c8 in strncpy (__len=<optimized out>,
__src=<optimized out>, __dest=<optimized out>)
at /usr/include/bits/string3.h:123
#7 xfrm_algo_parse (max=<optimized out>, buf=<optimized out>,
key=<optimized out>, name=<optimized out>, type=<optimized out>,
alg=<optimized out>) at xfrm_state.c:166
(gdb) l
161 len = slen;
162 if (len > 0) {
163 if (len > max)
164 invarg("\"ALGOKEY\" makes buffer
overflow\n", key);
165
166 strncpy(buf, key, len);
167 }
168 }
169
170 alg->alg_key_len = len * 8;
(gdb) up
#8 xfrm_state_modify (cmd=<optimized out>, flags=<optimized out>, argc=1,
argv=0x7fffffffe370) at xfrm_state.c:406
406 xfrm_algo_parse((void *)&alg, type,
name, key,
the compiler passes zero to __builtin___strncpy_chk as the buffer size.
xfrm_algo_parse is inlined into xfrm_state_modify.
Thanks!
Sincerely Yours,
Bin Li
http://zh.opensuse.org
[-- Attachment #2: iproute2-FORTIFY_SOURCE.patch --]
[-- Type: text/x-patch, Size: 2234 bytes --]
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index a76be47..30a9aa3 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -368,13 +368,16 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
struct xfrm_algo_auth auth;
} u;
char buf[XFRM_ALGO_KEY_BUF_SIZE];
- } alg = {};
+ } *alg;
int len;
__u32 icvlen, trunclen;
char *name;
char *key;
char *buf;
+ alg = alloca (sizeof (*alg) + XFRM_ALGO_KEY_BUF_SIZE);
+ memset (alg, 0, sizeof (*alg) + XFRM_ALGO_KEY_BUF_SIZE);
+
switch (type) {
case XFRMA_ALG_AEAD:
if (aeadop)
@@ -412,8 +415,8 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
NEXT_ARG();
key = *argv;
- buf = alg.u.alg.alg_key;
- len = sizeof(alg.u.alg);
+ buf = alg->u.alg.alg_key;
+ len = sizeof(alg->u.alg);
switch (type) {
case XFRMA_ALG_AEAD:
@@ -423,10 +426,10 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
if (get_u32(&icvlen, *argv, 0))
invarg("\"aead\" ICV length is invalid",
*argv);
- alg.u.aead.alg_icv_len = icvlen;
+ alg->u.aead.alg_icv_len = icvlen;
- buf = alg.u.aead.alg_key;
- len = sizeof(alg.u.aead);
+ buf = alg->u.aead.alg_key;
+ len = sizeof(alg->u.aead);
break;
case XFRMA_ALG_AUTH_TRUNC:
if (!NEXT_ARG_OK())
@@ -435,19 +438,19 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
if (get_u32(&trunclen, *argv, 0))
invarg("\"auth\" trunc length is invalid",
*argv);
- alg.u.auth.alg_trunc_len = trunclen;
+ alg->u.auth.alg_trunc_len = trunclen;
- buf = alg.u.auth.alg_key;
- len = sizeof(alg.u.auth);
+ buf = alg->u.auth.alg_key;
+ len = sizeof(alg->u.auth);
break;
}
- xfrm_algo_parse((void *)&alg, type, name, key,
- buf, sizeof(alg.buf));
- len += alg.u.alg.alg_key_len;
+ xfrm_algo_parse((void *)alg, type, name, key,
+ buf, sizeof(alg->buf));
+ len += alg->u.alg.alg_key_len;
addattr_l(&req.n, sizeof(req.buf), type,
- (void *)&alg, len);
+ (void *)alg, len);
break;
}
default:
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iproute2: Conforming to -D_FORTIFY_SOURCE=2 restrictions
2011-10-17 7:35 [PATCH] iproute2: Conforming to -D_FORTIFY_SOURCE=2 restrictions Bin Li
@ 2011-10-17 15:23 ` Stephen Hemminger
2011-10-19 9:15 ` Bin Li
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2011-10-17 15:23 UTC (permalink / raw)
To: Bin Li; +Cc: netdev
On Mon, 17 Oct 2011 15:35:35 +0800
Bin Li <libin.charles@gmail.com> wrote:
> (gdb) l
> 161 len = slen;
> 162 if (len > 0) {
> 163 if (len > max)
> 164 invarg("\"ALGOKEY\" makes buffer
> overflow\n", key);
> 165
> 166 strncpy(buf, key, len);
> 167 }
> 168 }
> 169
> 170 alg->alg_key_len = len * 8;
> (gdb) up
> #8 xfrm_state_modify (cmd=<optimized out>, flags=<optimized out>, argc=1,
> argv=0x7fffffffe370) at xfrm_state.c:406
> 406 xfrm_algo_parse((void *)&alg, type,
> name, key,
>
> the compiler passes zero to __builtin___strncpy_chk as the buffer size.
> xfrm_algo_parse is inlined into xfrm_state_modify.
I don't understand, looks like a compiler bug. Call strncpy with
0 length should not be possible since the check was 3 lines
before for len > 0.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iproute2: Conforming to -D_FORTIFY_SOURCE=2 restrictions
2011-10-17 15:23 ` Stephen Hemminger
@ 2011-10-19 9:15 ` Bin Li
2011-10-19 11:30 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Bin Li @ 2011-10-19 9:15 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Stephen,
You can reproduce this issue in 2.6.37 like below. And the previous
gdb log is after the install the debuginfo package in SUSE.
# ip -6 xfrm state add src 3ffe:501:ffff:ff03:21a:64ff:fe12:e4c1 dst
3ffe:501:ffff:ff05:200:ff:fe00:c1c1 proto ah spi 0x1000 mode transport
auth md5 "TAHITEST89ABCDEF"
*** buffer overflow detected ***: ip terminated
======= Backtrace: =========
/lib/libc.so.6(__fortify_fail+0x40)[0xb76d0070]
/lib/libc.so.6(+0xe8e27)[0xb76cde27]
/lib/libc.so.6(+0xe8317)[0xb76cd317]
ip[0x806d6c4]
ip(do_xfrm_state+0x120)[0x806dc70]
ip(do_xfrm+0x81)[0x806ad51]
ip[0x804c355]
ip(main+0x476)[0x804caa6]
/lib/libc.so.6(__libc_start_main+0xfe)[0xb75fbc2e]
ip[0x804c261]
======= Memory map: ========
08048000-08087000 r-xp 00000000 08:01 4465 /sbin/ip
08087000-08088000 r--p 0003e000 08:01 4465 /sbin/ip
08088000-0808a000 rw-p 0003f000 08:01 4465 /sbin/ip
0808a000-080ad000 rw-p 00000000 00:00 0 [heap]
b75c6000-b75e2000 r-xp 00000000 08:01 131084 /lib/libgcc_s.so.1
b75e2000-b75e3000 r--p 0001b000 08:01 131084 /lib/libgcc_s.so.1
b75e3000-b75e4000 rw-p 0001c000 08:01 131084 /lib/libgcc_s.so.1
b75e4000-b75e5000 rw-p 00000000 00:00 0
b75e5000-b774b000 r-xp 00000000 08:01 131375 /lib/libc-2.11.3.so
b774b000-b774c000 ---p 00166000 08:01 131375 /lib/libc-2.11.3.so
b774c000-b774e000 r--p 00166000 08:01 131375 /lib/libc-2.11.3.so
b774e000-b774f000 rw-p 00168000 08:01 131375 /lib/libc-2.11.3.so
b774f000-b7752000 rw-p 00000000 00:00 0
b7752000-b7755000 r-xp 00000000 08:01 131428 /lib/libdl-2.11.3.so
b7755000-b7756000 r--p 00002000 08:01 131428 /lib/libdl-2.11.3.so
b7756000-b7757000 rw-p 00003000 08:01 131428 /lib/libdl-2.11.3.so
b7774000-b7775000 rw-p 00000000 00:00 0
b7775000-b7794000 r-xp 00000000 08:01 154467 /lib/ld-2.11.3.so
b7794000-b7795000 r--p 0001e000 08:01 154467 /lib/ld-2.11.3.so
b7795000-b7796000 rw-p 0001f000 08:01 154467 /lib/ld-2.11.3.so
bfa02000-bfa23000 rw-p 00000000 00:00 0 [stack]
ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso]
Aborted
And If without -D_FORTIFY_SOURCE=2 in gcc, it works fine, so It's a
bug in iproute2 which is not conforming to -D_FORTIFY_SOURCE=2
restrictions.
Thanks!
On Mon, Oct 17, 2011 at 11:23 PM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Mon, 17 Oct 2011 15:35:35 +0800
> Bin Li <libin.charles@gmail.com> wrote:
>
>> (gdb) l
>> 161 len = slen;
>> 162 if (len > 0) {
>> 163 if (len > max)
>> 164 invarg("\"ALGOKEY\" makes buffer
>> overflow\n", key);
>> 165
>> 166 strncpy(buf, key, len);
>> 167 }
>> 168 }
>> 169
>> 170 alg->alg_key_len = len * 8;
>> (gdb) up
>> #8 xfrm_state_modify (cmd=<optimized out>, flags=<optimized out>, argc=1,
>> argv=0x7fffffffe370) at xfrm_state.c:406
>> 406 xfrm_algo_parse((void *)&alg, type,
>> name, key,
>>
>> the compiler passes zero to __builtin___strncpy_chk as the buffer size.
>> xfrm_algo_parse is inlined into xfrm_state_modify.
>
> I don't understand, looks like a compiler bug. Call strncpy with
> 0 length should not be possible since the check was 3 lines
> before for len > 0.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iproute2: Conforming to -D_FORTIFY_SOURCE=2 restrictions
2011-10-19 9:15 ` Bin Li
@ 2011-10-19 11:30 ` Eric Dumazet
2011-10-19 16:50 ` Stephen Hemminger
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2011-10-19 11:30 UTC (permalink / raw)
To: Bin Li; +Cc: Stephen Hemminger, netdev
Le mercredi 19 octobre 2011 à 17:15 +0800, Bin Li a écrit :
> Stephen,
>
> You can reproduce this issue in 2.6.37 like below. And the previous
> gdb log is after the install the debuginfo package in SUSE.
>
> # ip -6 xfrm state add src 3ffe:501:ffff:ff03:21a:64ff:fe12:e4c1 dst
> 3ffe:501:ffff:ff05:200:ff:fe00:c1c1 proto ah spi 0x1000 mode transport
> auth md5 "TAHITEST89ABCDEF"
>
> *** buffer overflow detected ***: ip terminated
> ======= Backtrace: =========
> /lib/libc.so.6(__fortify_fail+0x40)[0xb76d0070]
> /lib/libc.so.6(+0xe8e27)[0xb76cde27]
> /lib/libc.so.6(+0xe8317)[0xb76cd317]
> ip[0x806d6c4]
> ip(do_xfrm_state+0x120)[0x806dc70]
> ip(do_xfrm+0x81)[0x806ad51]
> ip[0x804c355]
> ip(main+0x476)[0x804caa6]
> /lib/libc.so.6(__libc_start_main+0xfe)[0xb75fbc2e]
> ip[0x804c261]
> ======= Memory map: ========
> 08048000-08087000 r-xp 00000000 08:01 4465 /sbin/ip
> 08087000-08088000 r--p 0003e000 08:01 4465 /sbin/ip
> 08088000-0808a000 rw-p 0003f000 08:01 4465 /sbin/ip
> 0808a000-080ad000 rw-p 00000000 00:00 0 [heap]
> b75c6000-b75e2000 r-xp 00000000 08:01 131084 /lib/libgcc_s.so.1
> b75e2000-b75e3000 r--p 0001b000 08:01 131084 /lib/libgcc_s.so.1
> b75e3000-b75e4000 rw-p 0001c000 08:01 131084 /lib/libgcc_s.so.1
> b75e4000-b75e5000 rw-p 00000000 00:00 0
> b75e5000-b774b000 r-xp 00000000 08:01 131375 /lib/libc-2.11.3.so
> b774b000-b774c000 ---p 00166000 08:01 131375 /lib/libc-2.11.3.so
> b774c000-b774e000 r--p 00166000 08:01 131375 /lib/libc-2.11.3.so
> b774e000-b774f000 rw-p 00168000 08:01 131375 /lib/libc-2.11.3.so
> b774f000-b7752000 rw-p 00000000 00:00 0
> b7752000-b7755000 r-xp 00000000 08:01 131428 /lib/libdl-2.11.3.so
> b7755000-b7756000 r--p 00002000 08:01 131428 /lib/libdl-2.11.3.so
> b7756000-b7757000 rw-p 00003000 08:01 131428 /lib/libdl-2.11.3.so
> b7774000-b7775000 rw-p 00000000 00:00 0
> b7775000-b7794000 r-xp 00000000 08:01 154467 /lib/ld-2.11.3.so
> b7794000-b7795000 r--p 0001e000 08:01 154467 /lib/ld-2.11.3.so
> b7795000-b7796000 rw-p 0001f000 08:01 154467 /lib/ld-2.11.3.so
> bfa02000-bfa23000 rw-p 00000000 00:00 0 [stack]
> ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso]
> Aborted
>
> And If without -D_FORTIFY_SOURCE=2 in gcc, it works fine, so It's a
> bug in iproute2 which is not conforming to -D_FORTIFY_SOURCE=2
> restrictions.
>
FORTIFY assumes we cant copy a string on alg.u.alg.alg_key !
This completely precludes 0-sized arrays
struct xfrm_algo {
char alg_name[64];
unsigned int alg_key_len; /* in bits */
char alg_key[0];
};
struct {
union {
struct xfrm_algo alg;
struct xfrm_algo_aead aead;
struct xfrm_algo_auth auth;
} u;
char buf[XFRM_ALGO_KEY_BUF_SIZE];
} alg = {};
I would say its a FORTIFY bug. This kind of construct is perfectly
valid.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iproute2: Conforming to -D_FORTIFY_SOURCE=2 restrictions
2011-10-19 11:30 ` Eric Dumazet
@ 2011-10-19 16:50 ` Stephen Hemminger
0 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2011-10-19 16:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Bin Li, netdev
On Wed, 19 Oct 2011 13:30:51 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mercredi 19 octobre 2011 à 17:15 +0800, Bin Li a écrit :
> > Stephen,
> >
> > You can reproduce this issue in 2.6.37 like below. And the previous
> > gdb log is after the install the debuginfo package in SUSE.
> >
> > # ip -6 xfrm state add src 3ffe:501:ffff:ff03:21a:64ff:fe12:e4c1 dst
> > 3ffe:501:ffff:ff05:200:ff:fe00:c1c1 proto ah spi 0x1000 mode transport
> > auth md5 "TAHITEST89ABCDEF"
> >
> > *** buffer overflow detected ***: ip terminated
> > ======= Backtrace: =========
> > /lib/libc.so.6(__fortify_fail+0x40)[0xb76d0070]
> > /lib/libc.so.6(+0xe8e27)[0xb76cde27]
> > /lib/libc.so.6(+0xe8317)[0xb76cd317]
> > ip[0x806d6c4]
> > ip(do_xfrm_state+0x120)[0x806dc70]
> > ip(do_xfrm+0x81)[0x806ad51]
> > ip[0x804c355]
> > ip(main+0x476)[0x804caa6]
> > /lib/libc.so.6(__libc_start_main+0xfe)[0xb75fbc2e]
> > ip[0x804c261]
> > ======= Memory map: ========
> > 08048000-08087000 r-xp 00000000 08:01 4465 /sbin/ip
> > 08087000-08088000 r--p 0003e000 08:01 4465 /sbin/ip
> > 08088000-0808a000 rw-p 0003f000 08:01 4465 /sbin/ip
> > 0808a000-080ad000 rw-p 00000000 00:00 0 [heap]
> > b75c6000-b75e2000 r-xp 00000000 08:01 131084 /lib/libgcc_s.so.1
> > b75e2000-b75e3000 r--p 0001b000 08:01 131084 /lib/libgcc_s.so.1
> > b75e3000-b75e4000 rw-p 0001c000 08:01 131084 /lib/libgcc_s.so.1
> > b75e4000-b75e5000 rw-p 00000000 00:00 0
> > b75e5000-b774b000 r-xp 00000000 08:01 131375 /lib/libc-2.11.3.so
> > b774b000-b774c000 ---p 00166000 08:01 131375 /lib/libc-2.11.3.so
> > b774c000-b774e000 r--p 00166000 08:01 131375 /lib/libc-2.11.3.so
> > b774e000-b774f000 rw-p 00168000 08:01 131375 /lib/libc-2.11.3.so
> > b774f000-b7752000 rw-p 00000000 00:00 0
> > b7752000-b7755000 r-xp 00000000 08:01 131428 /lib/libdl-2.11.3.so
> > b7755000-b7756000 r--p 00002000 08:01 131428 /lib/libdl-2.11.3.so
> > b7756000-b7757000 rw-p 00003000 08:01 131428 /lib/libdl-2.11.3.so
> > b7774000-b7775000 rw-p 00000000 00:00 0
> > b7775000-b7794000 r-xp 00000000 08:01 154467 /lib/ld-2.11.3.so
> > b7794000-b7795000 r--p 0001e000 08:01 154467 /lib/ld-2.11.3.so
> > b7795000-b7796000 rw-p 0001f000 08:01 154467 /lib/ld-2.11.3.so
> > bfa02000-bfa23000 rw-p 00000000 00:00 0 [stack]
> > ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso]
> > Aborted
> >
> > And If without -D_FORTIFY_SOURCE=2 in gcc, it works fine, so It's a
> > bug in iproute2 which is not conforming to -D_FORTIFY_SOURCE=2
> > restrictions.
> >
>
> FORTIFY assumes we cant copy a string on alg.u.alg.alg_key !
>
> This completely precludes 0-sized arrays
>
> struct xfrm_algo {
> char alg_name[64];
> unsigned int alg_key_len; /* in bits */
> char alg_key[0];
> };
>
> struct {
> union {
> struct xfrm_algo alg;
> struct xfrm_algo_aead aead;
> struct xfrm_algo_auth auth;
> } u;
> char buf[XFRM_ALGO_KEY_BUF_SIZE];
> } alg = {};
>
> I would say its a FORTIFY bug. This kind of construct is perfectly
> valid.
Maybe it will handle flexible style arrays.
See also:
http://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
At this time, I won't accept the patch that uses alloca() just to deal
with this FORTIFY bug.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-10-19 16:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-17 7:35 [PATCH] iproute2: Conforming to -D_FORTIFY_SOURCE=2 restrictions Bin Li
2011-10-17 15:23 ` Stephen Hemminger
2011-10-19 9:15 ` Bin Li
2011-10-19 11:30 ` Eric Dumazet
2011-10-19 16:50 ` Stephen Hemminger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.