This is a note to let you know that I've just added the patch titled
l2tp: close all race conditions in l2tp_tunnel_register()
to the 5.15-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
l2tp-close-all-race-conditions-in-l2tp_tunnel_regist.patch
and it can be found in the queue-5.15 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
commit c51a28d0eff6ba9ecd122c74f45ef4b2464bd2df
Author: Cong Wang <cong.wang@xxxxxxxxxxxxx>
Date: Fri Jan 13 19:01:37 2023 -0800
l2tp: close all race conditions in l2tp_tunnel_register()
[ Upstream commit 0b2c59720e65885a394a017d0cf9cab118914682 ]
The code in l2tp_tunnel_register() is racy in several ways:
1. It modifies the tunnel socket _after_ publishing it.
2. It calls setup_udp_tunnel_sock() on an existing socket without
locking.
3. It changes sock lock class on fly, which triggers many syzbot
reports.
This patch amends all of them by moving socket initialization code
before publishing and under sock lock. As suggested by Jakub, the
l2tp lockdep class is not necessary as we can just switch to
bh_lock_sock_nested().
Fixes: 37159ef2c1ae ("l2tp: fix a lockdep splat")
Fixes: 6b9f34239b00 ("l2tp: fix races in tunnel creation")
Reported-by: syzbot+52866e24647f9a23403f@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: syzbot+94cc2a66fc228b23f360@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
Cc: Guillaume Nault <gnault@xxxxxxxxxx>
Cc: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
Cc: Tom Parkin <tparkin@xxxxxxxxxxx>
Signed-off-by: Cong Wang <cong.wang@xxxxxxxxxxxxx>
Reviewed-by: Guillaume Nault <gnault@xxxxxxxxxx>
Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 68d8b2aa6f6d..4c5227048be4 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1041,7 +1041,7 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb, uns
IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED | IPSKB_REROUTED);
nf_reset_ct(skb);
- bh_lock_sock(sk);
+ bh_lock_sock_nested(sk);
if (sock_owned_by_user(sk)) {
kfree_skb(skb);
ret = NET_XMIT_DROP;
@@ -1387,8 +1387,6 @@ static int l2tp_tunnel_sock_create(struct net *net,
return err;
}
-static struct lock_class_key l2tp_socket_class;
-
int l2tp_tunnel_create(int fd, int version, u32 tunnel_id, u32 peer_tunnel_id,
struct l2tp_tunnel_cfg *cfg, struct l2tp_tunnel **tunnelp)
{
@@ -1484,21 +1482,16 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,
}
sk = sock->sk;
+ lock_sock(sk);
write_lock_bh(&sk->sk_callback_lock);
ret = l2tp_validate_socket(sk, net, tunnel->encap);
- if (ret < 0)
+ if (ret < 0) {
+ release_sock(sk);
goto err_inval_sock;
+ }
rcu_assign_sk_user_data(sk, tunnel);
write_unlock_bh(&sk->sk_callback_lock);
- sock_hold(sk);
- tunnel->sock = sk;
- tunnel->l2tp_net = net;
-
- spin_lock_bh(&pn->l2tp_tunnel_idr_lock);
- idr_replace(&pn->l2tp_tunnel_idr, tunnel, tunnel->tunnel_id);
- spin_unlock_bh(&pn->l2tp_tunnel_idr_lock);
-
if (tunnel->encap == L2TP_ENCAPTYPE_UDP) {
struct udp_tunnel_sock_cfg udp_cfg = {
.sk_user_data = tunnel,
@@ -1512,9 +1505,16 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,
tunnel->old_sk_destruct = sk->sk_destruct;
sk->sk_destruct = &l2tp_tunnel_destruct;
- lockdep_set_class_and_name(&sk->sk_lock.slock, &l2tp_socket_class,
- "l2tp_sock");
sk->sk_allocation = GFP_ATOMIC;
+ release_sock(sk);
+
+ sock_hold(sk);
+ tunnel->sock = sk;
+ tunnel->l2tp_net = net;
+
+ spin_lock_bh(&pn->l2tp_tunnel_idr_lock);
+ idr_replace(&pn->l2tp_tunnel_idr, tunnel, tunnel->tunnel_id);
+ spin_unlock_bh(&pn->l2tp_tunnel_idr_lock);
trace_register_tunnel(tunnel);
