Commit 0470c8ca authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

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: default avatarEric Dumazet <edumazet@google.com>
Fixes: 13854e5a ("inet: add proper refcounting to request sock")
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent e3d95ad7
...@@ -275,11 +275,6 @@ static inline void inet_csk_reqsk_queue_add(struct sock *sk, ...@@ -275,11 +275,6 @@ static inline void inet_csk_reqsk_queue_add(struct sock *sk,
struct sock *child) struct sock *child)
{ {
reqsk_queue_add(&inet_csk(sk)->icsk_accept_queue, req, sk, 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, void inet_csk_reqsk_queue_hash_add(struct sock *sk, struct request_sock *req,
......
...@@ -81,7 +81,6 @@ struct inet_request_sock { ...@@ -81,7 +81,6 @@ struct inet_request_sock {
#define ir_cookie req.__req_common.skc_cookie #define ir_cookie req.__req_common.skc_cookie
#define ireq_net req.__req_common.skc_net #define ireq_net req.__req_common.skc_net
#define ireq_state req.__req_common.skc_state #define ireq_state req.__req_common.skc_state
#define ireq_refcnt req.__req_common.skc_refcnt
#define ireq_family req.__req_common.skc_family #define ireq_family req.__req_common.skc_family
kmemcheck_bitfield_begin(flags); kmemcheck_bitfield_begin(flags);
......
...@@ -77,6 +77,11 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener) ...@@ -77,6 +77,11 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener)
req->rsk_ops = ops; req->rsk_ops = ops;
sock_hold(sk_listener); sock_hold(sk_listener);
req->rsk_listener = 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; return req;
} }
...@@ -292,6 +297,12 @@ static inline void reqsk_queue_hash_req(struct request_sock_queue *queue, ...@@ -292,6 +297,12 @@ static inline void reqsk_queue_hash_req(struct request_sock_queue *queue,
req->sk = NULL; req->sk = NULL;
req->dl_next = lopt->syn_table[hash]; 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); write_lock(&queue->syn_wait_lock);
lopt->syn_table[hash] = req; lopt->syn_table[hash] = req;
write_unlock(&queue->syn_wait_lock); write_unlock(&queue->syn_wait_lock);
......
...@@ -227,11 +227,12 @@ static struct sock *get_cookie_sock(struct sock *sk, struct sk_buff *skb, ...@@ -227,11 +227,12 @@ static struct sock *get_cookie_sock(struct sock *sk, struct sk_buff *skb,
struct sock *child; struct sock *child;
child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst); 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); inet_csk_reqsk_queue_add(sk, req, child);
else } else {
reqsk_free(req); reqsk_free(req);
}
return child; return child;
} }
...@@ -356,7 +357,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) ...@@ -356,7 +357,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
ireq->opt = tcp_v4_save_options(skb); ireq->opt = tcp_v4_save_options(skb);
if (security_inet_conn_request(sk, skb, req)) { if (security_inet_conn_request(sk, skb, req)) {
reqsk_put(req); reqsk_free(req);
goto out; goto out;
} }
...@@ -377,7 +378,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) ...@@ -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)); security_req_classify_flow(req, flowi4_to_flowi(&fl4));
rt = ip_route_output_key(sock_net(sk), &fl4); rt = ip_route_output_key(sock_net(sk), &fl4);
if (IS_ERR(rt)) { if (IS_ERR(rt)) {
reqsk_put(req); reqsk_free(req);
goto out; goto out;
} }
......
...@@ -169,6 +169,7 @@ static bool tcp_fastopen_create_child(struct sock *sk, ...@@ -169,6 +169,7 @@ static bool tcp_fastopen_create_child(struct sock *sk,
inet_csk_reset_xmit_timer(child, ICSK_TIME_RETRANS, inet_csk_reset_xmit_timer(child, ICSK_TIME_RETRANS,
TCP_TIMEOUT_INIT, TCP_RTO_MAX); TCP_TIMEOUT_INIT, TCP_RTO_MAX);
atomic_set(&req->rsk_refcnt, 1);
/* Add the child socket directly into the accept queue */ /* Add the child socket directly into the accept queue */
inet_csk_reqsk_queue_add(sk, req, child); inet_csk_reqsk_queue_add(sk, req, child);
......
...@@ -5981,10 +5981,6 @@ struct request_sock *inet_reqsk_alloc(const struct request_sock_ops *ops, ...@@ -5981,10 +5981,6 @@ struct request_sock *inet_reqsk_alloc(const struct request_sock_ops *ops,
ireq->ireq_state = TCP_NEW_SYN_RECV; ireq->ireq_state = TCP_NEW_SYN_RECV;
write_pnet(&ireq->ireq_net, sock_net(sk_listener)); 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; return req;
......
...@@ -49,11 +49,12 @@ static inline struct sock *get_cookie_sock(struct sock *sk, struct sk_buff *skb, ...@@ -49,11 +49,12 @@ static inline struct sock *get_cookie_sock(struct sock *sk, struct sk_buff *skb,
struct sock *child; struct sock *child;
child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst); 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); inet_csk_reqsk_queue_add(sk, req, child);
else } else {
reqsk_free(req); reqsk_free(req);
}
return child; return child;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment