netdev - Re: [kmemcheck] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory, in sock_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20090826084713.GC11057@elte.hu>
Date:	Wed, 26 Aug 2009 10:47:13 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Vegard Nossum <vegard.nossum@...il.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Pekka Enberg <penberg@...helsinki.fi>,
	linux-kernel@...r.kernel.org,
	Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [kmemcheck] WARNING: kmemcheck: Caught 32-bit read from
	uninitialized memory, in sock_init_data()


* Vegard Nossum <vegard.nossum@...il.com> wrote:

> 2009/8/26 Ingo Molnar <mingo@...e.hu>:
> >
> > * Eric Dumazet <eric.dumazet@...il.com> wrote:
> >
> >> Vegard Nossum a ??crit :
> >> > 2009/8/26 Ingo Molnar <mingo@...e.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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html