tcp/dccp: fix lockdep issue when SYN is backlogged
authorEric Dumazet <edumazet@google.com>
Mon, 1 Oct 2018 22:02:26 +0000 (15:02 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 18 Oct 2018 07:13:23 +0000 (09:13 +0200)
[ Upstream commit 1ad98e9d1bdf4724c0a8532fabd84bf3c457c2bc ]

In normal SYN processing, packets are handled without listener
lock and in RCU protected ingress path.

But syzkaller is known to be able to trick us and SYN
packets might be processed in process context, after being
queued into socket backlog.

In commit 06f877d613be ("tcp/dccp: fix other lockdep splats
accessing ireq_opt") I made a very stupid fix, that happened
to work mostly because of the regular path being RCU protected.

Really the thing protecting ireq->ireq_opt is RCU read lock,
and the pseudo request refcnt is not relevant.

This patch extends what I did in commit 449809a66c1d ("tcp/dccp:
block BH for SYN processing") by adding an extra rcu_read_{lock|unlock}
pair in the paths that might be taken when processing SYN from
socket backlog (thus possibly in process context)

Fixes: 06f877d613be ("tcp/dccp: fix other lockdep splats accessing ireq_opt")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
include/net/inet_sock.h
net/dccp/input.c
net/ipv4/tcp_input.c

index 0464b207d0cfd2f2659b2eade5194029d7fdb85a..02255bf8c4d26d88676c9d26b52b2591ad1f262a 100644 (file)
@@ -134,8 +134,7 @@ static inline int inet_request_bound_dev_if(const struct sock *sk,
 
 static inline struct ip_options_rcu *ireq_opt_deref(const struct inet_request_sock *ireq)
 {
-       return rcu_dereference_check(ireq->ireq_opt,
-                                    atomic_read(&ireq->req.rsk_refcnt) > 0);
+       return rcu_dereference(ireq->ireq_opt);
 }
 
 struct inet_cork {
index 4a05d78768502df69275b4f91cb03bb2ada9f4c3..84ff43acd4272b054ffe5c31b3b231d4550fb776 100644 (file)
@@ -605,11 +605,13 @@ int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
        if (sk->sk_state == DCCP_LISTEN) {
                if (dh->dccph_type == DCCP_PKT_REQUEST) {
                        /* It is possible that we process SYN packets from backlog,
-                        * so we need to make sure to disable BH right there.
+                        * so we need to make sure to disable BH and RCU right there.
                         */
+                       rcu_read_lock();
                        local_bh_disable();
                        acceptable = inet_csk(sk)->icsk_af_ops->conn_request(sk, skb) >= 0;
                        local_bh_enable();
+                       rcu_read_unlock();
                        if (!acceptable)
                                return 1;
                        consume_skb(skb);
index 9d0b73aa649fcd157e352f163d1dd0e11a0b4333..3f8b5d3c1e595cd29080dd24e018bc8db4acccdc 100644 (file)
@@ -5978,11 +5978,13 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
                        if (th->fin)
                                goto discard;
                        /* It is possible that we process SYN packets from backlog,
-                        * so we need to make sure to disable BH right there.
+                        * so we need to make sure to disable BH and RCU right there.
                         */
+                       rcu_read_lock();
                        local_bh_disable();
                        acceptable = icsk->icsk_af_ops->conn_request(sk, skb) >= 0;
                        local_bh_enable();
+                       rcu_read_unlock();
 
                        if (!acceptable)
                                return 1;