* "ss -p" segfaults
@ 2015-07-15 14:09 Marc Dietrich
2015-07-15 15:02 ` Vadim Kochan
2015-07-15 15:12 ` Vadim Kochan
0 siblings, 2 replies; 11+ messages in thread
From: Marc Dietrich @ 2015-07-15 14:09 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 6761 bytes --]
Hi,
ss -p segfaults here with some kind of memory corruption:
*** Error in `/work/iproute2/misc/ss': free(): invalid pointer:
0x0000000000623000 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x71c6d)[0x7ffff7885c6d]
/lib64/libc.so.6(+0x771be)[0x7ffff788b1be]
/lib64/libc.so.6(+0x7799b)[0x7ffff788b99b]
/work/iproute2/misc/ss[0x403de1]
/work/iproute2/misc/ss[0x408247]
/work/iproute2/misc/ss[0x403295]
/lib64/libc.so.6(__libc_start_main+0xf0)[0x7ffff7834790]
/work/iproute2/misc/ss[0x4037f9]
======= Memory map: ========
00400000-00416000 r-xp 00000000 00:33 4207305
/work/iproute2/misc/ss
00616000-00617000 r--p 00016000 00:33 4207305
/work/iproute2/misc/ss
00617000-0061b000 rw-p 00017000 00:33 4207305
/work/iproute2/misc/ss
0061b000-0065f000 rw-p 00000000 00:00 0
[heap]
7ffff6f6d000-7ffff6f83000 r-xp 00000000 00:21 16154175
/lib64/libgcc_s.so.1
7ffff6f83000-7ffff7182000 ---p 00016000 00:21 16154175
/lib64/libgcc_s.so.1
7ffff7182000-7ffff7183000 r--p 00015000 00:21 16154175
/lib64/libgcc_s.so.1
7ffff7183000-7ffff7184000 rw-p 00016000 00:21 16154175
/lib64/libgcc_s.so.1
7ffff7184000-7ffff719c000 r-xp 00000000 00:21 16694826
/lib64/libpthread-2.21.so
7ffff719c000-7ffff739b000 ---p 00018000 00:21 16694826
/lib64/libpthread-2.21.so
7ffff739b000-7ffff739c000 r--p 00017000 00:21 16694826
/lib64/libpthread-2.21.so
7ffff739c000-7ffff739d000 rw-p 00018000 00:21 16694826
/lib64/libpthread-2.21.so
7ffff739d000-7ffff73a1000 rw-p 00000000 00:00 0
7ffff73a1000-7ffff73a4000 r-xp 00000000 00:21 16694804
/lib64/libdl-2.21.so
7ffff73a4000-7ffff75a3000 ---p 00003000 00:21 16694804
/lib64/libdl-2.21.so
7ffff75a3000-7ffff75a4000 r--p 00002000 00:21 16694804
/lib64/libdl-2.21.so
7ffff75a4000-7ffff75a5000 rw-p 00003000 00:21 16694804
/lib64/libdl-2.21.so
7ffff75a5000-7ffff7613000 r-xp 00000000 00:21 16153198
/usr/lib64/libpcre.so.1.2.5
7ffff7613000-7ffff7812000 ---p 0006e000 00:21 16153198
/usr/lib64/libpcre.so.1.2.5
7ffff7812000-7ffff7813000 r--p 0006d000 00:21 16153198
/usr/lib64/libpcre.so.1.2.5
7ffff7813000-7ffff7814000 rw-p 0006e000 00:21 16153198
/usr/lib64/libpcre.so.1.2.5
7ffff7814000-7ffff79ad000 r-xp 00000000 00:21 16694798
/lib64/libc-2.21.so
7ffff79ad000-7ffff7bac000 ---p 00199000 00:21 16694798
/lib64/libc-2.21.so
7ffff7bac000-7ffff7bb1000 r--p 00198000 00:21 16694798
/lib64/libc-2.21.so
7ffff7bb1000-7ffff7bb3000 rw-p 0019d000 00:21 16694798
/lib64/libc-2.21.so
7ffff7bb3000-7ffff7bb7000 rw-p 00000000 00:00 0
7ffff7bb7000-7ffff7bd8000 r-xp 00000000 00:21 16155991
/lib64/libselinux.so.1
7ffff7bd8000-7ffff7dd7000 ---p 00021000 00:21 16155991
/lib64/libselinux.so.1
7ffff7dd7000-7ffff7dd8000 r--p 00020000 00:21 16155991
/lib64/libselinux.so.1
7ffff7dd8000-7ffff7dd9000 rw-p 00021000 00:21 16155991
/lib64/libselinux.so.1
7ffff7dd9000-7ffff7ddb000 rw-p 00000000 00:00 0
7ffff7ddb000-7ffff7dfc000 r-xp 00000000 00:21 16694791
/lib64/ld-2.21.so
7ffff7fb5000-7ffff7fba000 rw-p 00000000 00:00 0
7ffff7ff5000-7ffff7ff8000 rw-p 00000000 00:00 0
7ffff7ff8000-7ffff7ffa000 r--p 00000000 00:00 0
[vvar]
7ffff7ffa000-7ffff7ffc000 r-xp 00000000 00:00 0
[vdso]
7ffff7ffc000-7ffff7ffd000 r--p 00021000 00:21 16694791
/lib64/ld-2.21.so
7ffff7ffd000-7ffff7ffe000 rw-p 00022000 00:21 16694791
/lib64/ld-2.21.so
7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0
7ffffffdd000-7ffffffff000 rw-p 00000000 00:00 0
[stack]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0
[vsyscall]
Program received signal SIGABRT, Aborted.
0x00007ffff7847638 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: zypper install libgcc_s1-
debuginfo-5.1.1+r224716-1.2.x86_64 libpcre1-debuginfo-8.37-1.18.x86_64
libselinux1-debuginfo-2.3-5.18.x86_64
(gdb) bt full
#0 0x00007ffff7847638 in raise () from /lib64/libc.so.6
No symbol table info available.
#1 0x00007ffff7848a1a in abort () from /lib64/libc.so.6
No symbol table info available.
#2 0x00007ffff7885c72 in __libc_message () from /lib64/libc.so.6
No symbol table info available.
#3 0x00007ffff788b1be in malloc_printerr () from /lib64/libc.so.6
No symbol table info available.
#4 0x00007ffff788b99b in _int_free () from /lib64/libc.so.6
No symbol table info available.
#5 0x0000000000403de1 in unix_list_free (list=0x6251a0, list@entry=0x645b50)
at ss.c:2516
s = 0x623010
name = 0x272 <error: Cannot access memory at address 0x272>
#6 0x0000000000408247 in unix_show (f=0x61cdf0 <current_filter>) at ss.c:2798
buf = "ffff880205a15b00: 00000003 00000000 00000000 0001 03
84307\n\000/tmp/.X11-
unix/X0\n\000/stdout\n\000adiserver.socket\n\000\n\000c\n\000cket\n", '\000'
<repeats 12 times>,
"q\017A\000\000\000\000\000p\205\201\367\377\177\000\000x\323\377\377\377\177\000\000\067\v\000\000\000\000\000\000h\323\377\377\377\177\000\000\060\374\336\367\377\177\000\000\000U\000\000\005\000\000\000\277\000\000\000;
\212\000\000\000\003\034\177\025\004\000\001"...
name = "\000/tmp/.X11-
unix/X0\000l/stdout\000nadiserver.socket\000\071\000ec\000ocket\000\021@\000\000\000\000\000\377\377\377\377\000\000\000\000\b\321\377\377\377\177\000\000p\205\201\367\377\177\000\000Д\373\367\377\177\000\000\330|
\335\367\377\177\000\000\226\226\204\367\377\177\000\000\370\n@\000\000\000\000\000\070\254a\000\000\000\000"
newformat = 0
cnt = 734
list = <optimized out>
#7 0x0000000000403295 in main (argc=<optimized out>, argv=0x7fffffffd378) at
ss.c:3921
saw_states = <optimized out>
saw_query = 0
do_summary = <optimized out>
dump_tcpdiag = 0x0
filter_fp = 0x0
ch = <optimized out>
state_filter = 2871
(gdb)
git bisect shows bad commit ec4d0d8 (ss: Replace unixstat struct by new
sockstat struct)
This is with a 4.1.2 kernel. Strange thing is, that this segfault does not
happen on my distro kernel (also v4.1) (openSUSE). Seems to be some random
stuff or kernel config problem maybe.
Marc
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-15 14:09 "ss -p" segfaults Marc Dietrich
@ 2015-07-15 15:02 ` Vadim Kochan
2015-07-15 15:12 ` Vadim Kochan
1 sibling, 0 replies; 11+ messages in thread
From: Vadim Kochan @ 2015-07-15 15:02 UTC (permalink / raw)
To: Marc Dietrich; +Cc: netdev
On Wed, Jul 15, 2015 at 04:09:03PM +0200, Marc Dietrich wrote:
> Hi,
>
> ss -p segfaults here with some kind of memory corruption:
>
> *** Error in `/work/iproute2/misc/ss': free(): invalid pointer:
> 0x0000000000623000 ***
> ======= Backtrace: =========
> /lib64/libc.so.6(+0x71c6d)[0x7ffff7885c6d]
> /lib64/libc.so.6(+0x771be)[0x7ffff788b1be]
> /lib64/libc.so.6(+0x7799b)[0x7ffff788b99b]
> /work/iproute2/misc/ss[0x403de1]
> /work/iproute2/misc/ss[0x408247]
> /work/iproute2/misc/ss[0x403295]
> /lib64/libc.so.6(__libc_start_main+0xf0)[0x7ffff7834790]
> /work/iproute2/misc/ss[0x4037f9]
> ======= Memory map: ========
> 00400000-00416000 r-xp 00000000 00:33 4207305
> /work/iproute2/misc/ss
> 00616000-00617000 r--p 00016000 00:33 4207305
> /work/iproute2/misc/ss
> 00617000-0061b000 rw-p 00017000 00:33 4207305
> /work/iproute2/misc/ss
> 0061b000-0065f000 rw-p 00000000 00:00 0
> [heap]
> 7ffff6f6d000-7ffff6f83000 r-xp 00000000 00:21 16154175
> /lib64/libgcc_s.so.1
> 7ffff6f83000-7ffff7182000 ---p 00016000 00:21 16154175
> /lib64/libgcc_s.so.1
> 7ffff7182000-7ffff7183000 r--p 00015000 00:21 16154175
> /lib64/libgcc_s.so.1
> 7ffff7183000-7ffff7184000 rw-p 00016000 00:21 16154175
> /lib64/libgcc_s.so.1
> 7ffff7184000-7ffff719c000 r-xp 00000000 00:21 16694826
> /lib64/libpthread-2.21.so
> 7ffff719c000-7ffff739b000 ---p 00018000 00:21 16694826
> /lib64/libpthread-2.21.so
> 7ffff739b000-7ffff739c000 r--p 00017000 00:21 16694826
> /lib64/libpthread-2.21.so
> 7ffff739c000-7ffff739d000 rw-p 00018000 00:21 16694826
> /lib64/libpthread-2.21.so
> 7ffff739d000-7ffff73a1000 rw-p 00000000 00:00 0
> 7ffff73a1000-7ffff73a4000 r-xp 00000000 00:21 16694804
> /lib64/libdl-2.21.so
> 7ffff73a4000-7ffff75a3000 ---p 00003000 00:21 16694804
> /lib64/libdl-2.21.so
> 7ffff75a3000-7ffff75a4000 r--p 00002000 00:21 16694804
> /lib64/libdl-2.21.so
> 7ffff75a4000-7ffff75a5000 rw-p 00003000 00:21 16694804
> /lib64/libdl-2.21.so
> 7ffff75a5000-7ffff7613000 r-xp 00000000 00:21 16153198
> /usr/lib64/libpcre.so.1.2.5
> 7ffff7613000-7ffff7812000 ---p 0006e000 00:21 16153198
> /usr/lib64/libpcre.so.1.2.5
> 7ffff7812000-7ffff7813000 r--p 0006d000 00:21 16153198
> /usr/lib64/libpcre.so.1.2.5
> 7ffff7813000-7ffff7814000 rw-p 0006e000 00:21 16153198
> /usr/lib64/libpcre.so.1.2.5
> 7ffff7814000-7ffff79ad000 r-xp 00000000 00:21 16694798
> /lib64/libc-2.21.so
> 7ffff79ad000-7ffff7bac000 ---p 00199000 00:21 16694798
> /lib64/libc-2.21.so
> 7ffff7bac000-7ffff7bb1000 r--p 00198000 00:21 16694798
> /lib64/libc-2.21.so
> 7ffff7bb1000-7ffff7bb3000 rw-p 0019d000 00:21 16694798
> /lib64/libc-2.21.so
> 7ffff7bb3000-7ffff7bb7000 rw-p 00000000 00:00 0
> 7ffff7bb7000-7ffff7bd8000 r-xp 00000000 00:21 16155991
> /lib64/libselinux.so.1
> 7ffff7bd8000-7ffff7dd7000 ---p 00021000 00:21 16155991
> /lib64/libselinux.so.1
> 7ffff7dd7000-7ffff7dd8000 r--p 00020000 00:21 16155991
> /lib64/libselinux.so.1
> 7ffff7dd8000-7ffff7dd9000 rw-p 00021000 00:21 16155991
> /lib64/libselinux.so.1
> 7ffff7dd9000-7ffff7ddb000 rw-p 00000000 00:00 0
> 7ffff7ddb000-7ffff7dfc000 r-xp 00000000 00:21 16694791
> /lib64/ld-2.21.so
> 7ffff7fb5000-7ffff7fba000 rw-p 00000000 00:00 0
> 7ffff7ff5000-7ffff7ff8000 rw-p 00000000 00:00 0
> 7ffff7ff8000-7ffff7ffa000 r--p 00000000 00:00 0
> [vvar]
> 7ffff7ffa000-7ffff7ffc000 r-xp 00000000 00:00 0
> [vdso]
> 7ffff7ffc000-7ffff7ffd000 r--p 00021000 00:21 16694791
> /lib64/ld-2.21.so
> 7ffff7ffd000-7ffff7ffe000 rw-p 00022000 00:21 16694791
> /lib64/ld-2.21.so
> 7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0
> 7ffffffdd000-7ffffffff000 rw-p 00000000 00:00 0
> [stack]
> ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0
> [vsyscall]
>
> Program received signal SIGABRT, Aborted.
> 0x00007ffff7847638 in raise () from /lib64/libc.so.6
> Missing separate debuginfos, use: zypper install libgcc_s1-
> debuginfo-5.1.1+r224716-1.2.x86_64 libpcre1-debuginfo-8.37-1.18.x86_64
> libselinux1-debuginfo-2.3-5.18.x86_64
> (gdb) bt full
> #0 0x00007ffff7847638 in raise () from /lib64/libc.so.6
> No symbol table info available.
> #1 0x00007ffff7848a1a in abort () from /lib64/libc.so.6
> No symbol table info available.
> #2 0x00007ffff7885c72 in __libc_message () from /lib64/libc.so.6
> No symbol table info available.
> #3 0x00007ffff788b1be in malloc_printerr () from /lib64/libc.so.6
> No symbol table info available.
> #4 0x00007ffff788b99b in _int_free () from /lib64/libc.so.6
> No symbol table info available.
> #5 0x0000000000403de1 in unix_list_free (list=0x6251a0, list@entry=0x645b50)
> at ss.c:2516
> s = 0x623010
> name = 0x272 <error: Cannot access memory at address 0x272>
> #6 0x0000000000408247 in unix_show (f=0x61cdf0 <current_filter>) at ss.c:2798
> buf = "ffff880205a15b00: 00000003 00000000 00000000 0001 03
> 84307\n\000/tmp/.X11-
> unix/X0\n\000/stdout\n\000adiserver.socket\n\000\n\000c\n\000cket\n", '\000'
> <repeats 12 times>,
> "q\017A\000\000\000\000\000p\205\201\367\377\177\000\000x\323\377\377\377\177\000\000\067\v\000\000\000\000\000\000h\323\377\377\377\177\000\000\060\374\336\367\377\177\000\000\000U\000\000\005\000\000\000\277\000\000\000;
> \212\000\000\000\003\034\177\025\004\000\001"...
> name = "\000/tmp/.X11-
> unix/X0\000l/stdout\000nadiserver.socket\000\071\000ec\000ocket\000\021@\000\000\000\000\000\377\377\377\377\000\000\000\000\b\321\377\377\377\177\000\000p\205\201\367\377\177\000\000Д\373\367\377\177\000\000\330|
> \335\367\377\177\000\000\226\226\204\367\377\177\000\000\370\n@\000\000\000\000\000\070\254a\000\000\000\000"
> newformat = 0
> cnt = 734
> list = <optimized out>
> #7 0x0000000000403295 in main (argc=<optimized out>, argv=0x7fffffffd378) at
> ss.c:3921
> saw_states = <optimized out>
> saw_query = 0
> do_summary = <optimized out>
> dump_tcpdiag = 0x0
> filter_fp = 0x0
> ch = <optimized out>
> state_filter = 2871
> (gdb)
>
> git bisect shows bad commit ec4d0d8 (ss: Replace unixstat struct by new
> sockstat struct)
>
> This is with a 4.1.2 kernel. Strange thing is, that this segfault does not
> happen on my distro kernel (also v4.1) (openSUSE). Seems to be some random
> stuff or kernel config problem maybe.
>
> Marc
Hm, I can reproduce this only when use proc:
$ PROC_ROOT=/proc ss -ap
I will fix this.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-15 14:09 "ss -p" segfaults Marc Dietrich
2015-07-15 15:02 ` Vadim Kochan
@ 2015-07-15 15:12 ` Vadim Kochan
2015-07-15 16:49 ` Rustad, Mark D
1 sibling, 1 reply; 11+ messages in thread
From: Vadim Kochan @ 2015-07-15 15:12 UTC (permalink / raw)
To: Marc Dietrich; +Cc: netdev
On Wed, Jul 15, 2015 at 04:09:03PM +0200, Marc Dietrich wrote:
> Hi,
>
> ss -p segfaults here with some kind of memory corruption:
>
> *** Error in `/work/iproute2/misc/ss': free(): invalid pointer:
> 0x0000000000623000 ***
> ======= Backtrace: =========
> /lib64/libc.so.6(+0x71c6d)[0x7ffff7885c6d]
> /lib64/libc.so.6(+0x771be)[0x7ffff788b1be]
> /lib64/libc.so.6(+0x7799b)[0x7ffff788b99b]
> /work/iproute2/misc/ss[0x403de1]
> /work/iproute2/misc/ss[0x408247]
> /work/iproute2/misc/ss[0x403295]
> /lib64/libc.so.6(__libc_start_main+0xf0)[0x7ffff7834790]
> /work/iproute2/misc/ss[0x4037f9]
> ======= Memory map: ========
> 00400000-00416000 r-xp 00000000 00:33 4207305
> /work/iproute2/misc/ss
> 00616000-00617000 r--p 00016000 00:33 4207305
> /work/iproute2/misc/ss
> 00617000-0061b000 rw-p 00017000 00:33 4207305
> /work/iproute2/misc/ss
> 0061b000-0065f000 rw-p 00000000 00:00 0
> [heap]
> 7ffff6f6d000-7ffff6f83000 r-xp 00000000 00:21 16154175
> /lib64/libgcc_s.so.1
> 7ffff6f83000-7ffff7182000 ---p 00016000 00:21 16154175
> /lib64/libgcc_s.so.1
> 7ffff7182000-7ffff7183000 r--p 00015000 00:21 16154175
> /lib64/libgcc_s.so.1
> 7ffff7183000-7ffff7184000 rw-p 00016000 00:21 16154175
> /lib64/libgcc_s.so.1
> 7ffff7184000-7ffff719c000 r-xp 00000000 00:21 16694826
> /lib64/libpthread-2.21.so
> 7ffff719c000-7ffff739b000 ---p 00018000 00:21 16694826
> /lib64/libpthread-2.21.so
> 7ffff739b000-7ffff739c000 r--p 00017000 00:21 16694826
> /lib64/libpthread-2.21.so
> 7ffff739c000-7ffff739d000 rw-p 00018000 00:21 16694826
> /lib64/libpthread-2.21.so
> 7ffff739d000-7ffff73a1000 rw-p 00000000 00:00 0
> 7ffff73a1000-7ffff73a4000 r-xp 00000000 00:21 16694804
> /lib64/libdl-2.21.so
> 7ffff73a4000-7ffff75a3000 ---p 00003000 00:21 16694804
> /lib64/libdl-2.21.so
> 7ffff75a3000-7ffff75a4000 r--p 00002000 00:21 16694804
> /lib64/libdl-2.21.so
> 7ffff75a4000-7ffff75a5000 rw-p 00003000 00:21 16694804
> /lib64/libdl-2.21.so
> 7ffff75a5000-7ffff7613000 r-xp 00000000 00:21 16153198
> /usr/lib64/libpcre.so.1.2.5
> 7ffff7613000-7ffff7812000 ---p 0006e000 00:21 16153198
> /usr/lib64/libpcre.so.1.2.5
> 7ffff7812000-7ffff7813000 r--p 0006d000 00:21 16153198
> /usr/lib64/libpcre.so.1.2.5
> 7ffff7813000-7ffff7814000 rw-p 0006e000 00:21 16153198
> /usr/lib64/libpcre.so.1.2.5
> 7ffff7814000-7ffff79ad000 r-xp 00000000 00:21 16694798
> /lib64/libc-2.21.so
> 7ffff79ad000-7ffff7bac000 ---p 00199000 00:21 16694798
> /lib64/libc-2.21.so
> 7ffff7bac000-7ffff7bb1000 r--p 00198000 00:21 16694798
> /lib64/libc-2.21.so
> 7ffff7bb1000-7ffff7bb3000 rw-p 0019d000 00:21 16694798
> /lib64/libc-2.21.so
> 7ffff7bb3000-7ffff7bb7000 rw-p 00000000 00:00 0
> 7ffff7bb7000-7ffff7bd8000 r-xp 00000000 00:21 16155991
> /lib64/libselinux.so.1
> 7ffff7bd8000-7ffff7dd7000 ---p 00021000 00:21 16155991
> /lib64/libselinux.so.1
> 7ffff7dd7000-7ffff7dd8000 r--p 00020000 00:21 16155991
> /lib64/libselinux.so.1
> 7ffff7dd8000-7ffff7dd9000 rw-p 00021000 00:21 16155991
> /lib64/libselinux.so.1
> 7ffff7dd9000-7ffff7ddb000 rw-p 00000000 00:00 0
> 7ffff7ddb000-7ffff7dfc000 r-xp 00000000 00:21 16694791
> /lib64/ld-2.21.so
> 7ffff7fb5000-7ffff7fba000 rw-p 00000000 00:00 0
> 7ffff7ff5000-7ffff7ff8000 rw-p 00000000 00:00 0
> 7ffff7ff8000-7ffff7ffa000 r--p 00000000 00:00 0
> [vvar]
> 7ffff7ffa000-7ffff7ffc000 r-xp 00000000 00:00 0
> [vdso]
> 7ffff7ffc000-7ffff7ffd000 r--p 00021000 00:21 16694791
> /lib64/ld-2.21.so
> 7ffff7ffd000-7ffff7ffe000 rw-p 00022000 00:21 16694791
> /lib64/ld-2.21.so
> 7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0
> 7ffffffdd000-7ffffffff000 rw-p 00000000 00:00 0
> [stack]
> ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0
> [vsyscall]
>
> Program received signal SIGABRT, Aborted.
> 0x00007ffff7847638 in raise () from /lib64/libc.so.6
> Missing separate debuginfos, use: zypper install libgcc_s1-
> debuginfo-5.1.1+r224716-1.2.x86_64 libpcre1-debuginfo-8.37-1.18.x86_64
> libselinux1-debuginfo-2.3-5.18.x86_64
> (gdb) bt full
> #0 0x00007ffff7847638 in raise () from /lib64/libc.so.6
> No symbol table info available.
> #1 0x00007ffff7848a1a in abort () from /lib64/libc.so.6
> No symbol table info available.
> #2 0x00007ffff7885c72 in __libc_message () from /lib64/libc.so.6
> No symbol table info available.
> #3 0x00007ffff788b1be in malloc_printerr () from /lib64/libc.so.6
> No symbol table info available.
> #4 0x00007ffff788b99b in _int_free () from /lib64/libc.so.6
> No symbol table info available.
> #5 0x0000000000403de1 in unix_list_free (list=0x6251a0, list@entry=0x645b50)
> at ss.c:2516
> s = 0x623010
> name = 0x272 <error: Cannot access memory at address 0x272>
> #6 0x0000000000408247 in unix_show (f=0x61cdf0 <current_filter>) at ss.c:2798
> buf = "ffff880205a15b00: 00000003 00000000 00000000 0001 03
> 84307\n\000/tmp/.X11-
> unix/X0\n\000/stdout\n\000adiserver.socket\n\000\n\000c\n\000cket\n", '\000'
> <repeats 12 times>,
> "q\017A\000\000\000\000\000p\205\201\367\377\177\000\000x\323\377\377\377\177\000\000\067\v\000\000\000\000\000\000h\323\377\377\377\177\000\000\060\374\336\367\377\177\000\000\000U\000\000\005\000\000\000\277\000\000\000;
> \212\000\000\000\003\034\177\025\004\000\001"...
> name = "\000/tmp/.X11-
> unix/X0\000l/stdout\000nadiserver.socket\000\071\000ec\000ocket\000\021@\000\000\000\000\000\377\377\377\377\000\000\000\000\b\321\377\377\377\177\000\000p\205\201\367\377\177\000\000Д\373\367\377\177\000\000\330|
> \335\367\377\177\000\000\226\226\204\367\377\177\000\000\370\n@\000\000\000\000\000\070\254a\000\000\000\000"
> newformat = 0
> cnt = 734
> list = <optimized out>
> #7 0x0000000000403295 in main (argc=<optimized out>, argv=0x7fffffffd378) at
> ss.c:3921
> saw_states = <optimized out>
> saw_query = 0
> do_summary = <optimized out>
> dump_tcpdiag = 0x0
> filter_fp = 0x0
> ch = <optimized out>
> state_filter = 2871
> (gdb)
>
> git bisect shows bad commit ec4d0d8 (ss: Replace unixstat struct by new
> sockstat struct)
>
> This is with a 4.1.2 kernel. Strange thing is, that this segfault does not
> happen on my distro kernel (also v4.1) (openSUSE). Seems to be some random
> stuff or kernel config problem maybe.
>
> Marc
Would you please check this fix ?
diff --git a/misc/ss.c b/misc/ss.c
index 03f92fa..3a826e4 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix *prefix, char **ptr)
static inline char *sock_addr_get_str(const inet_prefix *prefix)
{
- char *tmp ;
- memcpy(&tmp, prefix->data, sizeof(char *));
+ char *tmp;
+ memcpy(&tmp, &prefix->data[0], sizeof(char *));
return tmp;
}
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-15 15:12 ` Vadim Kochan
@ 2015-07-15 16:49 ` Rustad, Mark D
2015-07-15 18:52 ` Rustad, Mark D
0 siblings, 1 reply; 11+ messages in thread
From: Rustad, Mark D @ 2015-07-15 16:49 UTC (permalink / raw)
To: Vadim Kochan; +Cc: Marc Dietrich, netdev@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 765 bytes --]
> On Jul 15, 2015, at 8:12 AM, Vadim Kochan <vadim4j@gmail.com> wrote:
> Would you please check this fix ?
>
> diff --git a/misc/ss.c b/misc/ss.c
> index 03f92fa..3a826e4 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix *prefix, char **ptr)
>
> static inline char *sock_addr_get_str(const inet_prefix *prefix)
> {
> - char *tmp ;
> - memcpy(&tmp, prefix->data, sizeof(char *));
> + char *tmp;
> + memcpy(&tmp, &prefix->data[0], sizeof(char *));
> return tmp;
> }
That surely is not a fix! The destination of the memcpy is the address of an uninitialized stack variable! Both versions are equally bad.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-15 16:49 ` Rustad, Mark D
@ 2015-07-15 18:52 ` Rustad, Mark D
2015-07-15 18:57 ` Vadim Kochan
0 siblings, 1 reply; 11+ messages in thread
From: Rustad, Mark D @ 2015-07-15 18:52 UTC (permalink / raw)
To: Vadim Kochan; +Cc: Marc Dietrich, netdev@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]
> On Jul 15, 2015, at 9:49 AM, Rustad, Mark D <mark.d.rustad@intel.com> wrote:
>
>> On Jul 15, 2015, at 8:12 AM, Vadim Kochan <vadim4j@gmail.com> wrote:
>> Would you please check this fix ?
>>
>> diff --git a/misc/ss.c b/misc/ss.c
>> index 03f92fa..3a826e4 100644
>> --- a/misc/ss.c
>> +++ b/misc/ss.c
>> @@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix *prefix, char **ptr)
>>
>> static inline char *sock_addr_get_str(const inet_prefix *prefix)
>> {
>> - char *tmp ;
>> - memcpy(&tmp, prefix->data, sizeof(char *));
>> + char *tmp;
>> + memcpy(&tmp, &prefix->data[0], sizeof(char *));
>> return tmp;
>> }
>
> That surely is not a fix! The destination of the memcpy is the address of an uninitialized stack variable! Both versions are equally bad.
I probably over-reacted, but using memcpy to access a pointer in this way is just ugly. For one thing, it circumvents any sanity-checking that the compiler can do. And changing the prefix->data to &prefix->data[0] should be exactly the same thing and therefore should not fix anything. Anyway, never mind that.
Looking at more of the code, it looks to me like the the string pointer in data can sometimes point to a literal string instead of allocated memory when proc is in use. Free would not be happy with that. Look at the use of variable peer in function unix_stats_print.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-15 18:52 ` Rustad, Mark D
@ 2015-07-15 18:57 ` Vadim Kochan
2015-07-15 22:22 ` Vadim Kochan
0 siblings, 1 reply; 11+ messages in thread
From: Vadim Kochan @ 2015-07-15 18:57 UTC (permalink / raw)
To: Rustad, Mark D; +Cc: Vadim Kochan, Marc Dietrich, netdev@vger.kernel.org
On Wed, Jul 15, 2015 at 06:52:49PM +0000, Rustad, Mark D wrote:
> > On Jul 15, 2015, at 9:49 AM, Rustad, Mark D <mark.d.rustad@intel.com> wrote:
> >
> >> On Jul 15, 2015, at 8:12 AM, Vadim Kochan <vadim4j@gmail.com> wrote:
> >> Would you please check this fix ?
> >>
> >> diff --git a/misc/ss.c b/misc/ss.c
> >> index 03f92fa..3a826e4 100644
> >> --- a/misc/ss.c
> >> +++ b/misc/ss.c
> >> @@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix *prefix, char **ptr)
> >>
> >> static inline char *sock_addr_get_str(const inet_prefix *prefix)
> >> {
> >> - char *tmp ;
> >> - memcpy(&tmp, prefix->data, sizeof(char *));
> >> + char *tmp;
> >> + memcpy(&tmp, &prefix->data[0], sizeof(char *));
> >> return tmp;
> >> }
> >
> > That surely is not a fix! The destination of the memcpy is the address of an uninitialized stack variable! Both versions are equally bad.
>
> I probably over-reacted, but using memcpy to access a pointer in this way is just ugly. For one thing, it circumvents any sanity-checking that the compiler can do. And changing the prefix->data to &prefix->data[0] should be exactly the same thing and therefore should not fix anything. Anyway, never mind that.
>
> Looking at more of the code, it looks to me like the the string pointer in data can sometimes point to a literal string instead of allocated memory when proc is in use. Free would not be happy with that. Look at the use of variable peer in function unix_stats_print.
>
Yes that right, I am already looking on this ...
> --
> Mark Rustad, Networking Division, Intel Corporation
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-15 18:57 ` Vadim Kochan
@ 2015-07-15 22:22 ` Vadim Kochan
2015-07-16 6:37 ` Marc Dietrich
0 siblings, 1 reply; 11+ messages in thread
From: Vadim Kochan @ 2015-07-15 22:22 UTC (permalink / raw)
To: Rustad, Mark D; +Cc: Vadim Kochan, Marc Dietrich, netdev@vger.kernel.org
On Wed, Jul 15, 2015 at 09:57:51PM +0300, Vadim Kochan wrote:
> On Wed, Jul 15, 2015 at 06:52:49PM +0000, Rustad, Mark D wrote:
> > > On Jul 15, 2015, at 9:49 AM, Rustad, Mark D <mark.d.rustad@intel.com> wrote:
> > >
> > >> On Jul 15, 2015, at 8:12 AM, Vadim Kochan <vadim4j@gmail.com> wrote:
> > >> Would you please check this fix ?
> > >>
> > >> diff --git a/misc/ss.c b/misc/ss.c
> > >> index 03f92fa..3a826e4 100644
> > >> --- a/misc/ss.c
> > >> +++ b/misc/ss.c
> > >> @@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix *prefix, char **ptr)
> > >>
> > >> static inline char *sock_addr_get_str(const inet_prefix *prefix)
> > >> {
> > >> - char *tmp ;
> > >> - memcpy(&tmp, prefix->data, sizeof(char *));
> > >> + char *tmp;
> > >> + memcpy(&tmp, &prefix->data[0], sizeof(char *));
> > >> return tmp;
> > >> }
> > >
> > > That surely is not a fix! The destination of the memcpy is the address of an uninitialized stack variable! Both versions are equally bad.
> >
> > I probably over-reacted, but using memcpy to access a pointer in this way is just ugly. For one thing, it circumvents any sanity-checking that the compiler can do. And changing the prefix->data to &prefix->data[0] should be exactly the same thing and therefore should not fix anything. Anyway, never mind that.
> >
> > Looking at more of the code, it looks to me like the the string pointer in data can sometimes point to a literal string instead of allocated memory when proc is in use. Free would not be happy with that. Look at the use of variable peer in function unix_stats_print.
> >
> Yes that right, I am already looking on this ...
> > --
> > Mark Rustad, Networking Division, Intel Corporation
> >
I did partially revert of the buggy commit and it does not crash, but I will do
more testing, and after will send the patch and will try to prepare some
test scripts for ss.
The crash appears only if to dump processes info from /proc, which might
be caused that netlink stats returned error, probably by wrong request
(not supported attribute or flag ?).
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-15 22:22 ` Vadim Kochan
@ 2015-07-16 6:37 ` Marc Dietrich
0 siblings, 0 replies; 11+ messages in thread
From: Marc Dietrich @ 2015-07-16 6:37 UTC (permalink / raw)
To: Vadim Kochan; +Cc: Rustad, Mark D, netdev@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]
Am Donnerstag, 16. Juli 2015, 01:22:38 schrieb Vadim Kochan:
> On Wed, Jul 15, 2015 at 09:57:51PM +0300, Vadim Kochan wrote:
> > On Wed, Jul 15, 2015 at 06:52:49PM +0000, Rustad, Mark D wrote:
> > > > On Jul 15, 2015, at 9:49 AM, Rustad, Mark D <mark.d.rustad@intel.com>
wrote:
> > > >> On Jul 15, 2015, at 8:12 AM, Vadim Kochan <vadim4j@gmail.com> wrote:
> > > >> Would you please check this fix ?
> > > >>
> > > >> diff --git a/misc/ss.c b/misc/ss.c
> > > >> index 03f92fa..3a826e4 100644
> > > >> --- a/misc/ss.c
> > > >> +++ b/misc/ss.c
> > > >> @@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix
> > > >> *prefix, char **ptr)
> > > >>
> > > >> static inline char *sock_addr_get_str(const inet_prefix *prefix)
> > > >> {
> > > >> - char *tmp ;
> > > >> - memcpy(&tmp, prefix->data, sizeof(char *));
> > > >> + char *tmp;
> > > >> + memcpy(&tmp, &prefix->data[0], sizeof(char *));
> > > >>
> > > >> return tmp;
> > > >>
> > > >> }
> > > >
> > > > That surely is not a fix! The destination of the memcpy is the address
> > > > of an uninitialized stack variable! Both versions are equally bad.> >
> > > I probably over-reacted, but using memcpy to access a pointer in this
> > > way is just ugly. For one thing, it circumvents any sanity-checking
> > > that the compiler can do. And changing the prefix->data to
> > > &prefix->data[0] should be exactly the same thing and therefore should
> > > not fix anything. Anyway, never mind that.
> > >
> > > Looking at more of the code, it looks to me like the the string pointer
> > > in data can sometimes point to a literal string instead of allocated
> > > memory when proc is in use. Free would not be happy with that. Look at
> > > the use of variable peer in function unix_stats_print.>
> > Yes that right, I am already looking on this ...
> >
> > > --
> > > Mark Rustad, Networking Division, Intel Corporation
>
> I did partially revert of the buggy commit and it does not crash, but I will
> do more testing, and after will send the patch and will try to prepare some
> test scripts for ss.
>
> The crash appears only if to dump processes info from /proc, which might
> be caused that netlink stats returned error, probably by wrong request
> (not supported attribute or flag ?).
the reason it uses proc for me is my self compiled (and trimmed) kernel which
disabled all *_diag modules which seem to be required by ss. I didn't know
this. On the other hand, I managed to find a bug this way :-)
Marc
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <55AC9E8C.7040200@openmailbox.org>]
* "ss -p" segfaults
[not found] <55AC9E8C.7040200@openmailbox.org>
@ 2015-07-20 17:31 ` j.ps
2015-07-20 18:14 ` Andreas Schwab
0 siblings, 1 reply; 11+ messages in thread
From: j.ps @ 2015-07-20 17:31 UTC (permalink / raw)
To: netdev
Please find attached one simple patch for the code 4.1.0/4.1.1.
Essentially all that is needed to get rid of this issue is the
addition of:
memset(u, 0, sizeof(*u));
after:
if (!(u = malloc(sizeof(*u))))
break;
During the analysis of this issue I also found some other situations
(strcpy and sprintf uses) that potentially produce the same results.
Corrections are also added, all with full comments.
Regards,
Jose Santos
--- iproute2-4.1.1-orig/misc/ss.c 2015-07-06 22:57:34.000000000 +0100
+++ iproute2-4.1.1/misc/ss.c 2015-07-19 12:16:25.000000000 +0100
@@ -428,9 +428,12 @@
while (cnt != USER_ENT_HASH_SIZE) {
p = user_ent_hash[cnt];
while (p) {
- free(p->process);
- free(p->process_ctx);
- free(p->socket_ctx);
+ if (p->process)
+ free(p->process);
+ if (p->process_ctx)
+ free(p->process_ctx);
+ if (p->socket_ctx)
+ free(p->socket_ctx);
p_next = p->next;
free(p);
p = p_next;
@@ -456,7 +459,21 @@
user_ent_hash_build_init = 1;
- strcpy(name, root);
+ /* strcpy is really dangerous! in this case if we're going to
+ copy an unknown input size from getenv("PROC_ROOT") to a
+ fixed size buffer name[1024] we're going to get troubles.
+ If the size of a manipulated "PROC_ROOT" > size of name we
+ will have a buffer overrun smashing the stack, overwriting
+ other local variables and/or return address, etc... */
+
+ memset(name, 0, sizeof(name));
+
+ strncpy(name, root, 512);
+ /* Chose this value ^^^ (maybe too large) just to avoid buffer
+ overflow if sizeof getenv("PROC_ROOT") > sizeof(name) and
+ to allow the name composition that follows below to fit in
+ the remaining space. */
+
if (strlen(name) == 0 || name[strlen(name)-1] != '/')
strcat(name, "/");
@@ -480,7 +497,7 @@
if (getpidcon(pid, &pid_context) != 0)
pid_context = strdup(no_ctx);
- sprintf(name + nameoff, "%d/fd/", pid);
+ snprintf(name + nameoff, sizeof(name) - nameoff, "%d/fd/", pid);
pos = strlen(name);
if ((dir1 = opendir(name)) == NULL)
continue;
@@ -499,7 +516,7 @@
if (sscanf(d1->d_name, "%d%c", &fd, &crap) != 1)
continue;
- sprintf(name+pos, "%d", fd);
+ snprintf(name+pos, sizeof(name) - pos, "%d", fd);
link_len = readlink(name, lnk, sizeof(lnk)-1);
if (link_len == -1)
@@ -529,9 +546,16 @@
}
user_ent_add(ino, p, pid, fd,
pid_context, sock_context);
- free(sock_context);
- }
- free(pid_context);
+ /* memory was allocated conditionally so
+ freeing it should go the same way (even
+ though the actual code implies that in
+ this case it will always be allocated). */
+ if (sock_context)
+ free(sock_context);
+ }
+ /* same as above */
+ if (pid_context)
+ free(pid_context);
closedir(dir1);
}
closedir(dir);
@@ -2722,6 +2746,13 @@
if (!(u = malloc(sizeof(*u))))
break;
+ /* The following memset of 'u' struct is crucial to avoid a segfault
+ when freeing memory 'free(name)' at 'unix_list_free()'. In fact,
+ without this 'initialization', testing 'if (name)' at line 2513
+ will most likely return true even if 'name' isn't allocated. */
+
+ memset(u, 0, sizeof(*u));
+
if (sscanf(buf, "%x: %x %x %x %x %x %d %s",
&u->rport, &u->rq, &u->wq, &flags, &u->type,
&u->state, &u->ino, name) < 8)
@@ -3064,11 +3095,12 @@
strncpy(procname, "kernel", 6);
} else if (pid > 0) {
FILE *fp;
- sprintf(procname, "%s/%d/stat",
+ snprintf(procname, sizeof(procname), "%s/%d/stat",
getenv("PROC_ROOT") ? : "/proc", pid);
if ((fp = fopen(procname, "r")) != NULL) {
if (fscanf(fp, "%*d (%[^)])", procname) == 1) {
- sprintf(procname+strlen(procname), "/%d", pid);
+ snprintf(procname+strlen(procname),
+ sizeof(procname)-strlen(procname), "/%d", pid);
done = 1;
}
fclose(fp);
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-20 17:31 ` j.ps
@ 2015-07-20 18:14 ` Andreas Schwab
0 siblings, 0 replies; 11+ messages in thread
From: Andreas Schwab @ 2015-07-20 18:14 UTC (permalink / raw)
To: j.ps@openmailbox.org; +Cc: netdev
"j.ps@openmailbox.org" <j.ps@openmailbox.org> writes:
> - free(p->process);
> - free(p->process_ctx);
> - free(p->socket_ctx);
> + if (p->process)
> + free(p->process);
> + if (p->process_ctx)
> + free(p->process_ctx);
> + if (p->socket_ctx)
> + free(p->socket_ctx);
free(NULL) is a no-op.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Segmentation fault in iproute2 ss -p (versions 4.0.0, 4.1.0 and 4.1.1)
@ 2015-07-21 9:50 j.ps
2015-07-21 9:54 ` "ss -p" segfaults j.ps
0 siblings, 1 reply; 11+ messages in thread
From: j.ps @ 2015-07-21 9:50 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Patch for 4.1.1.
Essentially all that is needed to get rid of this issue is the
addition of:
memset(u, 0, sizeof(*u));
after:
if (!(u = malloc(sizeof(*u))))
break;
Also patched some other situations (strcpy and sprintf uses) that
potentially produce the same results.
Note: As far as I know strlcpy isn't (yet) available in glibc.
Signed-off-by: Jose P Santos <j.ps@openmailbox.org>
--- iproute2-4.1.1/misc/ss.c.orig 2015-07-06 22:57:34.000000000 +0100
+++ iproute2-4.1.1/misc/ss.c 2015-07-21 10:26:45.000000000 +0100
@@ -456,7 +456,10 @@ static void user_ent_hash_build(void)
user_ent_hash_build_init = 1;
- strcpy(name, root);
+ /* Avoid buffer overrun if input size from PROC_ROOT > name */
+ memset(name, 0, sizeof(name));
+ strncpy(name, root, sizeof(name)-2);
+
if (strlen(name) == 0 || name[strlen(name)-1] != '/')
strcat(name, "/");
@@ -480,7 +483,7 @@ static void user_ent_hash_build(void)
if (getpidcon(pid, &pid_context) != 0)
pid_context = strdup(no_ctx);
- sprintf(name + nameoff, "%d/fd/", pid);
+ snprintf(name + nameoff, sizeof(name) - nameoff, "%d/fd/", pid);
pos = strlen(name);
if ((dir1 = opendir(name)) == NULL)
continue;
@@ -499,7 +502,7 @@ static void user_ent_hash_build(void)
if (sscanf(d1->d_name, "%d%c", &fd, &crap) != 1)
continue;
- sprintf(name+pos, "%d", fd);
+ snprintf(name+pos, sizeof(name) - pos, "%d", fd);
link_len = readlink(name, lnk, sizeof(lnk)-1);
if (link_len == -1)
@@ -2722,6 +2725,11 @@ static int unix_show(struct filter *f)
if (!(u = malloc(sizeof(*u))))
break;
+ /* Zero initialization of 'u' struct avoids a segfault
+ * when freeing memory 'free(name)' at 'unix_list_free()'.
+ */
+ memset(u, 0, sizeof(*u));
+
if (sscanf(buf, "%x: %x %x %x %x %x %d %s",
&u->rport, &u->rq, &u->wq, &flags, &u->type,
&u->state, &u->ino, name) < 8)
@@ -3064,11 +3072,13 @@ static int netlink_show_one(struct filte
strncpy(procname, "kernel", 6);
} else if (pid > 0) {
FILE *fp;
- sprintf(procname, "%s/%d/stat",
+ snprintf(procname, sizeof(procname), "%s/%d/stat",
getenv("PROC_ROOT") ? : "/proc", pid);
if ((fp = fopen(procname, "r")) != NULL) {
if (fscanf(fp, "%*d (%[^)])", procname) == 1) {
- sprintf(procname+strlen(procname), "/%d", pid);
+ snprintf(procname+strlen(procname),
+ sizeof(procname)-strlen(procname),
+ "/%d", pid);
done = 1;
}
fclose(fp);
On 2015-07-20 20:09, Stephen Hemminger wrote:
> Patches are always appreciated and this looks like a real bug.
> But before I can accept it there are a couple of small
> changes needed.
>
> 1. There is no need to check for NULL when calling free().
> Glibc free is documented to accept NULL as a valid request
> and do nothing.
>
> 2. Please add a Signed-off-by: line with a real name.
> Signed-off-by has legal meaning for the Developer's Certificate of Origin
> see kernel documentation if you need more explaination.
>
> 3. Although what you found is important, giving a full paragraph
> of personal comment about it is not required. The point is software
> should read like one source independent of who the authors are.
> Your comment is basically just justifying using strncpy.
>
> 4. Rather than strncpy() which has issues with maximal sized strings
> consider using strlcpy() instead.
>
> 5. Iproute2 uses kernel identation and style, consider running checkpatch
> on your changes.
>
> Please fixup and resubmit to netdev.
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: "ss -p" segfaults
2015-07-21 9:50 Segmentation fault in iproute2 ss -p (versions 4.0.0, 4.1.0 and 4.1.1) j.ps
@ 2015-07-21 9:54 ` j.ps
0 siblings, 0 replies; 11+ messages in thread
From: j.ps @ 2015-07-21 9:54 UTC (permalink / raw)
To: netdev; +Cc: schwab
Patch for 4.1.1.
Essentially all that is needed to get rid of this issue is the
addition of:
memset(u, 0, sizeof(*u));
after:
if (!(u = malloc(sizeof(*u))))
break;
Also patched some other situations (strcpy and sprintf uses) that
potentially produce the same results.
Signed-off-by: Jose P Santos <j.ps@openmailbox.org>
--- iproute2-4.1.1/misc/ss.c.orig 2015-07-06 22:57:34.000000000 +0100
+++ iproute2-4.1.1/misc/ss.c 2015-07-21 10:26:45.000000000 +0100
@@ -456,7 +456,10 @@ static void user_ent_hash_build(void)
user_ent_hash_build_init = 1;
- strcpy(name, root);
+ /* Avoid buffer overrun if input size from PROC_ROOT > name */
+ memset(name, 0, sizeof(name));
+ strncpy(name, root, sizeof(name)-2);
+
if (strlen(name) == 0 || name[strlen(name)-1] != '/')
strcat(name, "/");
@@ -480,7 +483,7 @@ static void user_ent_hash_build(void)
if (getpidcon(pid, &pid_context) != 0)
pid_context = strdup(no_ctx);
- sprintf(name + nameoff, "%d/fd/", pid);
+ snprintf(name + nameoff, sizeof(name) - nameoff, "%d/fd/", pid);
pos = strlen(name);
if ((dir1 = opendir(name)) == NULL)
continue;
@@ -499,7 +502,7 @@ static void user_ent_hash_build(void)
if (sscanf(d1->d_name, "%d%c", &fd, &crap) != 1)
continue;
- sprintf(name+pos, "%d", fd);
+ snprintf(name+pos, sizeof(name) - pos, "%d", fd);
link_len = readlink(name, lnk, sizeof(lnk)-1);
if (link_len == -1)
@@ -2722,6 +2725,11 @@ static int unix_show(struct filter *f)
if (!(u = malloc(sizeof(*u))))
break;
+ /* Zero initialization of 'u' struct avoids a segfault
+ * when freeing memory 'free(name)' at 'unix_list_free()'.
+ */
+ memset(u, 0, sizeof(*u));
+
if (sscanf(buf, "%x: %x %x %x %x %x %d %s",
&u->rport, &u->rq, &u->wq, &flags, &u->type,
&u->state, &u->ino, name) < 8)
@@ -3064,11 +3072,13 @@ static int netlink_show_one(struct filte
strncpy(procname, "kernel", 6);
} else if (pid > 0) {
FILE *fp;
- sprintf(procname, "%s/%d/stat",
+ snprintf(procname, sizeof(procname), "%s/%d/stat",
getenv("PROC_ROOT") ? : "/proc", pid);
if ((fp = fopen(procname, "r")) != NULL) {
if (fscanf(fp, "%*d (%[^)])", procname) == 1) {
- sprintf(procname+strlen(procname), "/%d", pid);
+ snprintf(procname+strlen(procname),
+ sizeof(procname)-strlen(procname),
+ "/%d", pid);
done = 1;
}
fclose(fp);
On 2015-07-20 20:09, Stephen Hemminger wrote:
> Patches are always appreciated and this looks like a real bug.
> But before I can accept it there are a couple of small
> changes needed.
>
> 1. There is no need to check for NULL when calling free().
> Glibc free is documented to accept NULL as a valid request
> and do nothing.
>
> 2. Please add a Signed-off-by: line with a real name.
> Signed-off-by has legal meaning for the Developer's Certificate of Origin
> see kernel documentation if you need more explaination.
>
> 3. Although what you found is important, giving a full paragraph
> of personal comment about it is not required. The point is software
> should read like one source independent of who the authors are.
> Your comment is basically just justifying using strncpy.
>
> 4. Rather than strncpy() which has issues with maximal sized strings
> consider using strlcpy() instead.
>
> 5. Iproute2 uses kernel identation and style, consider running checkpatch
> on your changes.
>
> Please fixup and resubmit to netdev.
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-07-21 9:54 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-15 14:09 "ss -p" segfaults Marc Dietrich
2015-07-15 15:02 ` Vadim Kochan
2015-07-15 15:12 ` Vadim Kochan
2015-07-15 16:49 ` Rustad, Mark D
2015-07-15 18:52 ` Rustad, Mark D
2015-07-15 18:57 ` Vadim Kochan
2015-07-15 22:22 ` Vadim Kochan
2015-07-16 6:37 ` Marc Dietrich
[not found] <55AC9E8C.7040200@openmailbox.org>
2015-07-20 17:31 ` j.ps
2015-07-20 18:14 ` Andreas Schwab
-- strict thread matches above, loose matches on Subject: below --
2015-07-21 9:50 Segmentation fault in iproute2 ss -p (versions 4.0.0, 4.1.0 and 4.1.1) j.ps
2015-07-21 9:54 ` "ss -p" segfaults j.ps
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).