* Re: [kmemcheck] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory, in sock_init_data() [not found] ` <19f34abd0908260013n4e701796j90fb2b1ab74495de@mail.gmail.com> @ 2009-08-26 8:08 ` Eric Dumazet 2009-08-26 8:27 ` Ingo Molnar 0 siblings, 1 reply; 4+ messages in thread From: Eric Dumazet @ 2009-08-26 8:08 UTC (permalink / raw) To: Vegard Nossum; +Cc: Ingo Molnar, Pekka Enberg, linux-kernel, Linux Netdev List Vegard Nossum a écrit : > 2009/8/26 Ingo Molnar <mingo@elte.hu>: >> -tip testing found another kmemcheck warning: >> >> calling netlink_proto_init+0x0/0x1b0 @ 1 >> NET: Registered protocol family 16 >> initcall netlink_proto_init+0x0/0x1b0 returned 0 after 39062 usecs >> calling olpc_init+0x0/0x110 @ 1 >> WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f5c38304) >> 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff >> i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i >> ^ >> >> Pid: 1, comm: swapper Not tainted (2.6.31-rc7-tip-01170-gaaea9cf-dirty #24) P4DC6 >> EIP: 0060:[<c15c8ab1>] EFLAGS: 00010286 CPU: 0 >> EIP is at sock_init_data+0xe1/0x220 >> EAX: 0001b000 EBX: f606196c ECX: 00000000 EDX: c1a148d2 >> ESI: f6061800 EDI: f5c38300 EBP: f606ef0c ESP: c1ceb9ac >> DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 >> CR0: 8005003b CR2: f60a8108 CR3: 01a61000 CR4: 000006f0 >> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 >> DR6: ffff4ff0 DR7: 00000400 >> [<c15fac15>] __netlink_create+0x35/0xa0 >> [<c15fd01a>] netlink_kernel_create+0x5a/0x180 >> [<c15df55e>] rtnetlink_net_init+0x1e/0x50 >> [<c15d130a>] register_pernet_operations+0x6a/0xf0 >> [<c15d14fe>] register_pernet_subsys+0x1e/0x30 >> [<c1b3d84c>] rtnetlink_init+0x4c/0x100 >> [<c1b3e105>] netlink_proto_init+0x105/0x1b0 >> [<c1001037>] do_one_initcall+0x27/0x170 >> [<c1afea97>] kernel_init+0x157/0x210 >> [<c10039a7>] kernel_thread_helper+0x7/0x10 >> [<ffffffff>] 0xffffffff >> initcall olpc_init+0x0/0x110 returned 0 after 0 usecs >> calling bdi_class_init+0x0/0x40 @ 1 >> >> config attached. > > Thanks. AFAICT, it's this one: > > 1816 void sock_init_data(struct socket *sock, struct sock *sk) > 1817 { > ... > 1835 sock_set_flag(sk, SOCK_ZAPPED); Are you sure it is not the 16 bit padding in 'struct sock', after 'type' field ? struct socket { socket_state state; short type; // here, a 16 bits hole unsigned long flags; the warning is strange since I suspect it happens here : if (sock) { <<>> sk->sk_type = sock->type; // here, kmemcheck warning while reading sock->type sk->sk_sleep = &sock->wait; sock->sk = sk; and sock->type is a 16 bit field, correctly initialized (with value = 2) (Yes the hole, right after, is not initialized) WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f5c38304) 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i > > Which sort of makes sense, given all the trouble we've had with bitfields/flags. > > Is it possible that struct sock's sk_flags is (partially) > uninitialized at this point? > > I'll investigate more later, lecture of statistics is about to begin. > > > Vegard > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [kmemcheck] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory, in sock_init_data() 2009-08-26 8:08 ` [kmemcheck] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory, in sock_init_data() Eric Dumazet @ 2009-08-26 8:27 ` Ingo Molnar 2009-08-26 8:44 ` Vegard Nossum 0 siblings, 1 reply; 4+ messages in thread From: Ingo Molnar @ 2009-08-26 8:27 UTC (permalink / raw) To: Eric Dumazet; +Cc: Vegard Nossum, Pekka Enberg, linux-kernel, Linux Netdev List * Eric Dumazet <eric.dumazet@gmail.com> wrote: > Vegard Nossum a ??crit : > > 2009/8/26 Ingo Molnar <mingo@elte.hu>: > >> -tip testing found another kmemcheck warning: > >> > >> calling netlink_proto_init+0x0/0x1b0 @ 1 > >> NET: Registered protocol family 16 > >> initcall netlink_proto_init+0x0/0x1b0 returned 0 after 39062 usecs > >> calling olpc_init+0x0/0x110 @ 1 > >> WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f5c38304) > >> 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff > >> i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i > >> ^ > >> > >> Pid: 1, comm: swapper Not tainted (2.6.31-rc7-tip-01170-gaaea9cf-dirty #24) P4DC6 > >> EIP: 0060:[<c15c8ab1>] EFLAGS: 00010286 CPU: 0 > >> EIP is at sock_init_data+0xe1/0x220 > >> EAX: 0001b000 EBX: f606196c ECX: 00000000 EDX: c1a148d2 > >> ESI: f6061800 EDI: f5c38300 EBP: f606ef0c ESP: c1ceb9ac > >> DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 > >> CR0: 8005003b CR2: f60a8108 CR3: 01a61000 CR4: 000006f0 > >> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 > >> DR6: ffff4ff0 DR7: 00000400 > >> [<c15fac15>] __netlink_create+0x35/0xa0 > >> [<c15fd01a>] netlink_kernel_create+0x5a/0x180 > >> [<c15df55e>] rtnetlink_net_init+0x1e/0x50 > >> [<c15d130a>] register_pernet_operations+0x6a/0xf0 > >> [<c15d14fe>] register_pernet_subsys+0x1e/0x30 > >> [<c1b3d84c>] rtnetlink_init+0x4c/0x100 > >> [<c1b3e105>] netlink_proto_init+0x105/0x1b0 > >> [<c1001037>] do_one_initcall+0x27/0x170 > >> [<c1afea97>] kernel_init+0x157/0x210 > >> [<c10039a7>] kernel_thread_helper+0x7/0x10 > >> [<ffffffff>] 0xffffffff > >> initcall olpc_init+0x0/0x110 returned 0 after 0 usecs > >> calling bdi_class_init+0x0/0x40 @ 1 > >> > >> config attached. > > > > Thanks. AFAICT, it's this one: > > > > 1816 void sock_init_data(struct socket *sock, struct sock *sk) > > 1817 { > > ... > > 1835 sock_set_flag(sk, SOCK_ZAPPED); > > Are you sure it is not the 16 bit padding in 'struct sock', after 'type' field ? > > struct socket { > socket_state state; > short type; > // here, a 16 bits hole > unsigned long flags; > > the warning is strange since I suspect it happens here : > > if (sock) { > <<>> sk->sk_type = sock->type; // here, kmemcheck warning while reading sock->type > sk->sk_sleep = &sock->wait; > sock->sk = sk; > > and sock->type is a 16 bit field, correctly initialized (with value = 2) > (Yes the hole, right after, is not initialized) > > WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f5c38304) > 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff > i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i if so then we could perhaps annotate that by initializing it to zero on kmemcheck only. (or initialize it unconditionally if possible - that's generally the cleanest, 16-bit accesses arent cheap on all platforms) Ingo ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [kmemcheck] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory, in sock_init_data() 2009-08-26 8:27 ` Ingo Molnar @ 2009-08-26 8:44 ` Vegard Nossum 2009-08-26 8:47 ` Ingo Molnar 0 siblings, 1 reply; 4+ messages in thread From: Vegard Nossum @ 2009-08-26 8:44 UTC (permalink / raw) To: Ingo Molnar; +Cc: Eric Dumazet, Pekka Enberg, linux-kernel, Linux Netdev List 2009/8/26 Ingo Molnar <mingo@elte.hu>: > > * Eric Dumazet <eric.dumazet@gmail.com> wrote: > >> Vegard Nossum a ??crit : >> > 2009/8/26 Ingo Molnar <mingo@elte.hu>: >> >> -tip testing found another kmemcheck warning: >> >> >> >> calling netlink_proto_init+0x0/0x1b0 @ 1 >> >> NET: Registered protocol family 16 >> >> initcall netlink_proto_init+0x0/0x1b0 returned 0 after 39062 usecs >> >> calling olpc_init+0x0/0x110 @ 1 >> >> WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f5c38304) >> >> 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff >> >> i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i >> >> ^ >> >> >> >> Pid: 1, comm: swapper Not tainted (2.6.31-rc7-tip-01170-gaaea9cf-dirty #24) P4DC6 >> >> EIP: 0060:[<c15c8ab1>] EFLAGS: 00010286 CPU: 0 >> >> EIP is at sock_init_data+0xe1/0x220 >> >> EAX: 0001b000 EBX: f606196c ECX: 00000000 EDX: c1a148d2 >> >> ESI: f6061800 EDI: f5c38300 EBP: f606ef0c ESP: c1ceb9ac >> >> DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 >> >> CR0: 8005003b CR2: f60a8108 CR3: 01a61000 CR4: 000006f0 >> >> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 >> >> DR6: ffff4ff0 DR7: 00000400 >> >> [<c15fac15>] __netlink_create+0x35/0xa0 >> >> [<c15fd01a>] netlink_kernel_create+0x5a/0x180 >> >> [<c15df55e>] rtnetlink_net_init+0x1e/0x50 >> >> [<c15d130a>] register_pernet_operations+0x6a/0xf0 >> >> [<c15d14fe>] register_pernet_subsys+0x1e/0x30 >> >> [<c1b3d84c>] rtnetlink_init+0x4c/0x100 >> >> [<c1b3e105>] netlink_proto_init+0x105/0x1b0 >> >> [<c1001037>] do_one_initcall+0x27/0x170 >> >> [<c1afea97>] kernel_init+0x157/0x210 >> >> [<c10039a7>] kernel_thread_helper+0x7/0x10 >> >> [<ffffffff>] 0xffffffff >> >> initcall olpc_init+0x0/0x110 returned 0 after 0 usecs >> >> calling bdi_class_init+0x0/0x40 @ 1 >> >> >> >> config attached. >> > >> > Thanks. AFAICT, it's this one: >> > >> > 1816 void sock_init_data(struct socket *sock, struct sock *sk) >> > 1817 { >> > ... >> > 1835 sock_set_flag(sk, SOCK_ZAPPED); >> >> Are you sure it is not the 16 bit padding in 'struct sock', after 'type' field ? >> >> struct socket { >> socket_state state; >> short type; >> // here, a 16 bits hole >> unsigned long flags; >> >> the warning is strange since I suspect it happens here : >> >> if (sock) { >> <<>> sk->sk_type = sock->type; // here, kmemcheck warning while reading sock->type >> sk->sk_sleep = &sock->wait; >> sock->sk = sk; >> >> and sock->type is a 16 bit field, correctly initialized (with value = 2) >> (Yes the hole, right after, is not initialized) >> Ah, right, makes sense. There are just two uninitialized bytes, too, we can see it in the shadow dump: >> WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f5c38304) >> 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff >> i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i > > if so then we could perhaps annotate that by initializing it to zero > on kmemcheck only. (or initialize it unconditionally if possible - > that's generally the cleanest, 16-bit accesses arent cheap on all > platforms) > We should have eliminated these padding-related false-positives by droppnig the -Os / CONFIG_CC_OPTIMIZE_FOR_SIZE, which should emit a movzwl instruction or so instead of movl, but it's admittedly somewhat dependent on gcc anyway. I'm wondering whether it would be worth it to (try to) patch gcc not to emit these "bigger-than-necessary" loads with a new -fsomething flag. Vegard ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [kmemcheck] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory, in sock_init_data() 2009-08-26 8:44 ` Vegard Nossum @ 2009-08-26 8:47 ` Ingo Molnar 0 siblings, 0 replies; 4+ messages in thread From: Ingo Molnar @ 2009-08-26 8:47 UTC (permalink / raw) To: Vegard Nossum; +Cc: Eric Dumazet, Pekka Enberg, linux-kernel, Linux Netdev List * Vegard Nossum <vegard.nossum@gmail.com> wrote: > 2009/8/26 Ingo Molnar <mingo@elte.hu>: > > > > * Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > >> Vegard Nossum a ??crit : > >> > 2009/8/26 Ingo Molnar <mingo@elte.hu>: > >> >> -tip testing found another kmemcheck warning: > >> >> > >> >> calling ??netlink_proto_init+0x0/0x1b0 @ 1 > >> >> NET: Registered protocol family 16 > >> >> initcall netlink_proto_init+0x0/0x1b0 returned 0 after 39062 usecs > >> >> calling ??olpc_init+0x0/0x110 @ 1 > >> >> WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f5c38304) > >> >> 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff > >> >> ??i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i > >> >> ?? ?? ?? ?? ^ > >> >> > >> >> Pid: 1, comm: swapper Not tainted (2.6.31-rc7-tip-01170-gaaea9cf-dirty #24) P4DC6 > >> >> EIP: 0060:[<c15c8ab1>] EFLAGS: 00010286 CPU: 0 > >> >> EIP is at sock_init_data+0xe1/0x220 > >> >> EAX: 0001b000 EBX: f606196c ECX: 00000000 EDX: c1a148d2 > >> >> ESI: f6061800 EDI: f5c38300 EBP: f606ef0c ESP: c1ceb9ac > >> >> ??DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 > >> >> CR0: 8005003b CR2: f60a8108 CR3: 01a61000 CR4: 000006f0 > >> >> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 > >> >> DR6: ffff4ff0 DR7: 00000400 > >> >> ??[<c15fac15>] __netlink_create+0x35/0xa0 > >> >> ??[<c15fd01a>] netlink_kernel_create+0x5a/0x180 > >> >> ??[<c15df55e>] rtnetlink_net_init+0x1e/0x50 > >> >> ??[<c15d130a>] register_pernet_operations+0x6a/0xf0 > >> >> ??[<c15d14fe>] register_pernet_subsys+0x1e/0x30 > >> >> ??[<c1b3d84c>] rtnetlink_init+0x4c/0x100 > >> >> ??[<c1b3e105>] netlink_proto_init+0x105/0x1b0 > >> >> ??[<c1001037>] do_one_initcall+0x27/0x170 > >> >> ??[<c1afea97>] kernel_init+0x157/0x210 > >> >> ??[<c10039a7>] kernel_thread_helper+0x7/0x10 > >> >> ??[<ffffffff>] 0xffffffff > >> >> initcall olpc_init+0x0/0x110 returned 0 after 0 usecs > >> >> calling ??bdi_class_init+0x0/0x40 @ 1 > >> >> > >> >> config attached. > >> > > >> > Thanks. AFAICT, it's this one: > >> > > >> > 1816 void sock_init_data(struct socket *sock, struct sock *sk) > >> > 1817 { > >> > ... > >> > 1835 ?? ?? ?? ?? sock_set_flag(sk, SOCK_ZAPPED); > >> > >> Are you sure it is not the 16 bit padding in 'struct sock', after 'type' field ? > >> > >> struct socket { > >> ?? ?? ?? socket_state ?? ?? ?? ??state; > >> ?? ?? ?? short ?? ?? ?? ?? ?? type; > >> // here, a 16 bits hole > >> ?? ?? ?? unsigned long ?? ?? ?? flags; > >> > >> the warning is strange since I suspect it happens here : > >> > >> ?? ?? if (sock) { > >> <<>> ??sk->sk_type = ?? sock->type; // here, kmemcheck warning while reading sock->type > >> ?? ?? ?? sk->sk_sleep ?? ??= ?? &sock->wait; > >> ?? ?? ?? sock->sk ?? ??= ?? sk; > >> > >> and sock->type is a 16 bit field, correctly initialized (with value = 2) > >> (Yes the hole, right after, is not initialized) > >> > > Ah, right, makes sense. There are just two uninitialized bytes, too, > we can see it in the shadow dump: > > >> WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f5c38304) > >> 0100000002000000000000000000000000000000ad4eaddeffffffffffffffff > >> ??i i i i i i u u i i i i i i i i i i i i i i i i i i i i i i i i > > > > if so then we could perhaps annotate that by initializing it to zero > > on kmemcheck only. (or initialize it unconditionally if possible - > > that's generally the cleanest, 16-bit accesses arent cheap on all > > platforms) > > We should have eliminated these padding-related false-positives by > droppnig the -Os / CONFIG_CC_OPTIMIZE_FOR_SIZE, which should emit > a movzwl instruction or so instead of movl, but it's admittedly > somewhat dependent on gcc anyway. I'm wondering whether it would > be worth it to (try to) patch gcc not to emit these > "bigger-than-necessary" loads with a new -fsomething flag. The latency of even the smallest enhancements to GCC is so huge that we cannot rely on it. There's really just a few places in the kernel that are that tightly/trickily packed - the signal code (which we annotated IIRC?) and the networking code. Lets annotate it so that we can have a 'no warnings' baseline. Ingo ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-08-26 8:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20090826055659.GA6066@elte.hu>
[not found] ` <19f34abd0908260013n4e701796j90fb2b1ab74495de@mail.gmail.com>
2009-08-26 8:08 ` [kmemcheck] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory, in sock_init_data() Eric Dumazet
2009-08-26 8:27 ` Ingo Molnar
2009-08-26 8:44 ` Vegard Nossum
2009-08-26 8:47 ` Ingo Molnar
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).