inet: fix request sock refcounting

While testing last patch series, I found req sock refcounting was wrong.

We must set skc_refcnt to 1 for all request socks added in hashes,
but also on request sockets created by FastOpen or syncookies.

It is tricky because we need to defer this initialization so that
future RCU lookups do not try to take a refcount on a not yet
fully initialized request socket.

Also get rid of ireq_refcnt alias.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: 13854e5a6046 ("inet: add proper refcounting to request sock")
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 191feec60205d21bfc344e6ff098cf1f857e00b1..b9a6b0a94cc6b52a70158dd2bc7eb847baa6bed0 100644 (file)
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -275,11 +275,6 @@ static inline void inet_csk_reqsk_queue_add(struct sock *sk,
                                            struct sock *child)
 {
        reqsk_queue_add(&inet_csk(sk)->icsk_accept_queue, req, sk, child);
-       /* before letting lookups find us, make sure all req fields
-        * are committed to memory.
-        */
-       smp_wmb();
-       atomic_set(&req->rsk_refcnt, 1);
 }
 
 void inet_csk_reqsk_queue_hash_add(struct sock *sk, struct request_sock *req,

diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 6fec7343070f2de1d8dca6071fcb5001e7dccd2d..b6c3737da4e94404585a97f59ad7a4e2e1f6e105 100644 (file)
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -81,7 +81,6 @@ struct inet_request_sock {
 #define ir_cookie              req.__req_common.skc_cookie
 #define ireq_net               req.__req_common.skc_net
 #define ireq_state             req.__req_common.skc_state
-#define ireq_refcnt            req.__req_common.skc_refcnt
 #define ireq_family            req.__req_common.skc_family
 
        kmemcheck_bitfield_begin(flags);

diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index 723d1cbdf20ebe4f31ff70f6fe97fccaac675d50..3fa4f824900a78b9c1d75a94e87c6fd89056ec07 100644 (file)
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -77,6 +77,11 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener)
                req->rsk_ops = ops;
                sock_hold(sk_listener);
                req->rsk_listener = sk_listener;
+
+               /* Following is temporary. It is coupled with debugging
+                * helpers in reqsk_put() & reqsk_free()
+                */
+               atomic_set(&req->rsk_refcnt, 0);
        }
        return req;
 }
@@ -292,6 +297,12 @@ static inline void reqsk_queue_hash_req(struct request_sock_queue *queue,
        req->sk = NULL;
        req->dl_next = lopt->syn_table[hash];
 
+       /* before letting lookups find us, make sure all req fields
+        * are committed to memory and refcnt initialized.
+        */
+       smp_wmb();
+       atomic_set(&req->rsk_refcnt, 1);
+
        write_lock(&queue->syn_wait_lock);
        lopt->syn_table[hash] = req;
        write_unlock(&queue->syn_wait_lock);

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 574b67765a06017e12d9f8f45ad24b43c922772c..34e755403715812fbc125f1fa6d97964875ac7c0 100644 (file)
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -227,11 +227,12 @@ static struct sock *get_cookie_sock(struct sock *sk, struct sk_buff *skb,
        struct sock *child;
 
        child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst);
-       if (child)
+       if (child) {
+               atomic_set(&req->rsk_refcnt, 1);
                inet_csk_reqsk_queue_add(sk, req, child);
-       else
+       } else {
                reqsk_free(req);
-
+       }
        return child;
 }
 
@@ -356,7 +357,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
        ireq->opt = tcp_v4_save_options(skb);
 
        if (security_inet_conn_request(sk, skb, req)) {
-               reqsk_put(req);
+               reqsk_free(req);
                goto out;
        }
 
@@ -377,7 +378,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
        security_req_classify_flow(req, flowi4_to_flowi(&fl4));
        rt = ip_route_output_key(sock_net(sk), &fl4);
        if (IS_ERR(rt)) {
-               reqsk_put(req);
+               reqsk_free(req);
                goto out;
        }
 

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 186fd394ec0afc173b9b5307846ccd7f5995a1f6..82e375a0cbcf224e242473b80727b4128c90a9a2 100644 (file)
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -169,6 +169,7 @@ static bool tcp_fastopen_create_child(struct sock *sk,
        inet_csk_reset_xmit_timer(child, ICSK_TIME_RETRANS,
                                  TCP_TIMEOUT_INIT, TCP_RTO_MAX);
 
+       atomic_set(&req->rsk_refcnt, 1);
        /* Add the child socket directly into the accept queue */
        inet_csk_reqsk_queue_add(sk, req, child);
 

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a94ddb96fc857e11c5b95baa0feadbd155a139ab..1dfbaee3554e5d0e580686e649ba44cce1305304 100644 (file)
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5981,10 +5981,6 @@ struct request_sock *inet_reqsk_alloc(const struct request_sock_ops *ops,
                ireq->ireq_state = TCP_NEW_SYN_RECV;
                write_pnet(&ireq->ireq_net, sock_net(sk_listener));
 
-               /* Following is temporary. It is coupled with debugging
-                * helpers in reqsk_put() & reqsk_free()
-                */
-               atomic_set(&ireq->ireq_refcnt, 0);
        }
 
        return req;

diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index 1ef0c926ce9dc4ecfc9ddae364394c170b4184aa..da5823e5e5a7634aadcdaa3ea30b7a60a278b4af 100644 (file)
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -49,11 +49,12 @@ static inline struct sock *get_cookie_sock(struct sock *sk, struct sk_buff *skb,
        struct sock *child;
 
        child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst);
-       if (child)
+       if (child) {
+               atomic_set(&req->rsk_refcnt, 1);
                inet_csk_reqsk_queue_add(sk, req, child);
-       else
+       } else {
                reqsk_free(req);
-
+       }
        return child;
 }