Re: dccp: potential deadlock in dccp_v4_ctl_send_reset

From: Eric Dumazet
Date: Tue Jul 05 2016 - 13:32:49 EST


On Tue, 2016-07-05 at 10:17 -0700, Cong Wang wrote:
> On Tue, Jul 5, 2016 at 4:59 AM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> > other info that might help us debug this:
> > Possible unsafe locking scenario:
> >
> > CPU0
> > ----
> > lock(slock-AF_INET);
> > <Interrupt>
> > lock(slock-AF_INET);
> >
> > *** DEADLOCK ***
> >
> > 1 lock held by syz-executor/354:
> > #0: (sk_lock-AF_INET){+.+.+.}, at: [< inline >] lock_sock
> > include/net/sock.h:1388
> > #0: (sk_lock-AF_INET){+.+.+.}, at: [<ffffffff85d193f4>]
> > inet_stream_connect+0x44/0xa0 net/ipv4/af_inet.c:660
> >
> > stack backtrace:
> > CPU: 3 PID: 354 Comm: syz-executor Not tainted 4.7.0-rc5+ #28
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> > ffffffff880b58e0 ffff8800361378c0 ffffffff82cc01af ffffffff00000000
> > fffffbfff1016b1c ffff88003abfe840 ffffffff899bb700 ffff88003abff0a8
> > ffffffff86cae460 0000000000000001 ffff880036137930 ffffffff8147684d
> > Call Trace:
> > [< inline >] __dump_stack lib/dump_stack.c:15
> > [<ffffffff82cc01af>] dump_stack+0x12e/0x18f lib/dump_stack.c:51
> > [<ffffffff8147684d>] print_usage_bug+0x34d/0x3a0 kernel/locking/lockdep.c:2383
> > [< inline >] valid_state kernel/locking/lockdep.c:2396
> > [< inline >] mark_lock_irq kernel/locking/lockdep.c:2594
> > [<ffffffff8147748c>] mark_lock+0xbec/0xe80 kernel/locking/lockdep.c:3057
> > [< inline >] mark_irqflags kernel/locking/lockdep.c:2933
> > [<ffffffff814793ce>] __lock_acquire+0xd3e/0x2fb0 kernel/locking/lockdep.c:3287
> > [<ffffffff8147c293>] lock_acquire+0x1e3/0x460 kernel/locking/lockdep.c:3741
> > [< inline >] __raw_spin_lock include/linux/spinlock_api_smp.h:144
> > [<ffffffff86a93f83>] _raw_spin_lock+0x33/0x50 kernel/locking/spinlock.c:151
> > [< inline >] spin_lock include/linux/spinlock.h:302
> > [<ffffffff864831b1>] dccp_v4_ctl_send_reset+0xac1/0x10d0 net/dccp/ipv4.c:530
> > [<ffffffff864838b9>] dccp_v4_do_rcv+0xf9/0x190 net/dccp/ipv4.c:684
> > [< inline >] sk_backlog_rcv include/net/sock.h:872
> > [<ffffffff858b42c7>] __release_sock+0x127/0x3a0 net/core/sock.c:2058
> > [<ffffffff858b4599>] release_sock+0x59/0x1c0 net/core/sock.c:2516
> > [<ffffffff85d19428>] inet_stream_connect+0x78/0xa0 net/ipv4/af_inet.c:662
> > [<ffffffff858a62ae>] SYSC_connect+0x23e/0x2e0 net/socket.c:1536
> > [<ffffffff858ab3d4>] SyS_connect+0x24/0x30 net/socket.c:1517
> > [<ffffffff86a94e00>] entry_SYSCALL_64_fastpath+0x23/0xc1
> > arch/x86/entry/entry_64.S:207
>
> This is probably a known deadlock for sk backlog recv path,
> at least the comments on tcp_v4_do_rcv() mentioned this:
>
>
> * We have a potential double-lock case here, so even when
> * doing backlog processing we use the BH locking scheme.
> * This is because we cannot sleep with the original spinlock
> * held.
>
> the ->sk_backlog_rcv() is called in process context, which
> is not supposed to hold bh_lock_sock, but most of its
> implementations are called in BH context too... Interesting...


Very similar to a previous report in TCP stack, fixed
in commit 47dcc20a39d06585bf3cb9fb381f0e81c20002c3
("ipv4: tcp: ip_send_unicast_reply() is not BH safe")

I would try :

diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 5c7e413a3ae407e67565b48a8bd6f43e3b02de4d..3cdf5b29ce451e7b7c3290ebb231cf5f4e43f202 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -527,10 +527,12 @@ static void dccp_v4_ctl_send_reset(const struct sock *sk, struct sk_buff *rxskb)
rxiph->daddr);
skb_dst_set(skb, dst_clone(dst));

+ local_bh_disable();
bh_lock_sock(ctl_sk);
err = ip_build_and_send_pkt(skb, ctl_sk,
rxiph->daddr, rxiph->saddr, NULL);
bh_unlock_sock(ctl_sk);
+ local_bh_enable();

if (net_xmit_eval(err) == 0) {
DCCP_INC_STATS(DCCP_MIB_OUTSEGS);