netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.11 001/244] l2tp: prevent possible tunnel refcount underflow
@ 2024-09-25 11:23 Sasha Levin
  2024-09-25 11:23 ` [PATCH AUTOSEL 6.11 009/244] ice: Adjust over allocation of memory in ice_sched_add_root_node() and ice_sched_add_node() Sasha Levin
                   ` (28 more replies)
  0 siblings, 29 replies; 34+ messages in thread
From: Sasha Levin @ 2024-09-25 11:23 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: James Chapman, Tom Parkin, David S . Miller, Sasha Levin,
	edumazet, kuba, pabeni, samuel.thibault, thorsten.blum, netdev

From: James Chapman <jchapman@katalix.com>

[ Upstream commit 24256415d18695b46da06c93135f5b51c548b950 ]

When a session is created, it sets a backpointer to its tunnel. When
the session refcount drops to 0, l2tp_session_free drops the tunnel
refcount if session->tunnel is non-NULL. However, session->tunnel is
set in l2tp_session_create, before the tunnel refcount is incremented
by l2tp_session_register, which leaves a small window where
session->tunnel is non-NULL when the tunnel refcount hasn't been
bumped.

Moving the assignment to l2tp_session_register is trivial but
l2tp_session_create calls l2tp_session_set_header_len which uses
session->tunnel to get the tunnel's encap. Add an encap arg to
l2tp_session_set_header_len to avoid using session->tunnel.

If l2tpv3 sessions have colliding IDs, it is possible for
l2tp_v3_session_get to race with l2tp_session_register and fetch a
session which doesn't yet have session->tunnel set. Add a check for
this case.

Signed-off-by: James Chapman <jchapman@katalix.com>
Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/l2tp/l2tp_core.c    | 24 +++++++++++++++++-------
 net/l2tp/l2tp_core.h    |  3 ++-
 net/l2tp/l2tp_netlink.c |  4 +++-
 net/l2tp/l2tp_ppp.c     |  3 ++-
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 2e86f520f7994..a9cbcbc9d016d 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -254,7 +254,14 @@ struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk,
 
 		hash_for_each_possible_rcu(pn->l2tp_v3_session_htable, session,
 					   hlist, key) {
-			if (session->tunnel->sock == sk &&
+			/* session->tunnel may be NULL if another thread is in
+			 * l2tp_session_register and has added an item to
+			 * l2tp_v3_session_htable but hasn't yet added the
+			 * session to its tunnel's session_list.
+			 */
+			struct l2tp_tunnel *tunnel = READ_ONCE(session->tunnel);
+
+			if (tunnel && tunnel->sock == sk &&
 			    refcount_inc_not_zero(&session->ref_count)) {
 				rcu_read_unlock_bh();
 				return session;
@@ -482,6 +489,7 @@ int l2tp_session_register(struct l2tp_session *session,
 	}
 
 	l2tp_tunnel_inc_refcount(tunnel);
+	WRITE_ONCE(session->tunnel, tunnel);
 	list_add(&session->list, &tunnel->session_list);
 
 	if (tunnel->version == L2TP_HDR_VER_3) {
@@ -797,7 +805,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 		if (!session->lns_mode && !session->send_seq) {
 			trace_session_seqnum_lns_enable(session);
 			session->send_seq = 1;
-			l2tp_session_set_header_len(session, tunnel->version);
+			l2tp_session_set_header_len(session, tunnel->version,
+						    tunnel->encap);
 		}
 	} else {
 		/* No sequence numbers.
@@ -818,7 +827,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 		if (!session->lns_mode && session->send_seq) {
 			trace_session_seqnum_lns_disable(session);
 			session->send_seq = 0;
-			l2tp_session_set_header_len(session, tunnel->version);
+			l2tp_session_set_header_len(session, tunnel->version,
+						    tunnel->encap);
 		} else if (session->send_seq) {
 			pr_debug_ratelimited("%s: recv data has no seq numbers when required. Discarding.\n",
 					     session->name);
@@ -1663,7 +1673,8 @@ EXPORT_SYMBOL_GPL(l2tp_session_delete);
 /* We come here whenever a session's send_seq, cookie_len or
  * l2specific_type parameters are set.
  */
-void l2tp_session_set_header_len(struct l2tp_session *session, int version)
+void l2tp_session_set_header_len(struct l2tp_session *session, int version,
+				 enum l2tp_encap_type encap)
 {
 	if (version == L2TP_HDR_VER_2) {
 		session->hdr_len = 6;
@@ -1672,7 +1683,7 @@ void l2tp_session_set_header_len(struct l2tp_session *session, int version)
 	} else {
 		session->hdr_len = 4 + session->cookie_len;
 		session->hdr_len += l2tp_get_l2specific_len(session);
-		if (session->tunnel->encap == L2TP_ENCAPTYPE_UDP)
+		if (encap == L2TP_ENCAPTYPE_UDP)
 			session->hdr_len += 4;
 	}
 }
@@ -1686,7 +1697,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
 	session = kzalloc(sizeof(*session) + priv_size, GFP_KERNEL);
 	if (session) {
 		session->magic = L2TP_SESSION_MAGIC;
-		session->tunnel = tunnel;
 
 		session->session_id = session_id;
 		session->peer_session_id = peer_session_id;
@@ -1724,7 +1734,7 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
 			memcpy(&session->peer_cookie[0], &cfg->peer_cookie[0], cfg->peer_cookie_len);
 		}
 
-		l2tp_session_set_header_len(session, tunnel->version);
+		l2tp_session_set_header_len(session, tunnel->version, tunnel->encap);
 
 		refcount_set(&session->ref_count, 1);
 
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 8ac81bc1bc6fa..6c25c196cc222 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -260,7 +260,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb);
 
 /* Transmit path helpers for sending packets over the tunnel socket. */
-void l2tp_session_set_header_len(struct l2tp_session *session, int version);
+void l2tp_session_set_header_len(struct l2tp_session *session, int version,
+				 enum l2tp_encap_type encap);
 int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb);
 
 /* Pseudowire management.
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index d105030520f95..fc43ecbd128cc 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -692,8 +692,10 @@ static int l2tp_nl_cmd_session_modify(struct sk_buff *skb, struct genl_info *inf
 		session->recv_seq = nla_get_u8(info->attrs[L2TP_ATTR_RECV_SEQ]);
 
 	if (info->attrs[L2TP_ATTR_SEND_SEQ]) {
+		struct l2tp_tunnel *tunnel = session->tunnel;
+
 		session->send_seq = nla_get_u8(info->attrs[L2TP_ATTR_SEND_SEQ]);
-		l2tp_session_set_header_len(session, session->tunnel->version);
+		l2tp_session_set_header_len(session, tunnel->version, tunnel->encap);
 	}
 
 	if (info->attrs[L2TP_ATTR_LNS_MODE])
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 3596290047b28..4f25c1212cacb 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1205,7 +1205,8 @@ static int pppol2tp_session_setsockopt(struct sock *sk,
 			po->chan.hdrlen = val ? PPPOL2TP_L2TP_HDR_SIZE_SEQ :
 				PPPOL2TP_L2TP_HDR_SIZE_NOSEQ;
 		}
-		l2tp_session_set_header_len(session, session->tunnel->version);
+		l2tp_session_set_header_len(session, session->tunnel->version,
+					    session->tunnel->encap);
 		break;
 
 	case PPPOL2TP_SO_LNSMODE:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2024-10-06  0:28 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-25 11:23 [PATCH AUTOSEL 6.11 001/244] l2tp: prevent possible tunnel refcount underflow Sasha Levin
2024-09-25 11:23 ` [PATCH AUTOSEL 6.11 009/244] ice: Adjust over allocation of memory in ice_sched_add_root_node() and ice_sched_add_node() Sasha Levin
2024-09-25 11:23 ` [PATCH AUTOSEL 6.11 012/244] wifi: cfg80211: Set correct chandef when starting CAC Sasha Levin
2024-09-25 11:23 ` [PATCH AUTOSEL 6.11 013/244] net/xen-netback: prevent UAF in xenvif_flush_hash() Sasha Levin
2024-09-25 11:23 ` [PATCH AUTOSEL 6.11 014/244] net: hisilicon: hip04: fix OF node leak in probe() Sasha Levin
2024-09-25 11:23 ` [PATCH AUTOSEL 6.11 015/244] net: hisilicon: hns_dsaf_mac: fix OF node leak in hns_mac_get_info() Sasha Levin
2024-09-25 11:23 ` [PATCH AUTOSEL 6.11 016/244] net: hisilicon: hns_mdio: fix OF node leak in probe() Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 022/244] e1000e: avoid failing the system during pm_suspend Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 025/244] net: sched: consistently use rcu_replace_pointer() in taprio_change() Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 036/244] l2tp: don't use tunnel socket sk_user_data in ppp procfs output Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 037/244] l2tp: free sessions using rcu Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 038/244] l2tp: use rcu list add/del when updating lists Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 042/244] tipc: guard against string buffer overrun Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 043/244] net: skbuff: sprinkle more __GFP_NOWARN on ingress allocs Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 044/244] net: mvpp2: Increase size of queue_name buffer Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 045/244] bnxt_en: Extend maximum length of version string by 1 byte Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 046/244] ipv4: Check !in_dev earlier for ioctl(SIOCSIFADDR) Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 048/244] netfilter: nf_tables: do not remove elements if set backend implements .abort Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 049/244] netfilter: nf_tables: don't initialize registers in nft_do_chain() Sasha Levin
2024-09-25 11:58   ` Pablo Neira Ayuso
2024-09-25 12:17     ` Pablo Neira Ayuso
2024-09-25 12:20     ` Florian Westphal
2024-10-06  0:28       ` Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 050/244] ipv4: Mask upper DSCP bits and ECN bits in NETLINK_FIB_LOOKUP family Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 054/244] net: atlantic: Avoid warning about potential string truncation Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 056/244] netpoll: Ensure clean state on setup failures Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 057/244] tcp: avoid reusing FIN_WAIT2 when trying to find port in connect() process Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 062/244] wifi: mac80211: fix RCU list iterations Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 068/244] can: netlink: avoid call to do_set_data_bittiming callback with stale can_priv::ctrlmode Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 069/244] netdev-genl: Set extack and fix error on napi-get Sasha Levin
2024-09-25 11:24 ` [PATCH AUTOSEL 6.11 075/244] net: phy: Check for read errors in SIOCGMIIREG Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 079/244] net: napi: Prevent overflow of napi_defer_hard_irqs Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 084/244] net: tls: wait for async completion on last message Sasha Levin
2024-09-25 11:25 ` [PATCH AUTOSEL 6.11 087/244] nfp: Use IRQF_NO_AUTOEN flag in request_irq() Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).