Re: [PATCH net] inetpeer: fix uninit-value in inet_getpeer
On 04/09/2018 07:58 AM, David Miller wrote: > From: Eric Dumazet > Date: Mon, 9 Apr 2018 06:43:27 -0700 > >> syzbot/KMSAN reported that p->dtime was read while it was >> not yet initialized in : >> >> delta = (__u32)jiffies - p->dtime; >> if (delta < ttl || !refcount_dec_if_one(&p->refcnt)) >> gc_stack[i] = NULL; >> >> This is a false positive, because the inetpeer wont be erased >> from rb-tree if the refcount_dec_if_one(&p->refcnt) does not >> succeed. And this wont happen before first inet_putpeer() call >> for this inetpeer has been done, and ->dtime field is written >> exactly before the refcount_dec_and_test(&p->refcnt). >> >> The KMSAN report was : > ... >> Signed-off-by: Eric Dumazet >> Reported-by: syzbot > > Applied, but it looks like we are just adding assignments simply > to placate these reports when the tools and facilities cannot > see through the logic properly. > To be fair, this is because the check on ->dtime should be done a second time after the refcount_dec_if_one(&p->refcnt) It is a tiny race, and we do not really care given nature of inetpeer cache, best effort, and DDOS candidate anyway. If we purge one entry too soon, this is not a big deal. I believe tool is fine.
Re: [PATCH net] inetpeer: fix uninit-value in inet_getpeer
From: Eric Dumazet Date: Mon, 9 Apr 2018 06:43:27 -0700 > syzbot/KMSAN reported that p->dtime was read while it was > not yet initialized in : > > delta = (__u32)jiffies - p->dtime; > if (delta < ttl || !refcount_dec_if_one(&p->refcnt)) > gc_stack[i] = NULL; > > This is a false positive, because the inetpeer wont be erased > from rb-tree if the refcount_dec_if_one(&p->refcnt) does not > succeed. And this wont happen before first inet_putpeer() call > for this inetpeer has been done, and ->dtime field is written > exactly before the refcount_dec_and_test(&p->refcnt). > > The KMSAN report was : ... > Signed-off-by: Eric Dumazet > Reported-by: syzbot Applied, but it looks like we are just adding assignments simply to placate these reports when the tools and facilities cannot see through the logic properly.
[PATCH net] inetpeer: fix uninit-value in inet_getpeer
syzbot/KMSAN reported that p->dtime was read while it was not yet initialized in : delta = (__u32)jiffies - p->dtime; if (delta < ttl || !refcount_dec_if_one(&p->refcnt)) gc_stack[i] = NULL; This is a false positive, because the inetpeer wont be erased from rb-tree if the refcount_dec_if_one(&p->refcnt) does not succeed. And this wont happen before first inet_putpeer() call for this inetpeer has been done, and ->dtime field is written exactly before the refcount_dec_and_test(&p->refcnt). The KMSAN report was : BUG: KMSAN: uninit-value in inet_peer_gc net/ipv4/inetpeer.c:163 [inline] BUG: KMSAN: uninit-value in inet_getpeer+0x1567/0x1e70 net/ipv4/inetpeer.c:228 CPU: 0 PID: 9494 Comm: syz-executor5 Not tainted 4.16.0+ #82 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0x185/0x1d0 lib/dump_stack.c:53 kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067 __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676 inet_peer_gc net/ipv4/inetpeer.c:163 [inline] inet_getpeer+0x1567/0x1e70 net/ipv4/inetpeer.c:228 inet_getpeer_v4 include/net/inetpeer.h:110 [inline] icmpv4_xrlim_allow net/ipv4/icmp.c:330 [inline] icmp_send+0x2b44/0x3050 net/ipv4/icmp.c:725 ip_options_compile+0x237c/0x29f0 net/ipv4/ip_options.c:472 ip_rcv_options net/ipv4/ip_input.c:284 [inline] ip_rcv_finish+0xda8/0x16d0 net/ipv4/ip_input.c:365 NF_HOOK include/linux/netfilter.h:288 [inline] ip_rcv+0x119d/0x16f0 net/ipv4/ip_input.c:493 __netif_receive_skb_core+0x47cf/0x4a80 net/core/dev.c:4562 __netif_receive_skb net/core/dev.c:4627 [inline] netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701 netif_receive_skb+0x230/0x240 net/core/dev.c:4725 tun_rx_batched drivers/net/tun.c:1555 [inline] tun_get_user+0x6d88/0x7580 drivers/net/tun.c:1962 tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990 do_iter_readv_writev+0x7bb/0x970 include/linux/fs.h:1776 do_iter_write+0x30d/0xd40 fs/read_write.c:932 vfs_writev fs/read_write.c:977 [inline] do_writev+0x3c9/0x830 fs/read_write.c:1012 SYSC_writev+0x9b/0xb0 fs/read_write.c:1085 SyS_writev+0x56/0x80 fs/read_write.c:1082 do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x3d/0xa2 RIP: 0033:0x455111 RSP: 002b:7fae0365cba0 EFLAGS: 0293 ORIG_RAX: 0014 RAX: ffda RBX: 002e RCX: 00455111 RDX: 0001 RSI: 7fae0365cbf0 RDI: 00fc RBP: 2040 R08: 00fc R09: R10: 002e R11: 0293 R12: R13: 0658 R14: 006fc8e0 R15: Uninit was created at: kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline] kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188 kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314 kmem_cache_alloc+0xaab/0xb90 mm/slub.c:2756 inet_getpeer+0xed8/0x1e70 net/ipv4/inetpeer.c:210 inet_getpeer_v4 include/net/inetpeer.h:110 [inline] ip4_frag_init+0x4d1/0x740 net/ipv4/ip_fragment.c:153 inet_frag_alloc net/ipv4/inet_fragment.c:369 [inline] inet_frag_create net/ipv4/inet_fragment.c:385 [inline] inet_frag_find+0x7da/0x1610 net/ipv4/inet_fragment.c:418 ip_find net/ipv4/ip_fragment.c:275 [inline] ip_defrag+0x448/0x67a0 net/ipv4/ip_fragment.c:676 ip_check_defrag+0x775/0xda0 net/ipv4/ip_fragment.c:724 packet_rcv_fanout+0x2a8/0x8d0 net/packet/af_packet.c:1447 deliver_skb net/core/dev.c:1897 [inline] deliver_ptype_list_skb net/core/dev.c:1912 [inline] __netif_receive_skb_core+0x314a/0x4a80 net/core/dev.c:4545 __netif_receive_skb net/core/dev.c:4627 [inline] netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701 netif_receive_skb+0x230/0x240 net/core/dev.c:4725 tun_rx_batched drivers/net/tun.c:1555 [inline] tun_get_user+0x6d88/0x7580 drivers/net/tun.c:1962 tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990 do_iter_readv_writev+0x7bb/0x970 include/linux/fs.h:1776 do_iter_write+0x30d/0xd40 fs/read_write.c:932 vfs_writev fs/read_write.c:977 [inline] do_writev+0x3c9/0x830 fs/read_write.c:1012 SYSC_writev+0x9b/0xb0 fs/read_write.c:1085 SyS_writev+0x56/0x80 fs/read_write.c:1082 do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x3d/0xa2 Signed-off-by: Eric Dumazet Reported-by: syzbot --- net/ipv4/inetpeer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c index 1f04bd91fc2e999ddb82f4be92d39d229166b691..d757b9642d0d1c418bffad5bcd50e8e7bf336c66 100644 --- a/net/ipv4/inetpeer.c +++ b/net/ipv4/inetpeer.c @@ -211,6 +211,7 @@ struct inet_peer *inet_getpeer(struct inet_peer_base *base, p = kmem_cache_alloc(peer_cachep, GFP_ATOMIC); if (p) { p->daddr = *daddr; + p->dtime = (__u32)jiffies; refcount_set(&p->refcnt, 2);