* Uninitialized value in __sk_nulls_add_node_rcu()
@ 2017-10-26 12:51 Alexander Potapenko
2017-10-26 14:20 ` Alexander Potapenko
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Potapenko @ 2017-10-26 12:51 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: Networking
Hi David, Eric,
I've changed KMSAN instrumentation a bit and it's now reporting a new
error (see below) when I SSH into a VM.
For |req| allocated by inet_reqsk_alloc() the value of
req_to_sk(req)->sk_reuseport is uninitialized.
Does this look valid?
I'm a bit surprised this didn't show up before, but I couldn't find
any code to initialize sk_reuseport.
==================================================================
BUG: KMSAN: use of uninitialized memory in inet_ehash_insert+0xd40/0x1050
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0+ #3288
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:16
dump_stack+0x185/0x1d0 lib/dump_stack.c:52
kmsan_report+0x13f/0x1c0 mm/kmsan/kmsan.c:1016
__msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:766
__sk_nulls_add_node_rcu ./include/net/sock.h:684
inet_ehash_insert+0xd40/0x1050 net/ipv4/inet_hashtables.c:413
reqsk_queue_hash_req net/ipv4/inet_connection_sock.c:754
inet_csk_reqsk_queue_hash_add+0x1cc/0x300 net/ipv4/inet_connection_sock.c:765
tcp_conn_request+0x31e7/0x36f0 net/ipv4/tcp_input.c:6414
tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
NF_HOOK ./include/linux/netfilter.h:248
ip_local_deliver+0x3fa/0x480 net/ipv4/ip_input.c:257
dst_input ./include/net/dst.h:477
ip_rcv_finish+0x6fb/0x1540 net/ipv4/ip_input.c:397
NF_HOOK ./include/linux/netfilter.h:248
ip_rcv+0x10f6/0x15c0 net/ipv4/ip_input.c:488
__netif_receive_skb_core+0x36f6/0x3f60 net/core/dev.c:4298
__netif_receive_skb net/core/dev.c:4336
netif_receive_skb_internal+0x63c/0x19c0 net/core/dev.c:4497
napi_skb_finish net/core/dev.c:4858
napi_gro_receive+0x629/0xa50 net/core/dev.c:4889
e1000_receive_skb drivers/net/ethernet/intel/e1000/e1000_main.c:4018
e1000_clean_rx_irq+0x1492/0x1d30
drivers/net/ethernet/intel/e1000/e1000_main.c:4474
e1000_clean+0x43aa/0x5970 drivers/net/ethernet/intel/e1000/e1000_main.c:3819
napi_poll net/core/dev.c:5500
net_rx_action+0x73c/0x1820 net/core/dev.c:5566
__do_softirq+0x4b4/0x8dd kernel/softirq.c:284
invoke_softirq kernel/softirq.c:364
irq_exit+0x203/0x240 kernel/softirq.c:405
exiting_irq+0xe/0x10 ./arch/x86/include/asm/apic.h:638
do_IRQ+0x15e/0x1a0 arch/x86/kernel/irq.c:263
common_interrupt+0x86/0x86
...
</IRQ>
arch_cpu_idle+0x20/0x30 arch/x86/kernel/process.c:332
default_idle_call kernel/sched/idle.c:98
cpuidle_idle_call kernel/sched/idle.c:156
do_idle+0x334/0x730 kernel/sched/idle.c:246
cpu_startup_entry+0x35/0x40 kernel/sched/idle.c:351
rest_init+0xb8/0xc0 init/main.c:437
start_kernel+0x4d7/0x530 init/main.c:703
x86_64_start_reservations arch/x86/kernel/head64.c:318
x86_64_start_kernel+0x3cd/0x3e0 arch/x86/kernel/head64.c:299
secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:219
origin:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
reqsk_alloc ./include/net/request_sock.h:88
inet_reqsk_alloc+0xb8/0x6b0 net/ipv4/tcp_input.c:6231
tcp_conn_request+0x89d/0x36f0 net/ipv4/tcp_input.c:6330
tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
NF_HOOK ./include/linux/netfilter.h:248
...
==================================================================
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-10-26 12:51 Alexander Potapenko
@ 2017-10-26 14:20 ` Alexander Potapenko
2017-10-26 14:47 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Potapenko @ 2017-10-26 14:20 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: Networking
On Thu, Oct 26, 2017 at 2:51 PM, Alexander Potapenko <glider@google.com> wrote:
> Hi David, Eric,
>
> I've changed KMSAN instrumentation a bit and it's now reporting a new
> error (see below) when I SSH into a VM.
I've double-checked the old instrumentation and found a bug in it,
which led to masking some of the errors on uninitialized bitfields.
I now believe this is a true positive report.
> For |req| allocated by inet_reqsk_alloc() the value of
> req_to_sk(req)->sk_reuseport is uninitialized.
> Does this look valid?
> I'm a bit surprised this didn't show up before, but I couldn't find
> any code to initialize sk_reuseport.
>
> ==================================================================
> BUG: KMSAN: use of uninitialized memory in inet_ehash_insert+0xd40/0x1050
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0+ #3288
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> <IRQ>
> __dump_stack lib/dump_stack.c:16
> dump_stack+0x185/0x1d0 lib/dump_stack.c:52
> kmsan_report+0x13f/0x1c0 mm/kmsan/kmsan.c:1016
> __msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:766
> __sk_nulls_add_node_rcu ./include/net/sock.h:684
> inet_ehash_insert+0xd40/0x1050 net/ipv4/inet_hashtables.c:413
> reqsk_queue_hash_req net/ipv4/inet_connection_sock.c:754
> inet_csk_reqsk_queue_hash_add+0x1cc/0x300 net/ipv4/inet_connection_sock.c:765
> tcp_conn_request+0x31e7/0x36f0 net/ipv4/tcp_input.c:6414
> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
> NF_HOOK ./include/linux/netfilter.h:248
> ip_local_deliver+0x3fa/0x480 net/ipv4/ip_input.c:257
> dst_input ./include/net/dst.h:477
> ip_rcv_finish+0x6fb/0x1540 net/ipv4/ip_input.c:397
> NF_HOOK ./include/linux/netfilter.h:248
> ip_rcv+0x10f6/0x15c0 net/ipv4/ip_input.c:488
> __netif_receive_skb_core+0x36f6/0x3f60 net/core/dev.c:4298
> __netif_receive_skb net/core/dev.c:4336
> netif_receive_skb_internal+0x63c/0x19c0 net/core/dev.c:4497
> napi_skb_finish net/core/dev.c:4858
> napi_gro_receive+0x629/0xa50 net/core/dev.c:4889
> e1000_receive_skb drivers/net/ethernet/intel/e1000/e1000_main.c:4018
> e1000_clean_rx_irq+0x1492/0x1d30
> drivers/net/ethernet/intel/e1000/e1000_main.c:4474
> e1000_clean+0x43aa/0x5970 drivers/net/ethernet/intel/e1000/e1000_main.c:3819
> napi_poll net/core/dev.c:5500
> net_rx_action+0x73c/0x1820 net/core/dev.c:5566
> __do_softirq+0x4b4/0x8dd kernel/softirq.c:284
> invoke_softirq kernel/softirq.c:364
> irq_exit+0x203/0x240 kernel/softirq.c:405
> exiting_irq+0xe/0x10 ./arch/x86/include/asm/apic.h:638
> do_IRQ+0x15e/0x1a0 arch/x86/kernel/irq.c:263
> common_interrupt+0x86/0x86
> ...
> </IRQ>
> arch_cpu_idle+0x20/0x30 arch/x86/kernel/process.c:332
> default_idle_call kernel/sched/idle.c:98
> cpuidle_idle_call kernel/sched/idle.c:156
> do_idle+0x334/0x730 kernel/sched/idle.c:246
> cpu_startup_entry+0x35/0x40 kernel/sched/idle.c:351
> rest_init+0xb8/0xc0 init/main.c:437
> start_kernel+0x4d7/0x530 init/main.c:703
> x86_64_start_reservations arch/x86/kernel/head64.c:318
> x86_64_start_kernel+0x3cd/0x3e0 arch/x86/kernel/head64.c:299
> secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:219
> origin:
> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
> kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
> kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
> kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
> reqsk_alloc ./include/net/request_sock.h:88
> inet_reqsk_alloc+0xb8/0x6b0 net/ipv4/tcp_input.c:6231
> tcp_conn_request+0x89d/0x36f0 net/ipv4/tcp_input.c:6330
> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
> NF_HOOK ./include/linux/netfilter.h:248
> ...
> ==================================================================
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-10-26 14:20 ` Alexander Potapenko
@ 2017-10-26 14:47 ` Eric Dumazet
2017-10-26 14:52 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2017-10-26 14:47 UTC (permalink / raw)
To: Alexander Potapenko; +Cc: David Miller, Networking
On Thu, Oct 26, 2017 at 7:20 AM, Alexander Potapenko <glider@google.com> wrote:
> On Thu, Oct 26, 2017 at 2:51 PM, Alexander Potapenko <glider@google.com> wrote:
>> Hi David, Eric,
>>
>> I've changed KMSAN instrumentation a bit and it's now reporting a new
>> error (see below) when I SSH into a VM.
> I've double-checked the old instrumentation and found a bug in it,
> which led to masking some of the errors on uninitialized bitfields.
> I now believe this is a true positive report.
Please do not top post on netdev
A child is cloned from the listener, check sock_copy()
sk_reuseport is part of the copied fields.
You might have some bug at your side ?
>> For |req| allocated by inet_reqsk_alloc() the value of
>> req_to_sk(req)->sk_reuseport is uninitialized.
>> Does this look valid?
>> I'm a bit surprised this didn't show up before, but I couldn't find
>> any code to initialize sk_reuseport.
>>
>> ==================================================================
>> BUG: KMSAN: use of uninitialized memory in inet_ehash_insert+0xd40/0x1050
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0+ #3288
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> Call Trace:
>> <IRQ>
>> __dump_stack lib/dump_stack.c:16
>> dump_stack+0x185/0x1d0 lib/dump_stack.c:52
>> kmsan_report+0x13f/0x1c0 mm/kmsan/kmsan.c:1016
>> __msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:766
>> __sk_nulls_add_node_rcu ./include/net/sock.h:684
>> inet_ehash_insert+0xd40/0x1050 net/ipv4/inet_hashtables.c:413
>> reqsk_queue_hash_req net/ipv4/inet_connection_sock.c:754
>> inet_csk_reqsk_queue_hash_add+0x1cc/0x300 net/ipv4/inet_connection_sock.c:765
>> tcp_conn_request+0x31e7/0x36f0 net/ipv4/tcp_input.c:6414
>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>> NF_HOOK ./include/linux/netfilter.h:248
>> ip_local_deliver+0x3fa/0x480 net/ipv4/ip_input.c:257
>> dst_input ./include/net/dst.h:477
>> ip_rcv_finish+0x6fb/0x1540 net/ipv4/ip_input.c:397
>> NF_HOOK ./include/linux/netfilter.h:248
>> ip_rcv+0x10f6/0x15c0 net/ipv4/ip_input.c:488
>> __netif_receive_skb_core+0x36f6/0x3f60 net/core/dev.c:4298
>> __netif_receive_skb net/core/dev.c:4336
>> netif_receive_skb_internal+0x63c/0x19c0 net/core/dev.c:4497
>> napi_skb_finish net/core/dev.c:4858
>> napi_gro_receive+0x629/0xa50 net/core/dev.c:4889
>> e1000_receive_skb drivers/net/ethernet/intel/e1000/e1000_main.c:4018
>> e1000_clean_rx_irq+0x1492/0x1d30
>> drivers/net/ethernet/intel/e1000/e1000_main.c:4474
>> e1000_clean+0x43aa/0x5970 drivers/net/ethernet/intel/e1000/e1000_main.c:3819
>> napi_poll net/core/dev.c:5500
>> net_rx_action+0x73c/0x1820 net/core/dev.c:5566
>> __do_softirq+0x4b4/0x8dd kernel/softirq.c:284
>> invoke_softirq kernel/softirq.c:364
>> irq_exit+0x203/0x240 kernel/softirq.c:405
>> exiting_irq+0xe/0x10 ./arch/x86/include/asm/apic.h:638
>> do_IRQ+0x15e/0x1a0 arch/x86/kernel/irq.c:263
>> common_interrupt+0x86/0x86
>> ...
>> </IRQ>
>> arch_cpu_idle+0x20/0x30 arch/x86/kernel/process.c:332
>> default_idle_call kernel/sched/idle.c:98
>> cpuidle_idle_call kernel/sched/idle.c:156
>> do_idle+0x334/0x730 kernel/sched/idle.c:246
>> cpu_startup_entry+0x35/0x40 kernel/sched/idle.c:351
>> rest_init+0xb8/0xc0 init/main.c:437
>> start_kernel+0x4d7/0x530 init/main.c:703
>> x86_64_start_reservations arch/x86/kernel/head64.c:318
>> x86_64_start_kernel+0x3cd/0x3e0 arch/x86/kernel/head64.c:299
>> secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:219
>> origin:
>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
>> kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
>> kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
>> kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
>> reqsk_alloc ./include/net/request_sock.h:88
>> inet_reqsk_alloc+0xb8/0x6b0 net/ipv4/tcp_input.c:6231
>> tcp_conn_request+0x89d/0x36f0 net/ipv4/tcp_input.c:6330
>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>> NF_HOOK ./include/linux/netfilter.h:248
>> ...
>> ==================================================================
>>
>> --
>> Alexander Potapenko
>> Software Engineer
>>
>> Google Germany GmbH
>> Erika-Mann-Straße, 33
>> 80636 München
>>
>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>> Registergericht und -nummer: Hamburg, HRB 86891
>> Sitz der Gesellschaft: Hamburg
>
>
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-10-26 14:47 ` Eric Dumazet
@ 2017-10-26 14:52 ` Eric Dumazet
2017-10-26 14:56 ` Alexander Potapenko
0 siblings, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2017-10-26 14:52 UTC (permalink / raw)
To: Alexander Potapenko, Craig Gallek; +Cc: David Miller, Networking
On Thu, Oct 26, 2017 at 7:47 AM, Eric Dumazet <edumazet@google.com> wrote:
> On Thu, Oct 26, 2017 at 7:20 AM, Alexander Potapenko <glider@google.com> wrote:
>> On Thu, Oct 26, 2017 at 2:51 PM, Alexander Potapenko <glider@google.com> wrote:
>>> Hi David, Eric,
>>>
>>> I've changed KMSAN instrumentation a bit and it's now reporting a new
>>> error (see below) when I SSH into a VM.
>> I've double-checked the old instrumentation and found a bug in it,
>> which led to masking some of the errors on uninitialized bitfields.
>> I now believe this is a true positive report.
>
>
> Please do not top post on netdev
>
> A child is cloned from the listener, check sock_copy()
>
> sk_reuseport is part of the copied fields.
>
> You might have some bug at your side ?
>
Oh these are request socket.
This is an harmless bug added in commit d894ba18d4e449b3a7f6eb491f16c9e02933736e
("soreuseport: fix ordering for mixed v4/v6 sockets")
I will send a patch, but really this has no effect at all.
>
>>> For |req| allocated by inet_reqsk_alloc() the value of
>>> req_to_sk(req)->sk_reuseport is uninitialized.
>>> Does this look valid?
>>> I'm a bit surprised this didn't show up before, but I couldn't find
>>> any code to initialize sk_reuseport.
>>>
>>> ==================================================================
>>> BUG: KMSAN: use of uninitialized memory in inet_ehash_insert+0xd40/0x1050
>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0+ #3288
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>> Call Trace:
>>> <IRQ>
>>> __dump_stack lib/dump_stack.c:16
>>> dump_stack+0x185/0x1d0 lib/dump_stack.c:52
>>> kmsan_report+0x13f/0x1c0 mm/kmsan/kmsan.c:1016
>>> __msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:766
>>> __sk_nulls_add_node_rcu ./include/net/sock.h:684
>>> inet_ehash_insert+0xd40/0x1050 net/ipv4/inet_hashtables.c:413
>>> reqsk_queue_hash_req net/ipv4/inet_connection_sock.c:754
>>> inet_csk_reqsk_queue_hash_add+0x1cc/0x300 net/ipv4/inet_connection_sock.c:765
>>> tcp_conn_request+0x31e7/0x36f0 net/ipv4/tcp_input.c:6414
>>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>>> NF_HOOK ./include/linux/netfilter.h:248
>>> ip_local_deliver+0x3fa/0x480 net/ipv4/ip_input.c:257
>>> dst_input ./include/net/dst.h:477
>>> ip_rcv_finish+0x6fb/0x1540 net/ipv4/ip_input.c:397
>>> NF_HOOK ./include/linux/netfilter.h:248
>>> ip_rcv+0x10f6/0x15c0 net/ipv4/ip_input.c:488
>>> __netif_receive_skb_core+0x36f6/0x3f60 net/core/dev.c:4298
>>> __netif_receive_skb net/core/dev.c:4336
>>> netif_receive_skb_internal+0x63c/0x19c0 net/core/dev.c:4497
>>> napi_skb_finish net/core/dev.c:4858
>>> napi_gro_receive+0x629/0xa50 net/core/dev.c:4889
>>> e1000_receive_skb drivers/net/ethernet/intel/e1000/e1000_main.c:4018
>>> e1000_clean_rx_irq+0x1492/0x1d30
>>> drivers/net/ethernet/intel/e1000/e1000_main.c:4474
>>> e1000_clean+0x43aa/0x5970 drivers/net/ethernet/intel/e1000/e1000_main.c:3819
>>> napi_poll net/core/dev.c:5500
>>> net_rx_action+0x73c/0x1820 net/core/dev.c:5566
>>> __do_softirq+0x4b4/0x8dd kernel/softirq.c:284
>>> invoke_softirq kernel/softirq.c:364
>>> irq_exit+0x203/0x240 kernel/softirq.c:405
>>> exiting_irq+0xe/0x10 ./arch/x86/include/asm/apic.h:638
>>> do_IRQ+0x15e/0x1a0 arch/x86/kernel/irq.c:263
>>> common_interrupt+0x86/0x86
>>> ...
>>> </IRQ>
>>> arch_cpu_idle+0x20/0x30 arch/x86/kernel/process.c:332
>>> default_idle_call kernel/sched/idle.c:98
>>> cpuidle_idle_call kernel/sched/idle.c:156
>>> do_idle+0x334/0x730 kernel/sched/idle.c:246
>>> cpu_startup_entry+0x35/0x40 kernel/sched/idle.c:351
>>> rest_init+0xb8/0xc0 init/main.c:437
>>> start_kernel+0x4d7/0x530 init/main.c:703
>>> x86_64_start_reservations arch/x86/kernel/head64.c:318
>>> x86_64_start_kernel+0x3cd/0x3e0 arch/x86/kernel/head64.c:299
>>> secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:219
>>> origin:
>>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>>> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
>>> kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
>>> kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
>>> kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
>>> reqsk_alloc ./include/net/request_sock.h:88
>>> inet_reqsk_alloc+0xb8/0x6b0 net/ipv4/tcp_input.c:6231
>>> tcp_conn_request+0x89d/0x36f0 net/ipv4/tcp_input.c:6330
>>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>>> NF_HOOK ./include/linux/netfilter.h:248
>>> ...
>>> ==================================================================
>>>
>>> --
>>> Alexander Potapenko
>>> Software Engineer
>>>
>>> Google Germany GmbH
>>> Erika-Mann-Straße, 33
>>> 80636 München
>>>
>>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>>> Registergericht und -nummer: Hamburg, HRB 86891
>>> Sitz der Gesellschaft: Hamburg
>>
>>
>>
>> --
>> Alexander Potapenko
>> Software Engineer
>>
>> Google Germany GmbH
>> Erika-Mann-Straße, 33
>> 80636 München
>>
>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>> Registergericht und -nummer: Hamburg, HRB 86891
>> Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-10-26 14:52 ` Eric Dumazet
@ 2017-10-26 14:56 ` Alexander Potapenko
2017-11-22 13:38 ` Alexander Potapenko
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Potapenko @ 2017-10-26 14:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Craig Gallek, David Miller, Networking
On Thu, Oct 26, 2017 at 4:52 PM, Eric Dumazet <edumazet@google.com> wrote:
> On Thu, Oct 26, 2017 at 7:47 AM, Eric Dumazet <edumazet@google.com> wrote:
>> On Thu, Oct 26, 2017 at 7:20 AM, Alexander Potapenko <glider@google.com> wrote:
>>> On Thu, Oct 26, 2017 at 2:51 PM, Alexander Potapenko <glider@google.com> wrote:
>>>> Hi David, Eric,
>>>>
>>>> I've changed KMSAN instrumentation a bit and it's now reporting a new
>>>> error (see below) when I SSH into a VM.
>>> I've double-checked the old instrumentation and found a bug in it,
>>> which led to masking some of the errors on uninitialized bitfields.
>>> I now believe this is a true positive report.
>>
>>
>> Please do not top post on netdev
Sorry about that.
>> A child is cloned from the listener, check sock_copy()
>>
>> sk_reuseport is part of the copied fields.
>>
>> You might have some bug at your side ?
>>
>
> Oh these are request socket.
>
> This is an harmless bug added in commit d894ba18d4e449b3a7f6eb491f16c9e02933736e
> ("soreuseport: fix ordering for mixed v4/v6 sockets")
>
> I will send a patch, but really this has no effect at all.
Thanks for clarifying!
For me this was a question of the tool's correctness, because until
recently I wasn't able to understand whether this is a true bug or
not.
>
>>
>>>> For |req| allocated by inet_reqsk_alloc() the value of
>>>> req_to_sk(req)->sk_reuseport is uninitialized.
>>>> Does this look valid?
>>>> I'm a bit surprised this didn't show up before, but I couldn't find
>>>> any code to initialize sk_reuseport.
>>>>
>>>> ==================================================================
>>>> BUG: KMSAN: use of uninitialized memory in inet_ehash_insert+0xd40/0x1050
>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0+ #3288
>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>>> Call Trace:
>>>> <IRQ>
>>>> __dump_stack lib/dump_stack.c:16
>>>> dump_stack+0x185/0x1d0 lib/dump_stack.c:52
>>>> kmsan_report+0x13f/0x1c0 mm/kmsan/kmsan.c:1016
>>>> __msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:766
>>>> __sk_nulls_add_node_rcu ./include/net/sock.h:684
>>>> inet_ehash_insert+0xd40/0x1050 net/ipv4/inet_hashtables.c:413
>>>> reqsk_queue_hash_req net/ipv4/inet_connection_sock.c:754
>>>> inet_csk_reqsk_queue_hash_add+0x1cc/0x300 net/ipv4/inet_connection_sock.c:765
>>>> tcp_conn_request+0x31e7/0x36f0 net/ipv4/tcp_input.c:6414
>>>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>>>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>>>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>>>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>>>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>> ip_local_deliver+0x3fa/0x480 net/ipv4/ip_input.c:257
>>>> dst_input ./include/net/dst.h:477
>>>> ip_rcv_finish+0x6fb/0x1540 net/ipv4/ip_input.c:397
>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>> ip_rcv+0x10f6/0x15c0 net/ipv4/ip_input.c:488
>>>> __netif_receive_skb_core+0x36f6/0x3f60 net/core/dev.c:4298
>>>> __netif_receive_skb net/core/dev.c:4336
>>>> netif_receive_skb_internal+0x63c/0x19c0 net/core/dev.c:4497
>>>> napi_skb_finish net/core/dev.c:4858
>>>> napi_gro_receive+0x629/0xa50 net/core/dev.c:4889
>>>> e1000_receive_skb drivers/net/ethernet/intel/e1000/e1000_main.c:4018
>>>> e1000_clean_rx_irq+0x1492/0x1d30
>>>> drivers/net/ethernet/intel/e1000/e1000_main.c:4474
>>>> e1000_clean+0x43aa/0x5970 drivers/net/ethernet/intel/e1000/e1000_main.c:3819
>>>> napi_poll net/core/dev.c:5500
>>>> net_rx_action+0x73c/0x1820 net/core/dev.c:5566
>>>> __do_softirq+0x4b4/0x8dd kernel/softirq.c:284
>>>> invoke_softirq kernel/softirq.c:364
>>>> irq_exit+0x203/0x240 kernel/softirq.c:405
>>>> exiting_irq+0xe/0x10 ./arch/x86/include/asm/apic.h:638
>>>> do_IRQ+0x15e/0x1a0 arch/x86/kernel/irq.c:263
>>>> common_interrupt+0x86/0x86
>>>> ...
>>>> </IRQ>
>>>> arch_cpu_idle+0x20/0x30 arch/x86/kernel/process.c:332
>>>> default_idle_call kernel/sched/idle.c:98
>>>> cpuidle_idle_call kernel/sched/idle.c:156
>>>> do_idle+0x334/0x730 kernel/sched/idle.c:246
>>>> cpu_startup_entry+0x35/0x40 kernel/sched/idle.c:351
>>>> rest_init+0xb8/0xc0 init/main.c:437
>>>> start_kernel+0x4d7/0x530 init/main.c:703
>>>> x86_64_start_reservations arch/x86/kernel/head64.c:318
>>>> x86_64_start_kernel+0x3cd/0x3e0 arch/x86/kernel/head64.c:299
>>>> secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:219
>>>> origin:
>>>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>>>> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
>>>> kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
>>>> kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
>>>> kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
>>>> reqsk_alloc ./include/net/request_sock.h:88
>>>> inet_reqsk_alloc+0xb8/0x6b0 net/ipv4/tcp_input.c:6231
>>>> tcp_conn_request+0x89d/0x36f0 net/ipv4/tcp_input.c:6330
>>>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>>>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>>>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>>>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>>>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>> ...
>>>> ==================================================================
>>>>
>>>> --
>>>> Alexander Potapenko
>>>> Software Engineer
>>>>
>>>> Google Germany GmbH
>>>> Erika-Mann-Straße, 33
>>>> 80636 München
>>>>
>>>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>>>> Registergericht und -nummer: Hamburg, HRB 86891
>>>> Sitz der Gesellschaft: Hamburg
>>>
>>>
>>>
>>> --
>>> Alexander Potapenko
>>> Software Engineer
>>>
>>> Google Germany GmbH
>>> Erika-Mann-Straße, 33
>>> 80636 München
>>>
>>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>>> Registergericht und -nummer: Hamburg, HRB 86891
>>> Sitz der Gesellschaft: Hamburg
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-10-26 14:56 ` Alexander Potapenko
@ 2017-11-22 13:38 ` Alexander Potapenko
2017-11-22 15:58 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Potapenko @ 2017-11-22 13:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Craig Gallek, David Miller, Networking
On Thu, Oct 26, 2017 at 4:56 PM, Alexander Potapenko <glider@google.com> wrote:
> On Thu, Oct 26, 2017 at 4:52 PM, Eric Dumazet <edumazet@google.com> wrote:
>> On Thu, Oct 26, 2017 at 7:47 AM, Eric Dumazet <edumazet@google.com> wrote:
>>> On Thu, Oct 26, 2017 at 7:20 AM, Alexander Potapenko <glider@google.com> wrote:
>>>> On Thu, Oct 26, 2017 at 2:51 PM, Alexander Potapenko <glider@google.com> wrote:
>>>>> Hi David, Eric,
>>>>>
>>>>> I've changed KMSAN instrumentation a bit and it's now reporting a new
>>>>> error (see below) when I SSH into a VM.
>>>> I've double-checked the old instrumentation and found a bug in it,
>>>> which led to masking some of the errors on uninitialized bitfields.
>>>> I now believe this is a true positive report.
>>>
>>>
>>> Please do not top post on netdev
> Sorry about that.
>>> A child is cloned from the listener, check sock_copy()
>>>
>>> sk_reuseport is part of the copied fields.
>>>
>>> You might have some bug at your side ?
>>>
>>
>> Oh these are request socket.
>>
>> This is an harmless bug added in commit d894ba18d4e449b3a7f6eb491f16c9e02933736e
>> ("soreuseport: fix ordering for mixed v4/v6 sockets")
>>
>> I will send a patch, but really this has no effect at all.
> Thanks for clarifying!
> For me this was a question of the tool's correctness, because until
> recently I wasn't able to understand whether this is a true bug or
> not.
A friendly ping.
Eric, did you find the time to send the patch?
>>>
>>>>> For |req| allocated by inet_reqsk_alloc() the value of
>>>>> req_to_sk(req)->sk_reuseport is uninitialized.
>>>>> Does this look valid?
>>>>> I'm a bit surprised this didn't show up before, but I couldn't find
>>>>> any code to initialize sk_reuseport.
>>>>>
>>>>> ==================================================================
>>>>> BUG: KMSAN: use of uninitialized memory in inet_ehash_insert+0xd40/0x1050
>>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0+ #3288
>>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>>>> Call Trace:
>>>>> <IRQ>
>>>>> __dump_stack lib/dump_stack.c:16
>>>>> dump_stack+0x185/0x1d0 lib/dump_stack.c:52
>>>>> kmsan_report+0x13f/0x1c0 mm/kmsan/kmsan.c:1016
>>>>> __msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:766
>>>>> __sk_nulls_add_node_rcu ./include/net/sock.h:684
>>>>> inet_ehash_insert+0xd40/0x1050 net/ipv4/inet_hashtables.c:413
>>>>> reqsk_queue_hash_req net/ipv4/inet_connection_sock.c:754
>>>>> inet_csk_reqsk_queue_hash_add+0x1cc/0x300 net/ipv4/inet_connection_sock.c:765
>>>>> tcp_conn_request+0x31e7/0x36f0 net/ipv4/tcp_input.c:6414
>>>>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>>>>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>>>>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>>>>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>>>>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>>> ip_local_deliver+0x3fa/0x480 net/ipv4/ip_input.c:257
>>>>> dst_input ./include/net/dst.h:477
>>>>> ip_rcv_finish+0x6fb/0x1540 net/ipv4/ip_input.c:397
>>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>>> ip_rcv+0x10f6/0x15c0 net/ipv4/ip_input.c:488
>>>>> __netif_receive_skb_core+0x36f6/0x3f60 net/core/dev.c:4298
>>>>> __netif_receive_skb net/core/dev.c:4336
>>>>> netif_receive_skb_internal+0x63c/0x19c0 net/core/dev.c:4497
>>>>> napi_skb_finish net/core/dev.c:4858
>>>>> napi_gro_receive+0x629/0xa50 net/core/dev.c:4889
>>>>> e1000_receive_skb drivers/net/ethernet/intel/e1000/e1000_main.c:4018
>>>>> e1000_clean_rx_irq+0x1492/0x1d30
>>>>> drivers/net/ethernet/intel/e1000/e1000_main.c:4474
>>>>> e1000_clean+0x43aa/0x5970 drivers/net/ethernet/intel/e1000/e1000_main.c:3819
>>>>> napi_poll net/core/dev.c:5500
>>>>> net_rx_action+0x73c/0x1820 net/core/dev.c:5566
>>>>> __do_softirq+0x4b4/0x8dd kernel/softirq.c:284
>>>>> invoke_softirq kernel/softirq.c:364
>>>>> irq_exit+0x203/0x240 kernel/softirq.c:405
>>>>> exiting_irq+0xe/0x10 ./arch/x86/include/asm/apic.h:638
>>>>> do_IRQ+0x15e/0x1a0 arch/x86/kernel/irq.c:263
>>>>> common_interrupt+0x86/0x86
>>>>> ...
>>>>> </IRQ>
>>>>> arch_cpu_idle+0x20/0x30 arch/x86/kernel/process.c:332
>>>>> default_idle_call kernel/sched/idle.c:98
>>>>> cpuidle_idle_call kernel/sched/idle.c:156
>>>>> do_idle+0x334/0x730 kernel/sched/idle.c:246
>>>>> cpu_startup_entry+0x35/0x40 kernel/sched/idle.c:351
>>>>> rest_init+0xb8/0xc0 init/main.c:437
>>>>> start_kernel+0x4d7/0x530 init/main.c:703
>>>>> x86_64_start_reservations arch/x86/kernel/head64.c:318
>>>>> x86_64_start_kernel+0x3cd/0x3e0 arch/x86/kernel/head64.c:299
>>>>> secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:219
>>>>> origin:
>>>>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>>>>> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
>>>>> kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
>>>>> kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
>>>>> kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
>>>>> reqsk_alloc ./include/net/request_sock.h:88
>>>>> inet_reqsk_alloc+0xb8/0x6b0 net/ipv4/tcp_input.c:6231
>>>>> tcp_conn_request+0x89d/0x36f0 net/ipv4/tcp_input.c:6330
>>>>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>>>>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>>>>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>>>>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>>>>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>>> ...
>>>>> ==================================================================
>>>>>
>>>>> --
>>>>> Alexander Potapenko
>>>>> Software Engineer
>>>>>
>>>>> Google Germany GmbH
>>>>> Erika-Mann-Straße, 33
>>>>> 80636 München
>>>>>
>>>>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>>>>> Registergericht und -nummer: Hamburg, HRB 86891
>>>>> Sitz der Gesellschaft: Hamburg
>>>>
>>>>
>>>>
>>>> --
>>>> Alexander Potapenko
>>>> Software Engineer
>>>>
>>>> Google Germany GmbH
>>>> Erika-Mann-Straße, 33
>>>> 80636 München
>>>>
>>>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>>>> Registergericht und -nummer: Hamburg, HRB 86891
>>>> Sitz der Gesellschaft: Hamburg
>
>
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-11-22 13:38 ` Alexander Potapenko
@ 2017-11-22 15:58 ` Eric Dumazet
2017-12-05 14:15 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2017-11-22 15:58 UTC (permalink / raw)
To: Alexander Potapenko; +Cc: Craig Gallek, David Miller, Networking
On Wed, Nov 22, 2017 at 5:38 AM, Alexander Potapenko <glider@google.com> wrote:
> On Thu, Oct 26, 2017 at 4:56 PM, Alexander Potapenko <glider@google.com> wrote:
>> On Thu, Oct 26, 2017 at 4:52 PM, Eric Dumazet <edumazet@google.com> wrote:
>>> On Thu, Oct 26, 2017 at 7:47 AM, Eric Dumazet <edumazet@google.com> wrote:
>>>> On Thu, Oct 26, 2017 at 7:20 AM, Alexander Potapenko <glider@google.com> wrote:
>>>>> On Thu, Oct 26, 2017 at 2:51 PM, Alexander Potapenko <glider@google.com> wrote:
>>>>>> Hi David, Eric,
>>>>>>
>>>>>> I've changed KMSAN instrumentation a bit and it's now reporting a new
>>>>>> error (see below) when I SSH into a VM.
>>>>> I've double-checked the old instrumentation and found a bug in it,
>>>>> which led to masking some of the errors on uninitialized bitfields.
>>>>> I now believe this is a true positive report.
>>>>
>>>>
>>>> Please do not top post on netdev
>> Sorry about that.
>>>> A child is cloned from the listener, check sock_copy()
>>>>
>>>> sk_reuseport is part of the copied fields.
>>>>
>>>> You might have some bug at your side ?
>>>>
>>>
>>> Oh these are request socket.
>>>
>>> This is an harmless bug added in commit d894ba18d4e449b3a7f6eb491f16c9e02933736e
>>> ("soreuseport: fix ordering for mixed v4/v6 sockets")
>>>
>>> I will send a patch, but really this has no effect at all.
>> Thanks for clarifying!
>> For me this was a question of the tool's correctness, because until
>> recently I wasn't able to understand whether this is a true bug or
>> not.
> A friendly ping.
> Eric, did you find the time to send the patch?
Not yet, I am still investigating this issue.
Thanks.
>>>>
>>>>>> For |req| allocated by inet_reqsk_alloc() the value of
>>>>>> req_to_sk(req)->sk_reuseport is uninitialized.
>>>>>> Does this look valid?
>>>>>> I'm a bit surprised this didn't show up before, but I couldn't find
>>>>>> any code to initialize sk_reuseport.
>>>>>>
>>>>>> ==================================================================
>>>>>> BUG: KMSAN: use of uninitialized memory in inet_ehash_insert+0xd40/0x1050
>>>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0+ #3288
>>>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>>>>> Call Trace:
>>>>>> <IRQ>
>>>>>> __dump_stack lib/dump_stack.c:16
>>>>>> dump_stack+0x185/0x1d0 lib/dump_stack.c:52
>>>>>> kmsan_report+0x13f/0x1c0 mm/kmsan/kmsan.c:1016
>>>>>> __msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:766
>>>>>> __sk_nulls_add_node_rcu ./include/net/sock.h:684
>>>>>> inet_ehash_insert+0xd40/0x1050 net/ipv4/inet_hashtables.c:413
>>>>>> reqsk_queue_hash_req net/ipv4/inet_connection_sock.c:754
>>>>>> inet_csk_reqsk_queue_hash_add+0x1cc/0x300 net/ipv4/inet_connection_sock.c:765
>>>>>> tcp_conn_request+0x31e7/0x36f0 net/ipv4/tcp_input.c:6414
>>>>>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>>>>>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>>>>>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>>>>>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>>>>>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>>>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>>>> ip_local_deliver+0x3fa/0x480 net/ipv4/ip_input.c:257
>>>>>> dst_input ./include/net/dst.h:477
>>>>>> ip_rcv_finish+0x6fb/0x1540 net/ipv4/ip_input.c:397
>>>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>>>> ip_rcv+0x10f6/0x15c0 net/ipv4/ip_input.c:488
>>>>>> __netif_receive_skb_core+0x36f6/0x3f60 net/core/dev.c:4298
>>>>>> __netif_receive_skb net/core/dev.c:4336
>>>>>> netif_receive_skb_internal+0x63c/0x19c0 net/core/dev.c:4497
>>>>>> napi_skb_finish net/core/dev.c:4858
>>>>>> napi_gro_receive+0x629/0xa50 net/core/dev.c:4889
>>>>>> e1000_receive_skb drivers/net/ethernet/intel/e1000/e1000_main.c:4018
>>>>>> e1000_clean_rx_irq+0x1492/0x1d30
>>>>>> drivers/net/ethernet/intel/e1000/e1000_main.c:4474
>>>>>> e1000_clean+0x43aa/0x5970 drivers/net/ethernet/intel/e1000/e1000_main.c:3819
>>>>>> napi_poll net/core/dev.c:5500
>>>>>> net_rx_action+0x73c/0x1820 net/core/dev.c:5566
>>>>>> __do_softirq+0x4b4/0x8dd kernel/softirq.c:284
>>>>>> invoke_softirq kernel/softirq.c:364
>>>>>> irq_exit+0x203/0x240 kernel/softirq.c:405
>>>>>> exiting_irq+0xe/0x10 ./arch/x86/include/asm/apic.h:638
>>>>>> do_IRQ+0x15e/0x1a0 arch/x86/kernel/irq.c:263
>>>>>> common_interrupt+0x86/0x86
>>>>>> ...
>>>>>> </IRQ>
>>>>>> arch_cpu_idle+0x20/0x30 arch/x86/kernel/process.c:332
>>>>>> default_idle_call kernel/sched/idle.c:98
>>>>>> cpuidle_idle_call kernel/sched/idle.c:156
>>>>>> do_idle+0x334/0x730 kernel/sched/idle.c:246
>>>>>> cpu_startup_entry+0x35/0x40 kernel/sched/idle.c:351
>>>>>> rest_init+0xb8/0xc0 init/main.c:437
>>>>>> start_kernel+0x4d7/0x530 init/main.c:703
>>>>>> x86_64_start_reservations arch/x86/kernel/head64.c:318
>>>>>> x86_64_start_kernel+0x3cd/0x3e0 arch/x86/kernel/head64.c:299
>>>>>> secondary_startup_64+0x9f/0x9f arch/x86/kernel/head_64.S:219
>>>>>> origin:
>>>>>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>>>>>> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
>>>>>> kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
>>>>>> kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
>>>>>> kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
>>>>>> reqsk_alloc ./include/net/request_sock.h:88
>>>>>> inet_reqsk_alloc+0xb8/0x6b0 net/ipv4/tcp_input.c:6231
>>>>>> tcp_conn_request+0x89d/0x36f0 net/ipv4/tcp_input.c:6330
>>>>>> tcp_v4_conn_request+0x16d/0x220 net/ipv4/tcp_ipv4.c:1314
>>>>>> tcp_rcv_state_process+0x42a/0x7210 net/ipv4/tcp_input.c:5917
>>>>>> tcp_v4_do_rcv+0xa6a/0xcd0 net/ipv4/tcp_ipv4.c:1483
>>>>>> tcp_v4_rcv+0x3de0/0x4ab0 net/ipv4/tcp_ipv4.c:1763
>>>>>> ip_local_deliver_finish+0x6bb/0xcb0 net/ipv4/ip_input.c:216
>>>>>> NF_HOOK ./include/linux/netfilter.h:248
>>>>>> ...
>>>>>> ==================================================================
>>>>>>
>>>>>> --
>>>>>> Alexander Potapenko
>>>>>> Software Engineer
>>>>>>
>>>>>> Google Germany GmbH
>>>>>> Erika-Mann-Straße, 33
>>>>>> 80636 München
>>>>>>
>>>>>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>>>>>> Registergericht und -nummer: Hamburg, HRB 86891
>>>>>> Sitz der Gesellschaft: Hamburg
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Alexander Potapenko
>>>>> Software Engineer
>>>>>
>>>>> Google Germany GmbH
>>>>> Erika-Mann-Straße, 33
>>>>> 80636 München
>>>>>
>>>>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>>>>> Registergericht und -nummer: Hamburg, HRB 86891
>>>>> Sitz der Gesellschaft: Hamburg
>>
>>
>>
>> --
>> Alexander Potapenko
>> Software Engineer
>>
>> Google Germany GmbH
>> Erika-Mann-Straße, 33
>> 80636 München
>>
>> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
>> Registergericht und -nummer: Hamburg, HRB 86891
>> Sitz der Gesellschaft: Hamburg
>
>
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-11-22 15:58 ` Eric Dumazet
@ 2017-12-05 14:15 ` Eric Dumazet
2017-12-05 14:18 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2017-12-05 14:15 UTC (permalink / raw)
To: Eric Dumazet, Alexander Potapenko, cgallek
Cc: Craig Gallek, David Miller, Networking
On Wed, 2017-11-22 at 07:58 -0800, Eric Dumazet wrote:
> On Wed, Nov 22, 2017 at 5:38 AM, Alexander Potapenko <glider@google.c
> om> wrote:
> > On Thu, Oct 26, 2017 at 4:56 PM, Alexander Potapenko <glider@google
> > .com> wrote:
> > > On Thu, Oct 26, 2017 at 4:52 PM, Eric Dumazet <edumazet@google.co
> > > m> wrote:
> > > > On Thu, Oct 26, 2017 at 7:47 AM, Eric Dumazet <edumazet@google.
> > > > com> wrote:
> > > > > On Thu, Oct 26, 2017 at 7:20 AM, Alexander Potapenko <glider@
> > > > > google.com> wrote:
> > > > > > On Thu, Oct 26, 2017 at 2:51 PM, Alexander Potapenko <glide
> > > > > > r@google.com> wrote:
> > > > > > > Hi David, Eric,
> > > > > > >
> > > > > > > I've changed KMSAN instrumentation a bit and it's now
> > > > > > > reporting a new
> > > > > > > error (see below) when I SSH into a VM.
> > > > > >
> > > > > > I've double-checked the old instrumentation and found a bug
> > > > > > in it,
> > > > > > which led to masking some of the errors on uninitialized
> > > > > > bitfields.
> > > > > > I now believe this is a true positive report.
> > > > >
> > > > >
> > > > > Please do not top post on netdev
> > >
> > > Sorry about that.
> > > > > A child is cloned from the listener, check sock_copy()
> > > > >
> > > > > sk_reuseport is part of the copied fields.
> > > > >
> > > > > You might have some bug at your side ?
> > > > >
> > > >
> > > > Oh these are request socket.
> > > >
> > > > This is an harmless bug added in commit
> > > > d894ba18d4e449b3a7f6eb491f16c9e02933736e
> > > > ("soreuseport: fix ordering for mixed v4/v6 sockets")
> > > >
> > > > I will send a patch, but really this has no effect at all.
> > >
> > > Thanks for clarifying!
> > > For me this was a question of the tool's correctness, because
> > > until
> > > recently I wasn't able to understand whether this is a true bug
> > > or
> > > not.
> >
> > A friendly ping.
> > Eric, did you find the time to send the patch?
>
> Not yet, I am still investigating this issue.
>
> Thanks.
Craig, my take on this (minor) bug is that we should be able to remove
hlist_nulls_add_tail_rcu()
The original problem only applies to UDP sockets or TCP/DCCP listeners.
They no longer use sk_nulls_node (SLAB_DESTROY_BY_RCU)
Other sockets have unique 4-tuple so we can not have a mismatch at
lookup time.
Any objection with following patch ?
diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
index a328e8181e49f3a0947dd713daeef35b9d7c831f..e4b257ff881bfe439a945d7487f5700f17a26740 100644
--- a/include/linux/rculist_nulls.h
+++ b/include/linux/rculist_nulls.h
@@ -100,44 +100,6 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
first->pprev = &n->next;
}
-/**
- * hlist_nulls_add_tail_rcu
- * @n: the element to add to the hash list.
- * @h: the list to add to.
- *
- * Description:
- * Adds the specified element to the end of the specified hlist_nulls,
- * while permitting racing traversals. NOTE: tail insertion requires
- * list traversal.
- *
- * The caller must take whatever precautions are necessary
- * (such as holding appropriate locks) to avoid racing
- * with another list-mutation primitive, such as hlist_nulls_add_head_rcu()
- * or hlist_nulls_del_rcu(), running on this same list.
- * However, it is perfectly legal to run concurrently with
- * the _rcu list-traversal primitives, such as
- * hlist_nulls_for_each_entry_rcu(), used to prevent memory-consistency
- * problems on Alpha CPUs. Regardless of the type of CPU, the
- * list-traversal primitive must be guarded by rcu_read_lock().
- */
-static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
- struct hlist_nulls_head *h)
-{
- struct hlist_nulls_node *i, *last = NULL;
-
- for (i = hlist_nulls_first_rcu(h); !is_a_nulls(i);
- i = hlist_nulls_next_rcu(i))
- last = i;
-
- if (last) {
- n->next = last->next;
- n->pprev = &last->next;
- rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
- } else {
- hlist_nulls_add_head_rcu(n, h);
- }
-}
-
/**
* hlist_nulls_for_each_entry_rcu - iterate over rcu list of given type
* @tpos: the type * to use as a loop cursor.
diff --git a/include/net/sock.h b/include/net/sock.h
index 79e1a2c7912c03d8281d449609d57cc909138a3b..8e8adbb8f971546989b185c9f01593c0c0730431 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -685,11 +685,7 @@ static inline void sk_add_node_rcu(struct sock *sk, struct hlist_head *list)
static inline void __sk_nulls_add_node_rcu(struct sock *sk, struct hlist_nulls_head *list)
{
- if (IS_ENABLED(CONFIG_IPV6) && sk->sk_reuseport &&
- sk->sk_family == AF_INET6)
- hlist_nulls_add_tail_rcu(&sk->sk_nulls_node, list);
- else
- hlist_nulls_add_head_rcu(&sk->sk_nulls_node, list);
+ hlist_nulls_add_head_rcu(&sk->sk_nulss_node, list);
}
static inline void sk_nulls_add_node_rcu(struct sock *sk, struct hlist_nulls_head *list)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-12-05 14:15 ` Eric Dumazet
@ 2017-12-05 14:18 ` Eric Dumazet
0 siblings, 0 replies; 12+ messages in thread
From: Eric Dumazet @ 2017-12-05 14:18 UTC (permalink / raw)
To: Eric Dumazet, Alexander Potapenko, cgallek
Cc: Craig Gallek, David Miller, Networking
On Tue, 2017-12-05 at 06:15 -0800, Eric Dumazet wrote:
>
> + hlist_nulls_add_head_rcu(&sk->sk_nulss_node, list);
Typo here, this needs sk_nulls_node of course.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
@ 2017-12-05 19:39 Craig Gallek
2017-12-05 20:07 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Craig Gallek @ 2017-12-05 19:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, Alexander Potapenko, David Miller, Networking
On Tue, Dec 5, 2017 at 9:18 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2017-12-05 at 06:15 -0800, Eric Dumazet wrote:
>>
>> + hlist_nulls_add_head_rcu(&sk->sk_nulss_node, list);
>
> Typo here, this needs sk_nulls_node of course.
>
Thanks Eric, this looks good to me. The tail insertion is still
required in udp_lib_get_port for the second layer hash, but not here.
fwiw, reuseport_dualstack in the selftests directory verifies this
behavior. I tried it with your patch (it still passes) and removing
the udp_lib_get_port path (to make sure it breaks when it should).
Craig
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
2017-12-05 19:39 Uninitialized value in __sk_nulls_add_node_rcu() Craig Gallek
@ 2017-12-05 20:07 ` Eric Dumazet
0 siblings, 0 replies; 12+ messages in thread
From: Eric Dumazet @ 2017-12-05 20:07 UTC (permalink / raw)
To: Craig Gallek; +Cc: Eric Dumazet, Alexander Potapenko, David Miller, Networking
On Tue, 2017-12-05 at 14:39 -0500, Craig Gallek wrote:
> On Tue, Dec 5, 2017 at 9:18 AM, Eric Dumazet <eric.dumazet@gmail.com>
> wrote:
> > On Tue, 2017-12-05 at 06:15 -0800, Eric Dumazet wrote:
> > >
> > > + hlist_nulls_add_head_rcu(&sk->sk_nulss_node, list);
> >
> > Typo here, this needs sk_nulls_node of course.
> >
>
> Thanks Eric, this looks good to me. The tail insertion is still
> required in udp_lib_get_port for the second layer hash, but not here.
> fwiw, reuseport_dualstack in the selftests directory verifies this
> behavior. I tried it with your patch (it still passes) and removing
> the udp_lib_get_port path (to make sure it breaks when it should).
>
Thanks for confirming this, I will send the official patch then ;)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
@ 2017-12-05 20:11 Craig Gallek
0 siblings, 0 replies; 12+ messages in thread
From: Craig Gallek @ 2017-12-05 20:11 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, Alexander Potapenko, David Miller, Networking
On Tue, Dec 5, 2017 at 3:07 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2017-12-05 at 14:39 -0500, Craig Gallek wrote:
>> On Tue, Dec 5, 2017 at 9:18 AM, Eric Dumazet <eric.dumazet@gmail.com>
>> wrote:
>> > On Tue, 2017-12-05 at 06:15 -0800, Eric Dumazet wrote:
>> > >
>> > > + hlist_nulls_add_head_rcu(&sk->sk_nulss_node, list);
>> >
>> > Typo here, this needs sk_nulls_node of course.
>> >
>>
>> Thanks Eric, this looks good to me. The tail insertion is still
>> required in udp_lib_get_port for the second layer hash, but not here.
>> fwiw, reuseport_dualstack in the selftests directory verifies this
>> behavior. I tried it with your patch (it still passes) and removing
>> the udp_lib_get_port path (to make sure it breaks when it should).
>>
>
> Thanks for confirming this, I will send the official patch then ;)
Great, feel free to include my Acked-by.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-12-05 20:11 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-05 19:39 Uninitialized value in __sk_nulls_add_node_rcu() Craig Gallek
2017-12-05 20:07 ` Eric Dumazet
-- strict thread matches above, loose matches on Subject: below --
2017-12-05 20:11 Craig Gallek
2017-10-26 12:51 Alexander Potapenko
2017-10-26 14:20 ` Alexander Potapenko
2017-10-26 14:47 ` Eric Dumazet
2017-10-26 14:52 ` Eric Dumazet
2017-10-26 14:56 ` Alexander Potapenko
2017-11-22 13:38 ` Alexander Potapenko
2017-11-22 15:58 ` Eric Dumazet
2017-12-05 14:15 ` Eric Dumazet
2017-12-05 14:18 ` Eric Dumazet
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).