| Message ID | 20190927230031.3859970-1-kafai@fb.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: Unpublish sk from sk_reuseport_cb before call_rcu | expand |
From: Martin KaFai Lau <kafai@fb.com> Date: Fri, 27 Sep 2019 16:00:31 -0700 > The "reuse->sock[]" array is shared by multiple sockets. The going away > sk must unpublish itself from "reuse->sock[]" before making call_rcu() > call. However, this unpublish-action is currently done after a grace > period and it may cause use-after-free. > > The fix is to move reuseport_detach_sock() to sk_destruct(). > Due to the above reason, any socket with sk_reuseport_cb has > to go through the rcu grace period before freeing it. > > It is a rather old bug (~3 yrs). The Fixes tag is not necessary > the right commit but it is the one that introduced the SOCK_RCU_FREE > logic and this fix is depending on it. > > Fixes: a4298e4522d6 ("net: add SOCK_RCU_FREE socket flag") > Cc: Eric Dumazet <eric.dumazet@gmail.com> > Suggested-by: Eric Dumazet <eric.dumazet@gmail.com> > Signed-off-by: Martin KaFai Lau <kafai@fb.com> Applied and queued up for -stable, thanks Martin.
diff --git a/net/core/sock.c b/net/core/sock.c index 07863edbe6fc..e23ec80a67bf 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1700,8 +1700,6 @@ static void __sk_destruct(struct rcu_head *head) sk_filter_uncharge(sk, filter); RCU_INIT_POINTER(sk->sk_filter, NULL); } - if (rcu_access_pointer(sk->sk_reuseport_cb)) - reuseport_detach_sock(sk); sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP); @@ -1728,7 +1726,14 @@ static void __sk_destruct(struct rcu_head *head) void sk_destruct(struct sock *sk) { - if (sock_flag(sk, SOCK_RCU_FREE)) + bool use_call_rcu = sock_flag(sk, SOCK_RCU_FREE); + + if (rcu_access_pointer(sk->sk_reuseport_cb)) { + reuseport_detach_sock(sk); + use_call_rcu = true; + } + + if (use_call_rcu) call_rcu(&sk->sk_rcu, __sk_destruct); else __sk_destruct(&sk->sk_rcu);
The "reuse->sock[]" array is shared by multiple sockets. The going away sk must unpublish itself from "reuse->sock[]" before making call_rcu() call. However, this unpublish-action is currently done after a grace period and it may cause use-after-free. The fix is to move reuseport_detach_sock() to sk_destruct(). Due to the above reason, any socket with sk_reuseport_cb has to go through the rcu grace period before freeing it. It is a rather old bug (~3 yrs). The Fixes tag is not necessary the right commit but it is the one that introduced the SOCK_RCU_FREE logic and this fix is depending on it. Fixes: a4298e4522d6 ("net: add SOCK_RCU_FREE socket flag") Cc: Eric Dumazet <eric.dumazet@gmail.com> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: Martin KaFai Lau <kafai@fb.com> --- net/core/sock.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)