From a744e2d03a91507646ffff8a03a19a2f34a6798a Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 20 Jun 2024 12:22:37 +0100 Subject: l2tp: remove unused list_head member in l2tp_tunnel Remove an unused variable in struct l2tp_tunnel which was left behind by commit c4d48a58f32c5 ("l2tp: convert l2tp_tunnel_list to idr"). Signed-off-by: James Chapman Reviewed-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 2 -- net/l2tp/l2tp_core.h | 1 - 2 files changed, 3 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 88a34db265d8..69f8c9f5cdc7 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1462,8 +1462,6 @@ int l2tp_tunnel_create(int fd, int version, u32 tunnel_id, u32 peer_tunnel_id, /* Init delete workqueue struct */ INIT_WORK(&tunnel->del_work, l2tp_tunnel_del_work); - INIT_LIST_HEAD(&tunnel->list); - err = 0; err: if (tunnelp) diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 91ebf0a3f499..54dfba1eb91c 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -174,7 +174,6 @@ struct l2tp_tunnel { enum l2tp_encap_type encap; struct l2tp_stats stats; - struct list_head list; /* list node on per-namespace list of tunnels */ struct net *l2tp_net; /* the net we belong to */ refcount_t ref_count; -- cgit v1.2.3-58-ga151 From aa5e17e1f5ecb68d3f67a069f7345dbf1a8f274f Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 20 Jun 2024 12:22:38 +0100 Subject: l2tp: store l2tpv3 sessions in per-net IDR L2TPv3 sessions are currently held in one of two fixed-size hash lists: either a per-net hashlist (IP-encap), or a per-tunnel hashlist (UDP-encap), keyed by the L2TPv3 32-bit session_id. In order to lookup L2TPv3 sessions in UDP-encap tunnels efficiently without finding the tunnel first via sk_user_data, UDP sessions are now kept in a per-net session list, keyed by session ID. Convert the existing per-net hashlist to use an IDR for better performance when there are many sessions and have L2TPv3 UDP sessions use the same IDR. Although the L2TPv3 RFC states that the session ID alone identifies the session, our implementation has allowed the same session ID to be used in different L2TP UDP tunnels. To retain support for this, a new per-net session hashtable is used, keyed by the sock and session ID. If on creating a new session, a session already exists with that ID in the IDR, the colliding sessions are added to the new hashtable and the existing IDR entry is flagged. When looking up sessions, the approach is to first check the IDR and if no unflagged match is found, check the new hashtable. The sock is made available to session getters where session ID collisions are to be considered. In this way, the new hashtable is used only for session ID collisions so can be kept small. For managing session removal, we need a list of colliding sessions matching a given ID in order to update or remove the IDR entry of the ID. This is necessary to detect session ID collisions when future sessions are created. The list head is allocated on first collision of a given ID and refcounted. Signed-off-by: James Chapman Reviewed-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 240 +++++++++++++++++++++++++++++++++++++-------------- net/l2tp/l2tp_core.h | 18 ++-- net/l2tp/l2tp_ip.c | 2 +- net/l2tp/l2tp_ip6.c | 2 +- 4 files changed, 188 insertions(+), 74 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 69f8c9f5cdc7..d6bffdb16466 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -107,11 +107,17 @@ struct l2tp_net { /* Lock for write access to l2tp_tunnel_idr */ spinlock_t l2tp_tunnel_idr_lock; struct idr l2tp_tunnel_idr; - struct hlist_head l2tp_session_hlist[L2TP_HASH_SIZE_2]; - /* Lock for write access to l2tp_session_hlist */ - spinlock_t l2tp_session_hlist_lock; + /* Lock for write access to l2tp_v3_session_idr/htable */ + spinlock_t l2tp_session_idr_lock; + struct idr l2tp_v3_session_idr; + struct hlist_head l2tp_v3_session_htable[16]; }; +static inline unsigned long l2tp_v3_session_hashkey(struct sock *sk, u32 session_id) +{ + return ((unsigned long)sk) + session_id; +} + #if IS_ENABLED(CONFIG_IPV6) static bool l2tp_sk_is_v6(struct sock *sk) { @@ -125,17 +131,6 @@ static inline struct l2tp_net *l2tp_pernet(const struct net *net) return net_generic(net, l2tp_net_id); } -/* Session hash global list for L2TPv3. - * The session_id SHOULD be random according to RFC3931, but several - * L2TP implementations use incrementing session_ids. So we do a real - * hash on the session_id, rather than a simple bitmask. - */ -static inline struct hlist_head * -l2tp_session_id_hash_2(struct l2tp_net *pn, u32 session_id) -{ - return &pn->l2tp_session_hlist[hash_32(session_id, L2TP_HASH_BITS_2)]; -} - /* Session hash list. * The session_id SHOULD be random according to RFC2661, but several * L2TP implementations (Cisco and Microsoft) use incrementing @@ -262,26 +257,40 @@ struct l2tp_session *l2tp_tunnel_get_session(struct l2tp_tunnel *tunnel, } EXPORT_SYMBOL_GPL(l2tp_tunnel_get_session); -struct l2tp_session *l2tp_session_get(const struct net *net, u32 session_id) +struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id) { - struct hlist_head *session_list; + const struct l2tp_net *pn = l2tp_pernet(net); struct l2tp_session *session; - session_list = l2tp_session_id_hash_2(l2tp_pernet(net), session_id); - rcu_read_lock_bh(); - hlist_for_each_entry_rcu(session, session_list, global_hlist) - if (session->session_id == session_id) { - l2tp_session_inc_refcount(session); - rcu_read_unlock_bh(); + session = idr_find(&pn->l2tp_v3_session_idr, session_id); + if (session && !hash_hashed(&session->hlist) && + refcount_inc_not_zero(&session->ref_count)) { + rcu_read_unlock_bh(); + return session; + } - return session; + /* If we get here and session is non-NULL, the session_id + * collides with one in another tunnel. If sk is non-NULL, + * find the session matching sk. + */ + if (session && sk) { + unsigned long key = l2tp_v3_session_hashkey(sk, session->session_id); + + hash_for_each_possible_rcu(pn->l2tp_v3_session_htable, session, + hlist, key) { + if (session->tunnel->sock == sk && + refcount_inc_not_zero(&session->ref_count)) { + rcu_read_unlock_bh(); + return session; + } } + } rcu_read_unlock_bh(); return NULL; } -EXPORT_SYMBOL_GPL(l2tp_session_get); +EXPORT_SYMBOL_GPL(l2tp_v3_session_get); struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth) { @@ -313,12 +322,12 @@ struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, const char *ifname) { struct l2tp_net *pn = l2tp_pernet(net); - int hash; + unsigned long session_id, tmp; struct l2tp_session *session; rcu_read_lock_bh(); - for (hash = 0; hash < L2TP_HASH_SIZE_2; hash++) { - hlist_for_each_entry_rcu(session, &pn->l2tp_session_hlist[hash], global_hlist) { + idr_for_each_entry_ul(&pn->l2tp_v3_session_idr, session, tmp, session_id) { + if (session) { if (!strcmp(session->ifname, ifname)) { l2tp_session_inc_refcount(session); rcu_read_unlock_bh(); @@ -334,13 +343,106 @@ struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, } EXPORT_SYMBOL_GPL(l2tp_session_get_by_ifname); +static void l2tp_session_coll_list_add(struct l2tp_session_coll_list *clist, + struct l2tp_session *session) +{ + l2tp_session_inc_refcount(session); + WARN_ON_ONCE(session->coll_list); + session->coll_list = clist; + spin_lock(&clist->lock); + list_add(&session->clist, &clist->list); + spin_unlock(&clist->lock); +} + +static int l2tp_session_collision_add(struct l2tp_net *pn, + struct l2tp_session *session1, + struct l2tp_session *session2) +{ + struct l2tp_session_coll_list *clist; + + lockdep_assert_held(&pn->l2tp_session_idr_lock); + + if (!session2) + return -EEXIST; + + /* If existing session is in IP-encap tunnel, refuse new session */ + if (session2->tunnel->encap == L2TP_ENCAPTYPE_IP) + return -EEXIST; + + clist = session2->coll_list; + if (!clist) { + /* First collision. Allocate list to manage the collided sessions + * and add the existing session to the list. + */ + clist = kmalloc(sizeof(*clist), GFP_ATOMIC); + if (!clist) + return -ENOMEM; + + spin_lock_init(&clist->lock); + INIT_LIST_HEAD(&clist->list); + refcount_set(&clist->ref_count, 1); + l2tp_session_coll_list_add(clist, session2); + } + + /* If existing session isn't already in the session hlist, add it. */ + if (!hash_hashed(&session2->hlist)) + hash_add(pn->l2tp_v3_session_htable, &session2->hlist, + session2->hlist_key); + + /* Add new session to the hlist and collision list */ + hash_add(pn->l2tp_v3_session_htable, &session1->hlist, + session1->hlist_key); + refcount_inc(&clist->ref_count); + l2tp_session_coll_list_add(clist, session1); + + return 0; +} + +static void l2tp_session_collision_del(struct l2tp_net *pn, + struct l2tp_session *session) +{ + struct l2tp_session_coll_list *clist = session->coll_list; + unsigned long session_key = session->session_id; + struct l2tp_session *session2; + + lockdep_assert_held(&pn->l2tp_session_idr_lock); + + hash_del(&session->hlist); + + if (clist) { + /* Remove session from its collision list. If there + * are other sessions with the same ID, replace this + * session's IDR entry with that session, otherwise + * remove the IDR entry. If this is the last session, + * the collision list data is freed. + */ + spin_lock(&clist->lock); + list_del_init(&session->clist); + session2 = list_first_entry_or_null(&clist->list, struct l2tp_session, clist); + if (session2) { + void *old = idr_replace(&pn->l2tp_v3_session_idr, session2, session_key); + + WARN_ON_ONCE(IS_ERR_VALUE(old)); + } else { + void *removed = idr_remove(&pn->l2tp_v3_session_idr, session_key); + + WARN_ON_ONCE(removed != session); + } + session->coll_list = NULL; + spin_unlock(&clist->lock); + if (refcount_dec_and_test(&clist->ref_count)) + kfree(clist); + l2tp_session_dec_refcount(session); + } +} + int l2tp_session_register(struct l2tp_session *session, struct l2tp_tunnel *tunnel) { + struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net); struct l2tp_session *session_walk; - struct hlist_head *g_head; struct hlist_head *head; - struct l2tp_net *pn; + u32 session_key; int err; head = l2tp_session_id_hash(tunnel, session->session_id); @@ -358,39 +460,45 @@ int l2tp_session_register(struct l2tp_session *session, } if (tunnel->version == L2TP_HDR_VER_3) { - pn = l2tp_pernet(tunnel->l2tp_net); - g_head = l2tp_session_id_hash_2(pn, session->session_id); - - spin_lock_bh(&pn->l2tp_session_hlist_lock); - + session_key = session->session_id; + spin_lock_bh(&pn->l2tp_session_idr_lock); + err = idr_alloc_u32(&pn->l2tp_v3_session_idr, NULL, + &session_key, session_key, GFP_ATOMIC); /* IP encap expects session IDs to be globally unique, while - * UDP encap doesn't. + * UDP encap doesn't. This isn't per the RFC, which says that + * sessions are identified only by the session ID, but is to + * support existing userspace which depends on it. */ - hlist_for_each_entry(session_walk, g_head, global_hlist) - if (session_walk->session_id == session->session_id && - (session_walk->tunnel->encap == L2TP_ENCAPTYPE_IP || - tunnel->encap == L2TP_ENCAPTYPE_IP)) { - err = -EEXIST; - goto err_tlock_pnlock; - } + if (err == -ENOSPC && tunnel->encap == L2TP_ENCAPTYPE_UDP) { + struct l2tp_session *session2; - l2tp_tunnel_inc_refcount(tunnel); - hlist_add_head_rcu(&session->global_hlist, g_head); - - spin_unlock_bh(&pn->l2tp_session_hlist_lock); - } else { - l2tp_tunnel_inc_refcount(tunnel); + session2 = idr_find(&pn->l2tp_v3_session_idr, + session_key); + err = l2tp_session_collision_add(pn, session, session2); + } + spin_unlock_bh(&pn->l2tp_session_idr_lock); + if (err == -ENOSPC) + err = -EEXIST; } + if (err) + goto err_tlock; + + l2tp_tunnel_inc_refcount(tunnel); + hlist_add_head_rcu(&session->hlist, head); spin_unlock_bh(&tunnel->hlist_lock); + if (tunnel->version == L2TP_HDR_VER_3) { + spin_lock_bh(&pn->l2tp_session_idr_lock); + idr_replace(&pn->l2tp_v3_session_idr, session, session_key); + spin_unlock_bh(&pn->l2tp_session_idr_lock); + } + trace_register_session(session); return 0; -err_tlock_pnlock: - spin_unlock_bh(&pn->l2tp_session_hlist_lock); err_tlock: spin_unlock_bh(&tunnel->hlist_lock); @@ -1218,13 +1326,19 @@ static void l2tp_session_unhash(struct l2tp_session *session) hlist_del_init_rcu(&session->hlist); spin_unlock_bh(&tunnel->hlist_lock); - /* For L2TPv3 we have a per-net hash: remove from there, too */ - if (tunnel->version != L2TP_HDR_VER_2) { + /* For L2TPv3 we have a per-net IDR: remove from there, too */ + if (tunnel->version == L2TP_HDR_VER_3) { struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net); - - spin_lock_bh(&pn->l2tp_session_hlist_lock); - hlist_del_init_rcu(&session->global_hlist); - spin_unlock_bh(&pn->l2tp_session_hlist_lock); + struct l2tp_session *removed = session; + + spin_lock_bh(&pn->l2tp_session_idr_lock); + if (hash_hashed(&session->hlist)) + l2tp_session_collision_del(pn, session); + else + removed = idr_remove(&pn->l2tp_v3_session_idr, + session->session_id); + WARN_ON_ONCE(removed && removed != session); + spin_unlock_bh(&pn->l2tp_session_idr_lock); } synchronize_rcu(); @@ -1649,8 +1763,9 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn skb_queue_head_init(&session->reorder_q); + session->hlist_key = l2tp_v3_session_hashkey(tunnel->sock, session->session_id); INIT_HLIST_NODE(&session->hlist); - INIT_HLIST_NODE(&session->global_hlist); + INIT_LIST_HEAD(&session->clist); if (cfg) { session->pwtype = cfg->pw_type; @@ -1683,15 +1798,12 @@ EXPORT_SYMBOL_GPL(l2tp_session_create); static __net_init int l2tp_init_net(struct net *net) { struct l2tp_net *pn = net_generic(net, l2tp_net_id); - int hash; idr_init(&pn->l2tp_tunnel_idr); spin_lock_init(&pn->l2tp_tunnel_idr_lock); - for (hash = 0; hash < L2TP_HASH_SIZE_2; hash++) - INIT_HLIST_HEAD(&pn->l2tp_session_hlist[hash]); - - spin_lock_init(&pn->l2tp_session_hlist_lock); + idr_init(&pn->l2tp_v3_session_idr); + spin_lock_init(&pn->l2tp_session_idr_lock); return 0; } @@ -1701,7 +1813,6 @@ static __net_exit void l2tp_exit_net(struct net *net) struct l2tp_net *pn = l2tp_pernet(net); struct l2tp_tunnel *tunnel = NULL; unsigned long tunnel_id, tmp; - int hash; rcu_read_lock_bh(); idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) { @@ -1714,8 +1825,7 @@ static __net_exit void l2tp_exit_net(struct net *net) flush_workqueue(l2tp_wq); rcu_barrier(); - for (hash = 0; hash < L2TP_HASH_SIZE_2; hash++) - WARN_ON_ONCE(!hlist_empty(&pn->l2tp_session_hlist[hash])); + idr_destroy(&pn->l2tp_v3_session_idr); idr_destroy(&pn->l2tp_tunnel_idr); } diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 54dfba1eb91c..bfccc4ca2644 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -23,10 +23,6 @@ #define L2TP_HASH_BITS 4 #define L2TP_HASH_SIZE BIT(L2TP_HASH_BITS) -/* System-wide session hash table size */ -#define L2TP_HASH_BITS_2 8 -#define L2TP_HASH_SIZE_2 BIT(L2TP_HASH_BITS_2) - struct sk_buff; struct l2tp_stats { @@ -61,6 +57,12 @@ struct l2tp_session_cfg { char *ifname; }; +struct l2tp_session_coll_list { + spinlock_t lock; /* for access to list */ + struct list_head list; + refcount_t ref_count; +}; + /* Represents a session (pseudowire) instance. * Tracks runtime state including cookies, dataplane packet sequencing, and IO statistics. * Is linked into a per-tunnel session hashlist; and in the case of an L2TPv3 session into @@ -88,8 +90,11 @@ struct l2tp_session { u32 nr_oos; /* NR of last OOS packet */ int nr_oos_count; /* for OOS recovery */ int nr_oos_count_max; - struct hlist_node hlist; /* hash list node */ refcount_t ref_count; + struct hlist_node hlist; /* per-net session hlist */ + unsigned long hlist_key; /* key for session hlist */ + struct l2tp_session_coll_list *coll_list; /* session collision list */ + struct list_head clist; /* for coll_list */ char name[L2TP_SESSION_NAME_MAX]; /* for logging */ char ifname[IFNAMSIZ]; @@ -102,7 +107,6 @@ struct l2tp_session { int reorder_skip; /* set if skip to next nr */ enum l2tp_pwtype pwtype; struct l2tp_stats stats; - struct hlist_node global_hlist; /* global hash list node */ /* Session receive handler for data packets. * Each pseudowire implementation should implement this callback in order to @@ -226,7 +230,7 @@ struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth); struct l2tp_session *l2tp_tunnel_get_session(struct l2tp_tunnel *tunnel, u32 session_id); -struct l2tp_session *l2tp_session_get(const struct net *net, u32 session_id); +struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id); struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth); struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, const char *ifname); diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index 19c8cc5289d5..e48aa177d74c 100644 --- a/net/l2tp/l2tp_ip.c +++ b/net/l2tp/l2tp_ip.c @@ -140,7 +140,7 @@ static int l2tp_ip_recv(struct sk_buff *skb) } /* Ok, this is a data packet. Lookup the session. */ - session = l2tp_session_get(net, session_id); + session = l2tp_v3_session_get(net, NULL, session_id); if (!session) goto discard; diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c index 8780ec64f376..d217ff1f229e 100644 --- a/net/l2tp/l2tp_ip6.c +++ b/net/l2tp/l2tp_ip6.c @@ -150,7 +150,7 @@ static int l2tp_ip6_recv(struct sk_buff *skb) } /* Ok, this is a data packet. Lookup the session. */ - session = l2tp_session_get(net, session_id); + session = l2tp_v3_session_get(net, NULL, session_id); if (!session) goto discard; -- cgit v1.2.3-58-ga151 From 2a3339f6c9636aa39f2493865e4664df1ef2baed Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 20 Jun 2024 12:22:39 +0100 Subject: l2tp: store l2tpv2 sessions in per-net IDR L2TPv2 sessions are currently kept in a per-tunnel hashlist, keyed by 16-bit session_id. When handling received L2TPv2 packets, we need to first derive the tunnel using the 16-bit tunnel_id or sock, then lookup the session in a per-tunnel hlist using the 16-bit session_id. We want to avoid using sk_user_data in the datapath and double lookups on every packet. So instead, use a per-net IDR to hold L2TPv2 sessions, keyed by a 32-bit value derived from the 16-bit tunnel_id and session_id. This will allow the L2TPv2 UDP receive datapath to lookup a session with a single lookup without deriving the tunnel first. L2TPv2 sessions are held in their own IDR to avoid potential key collisions with L2TPv3 sessions. Signed-off-by: James Chapman Reviewed-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 70 +++++++++++++++++++++++++++++++++++++++++----------- net/l2tp/l2tp_core.h | 1 + 2 files changed, 56 insertions(+), 15 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index d6bffdb16466..6f30b347fd46 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -107,12 +107,18 @@ struct l2tp_net { /* Lock for write access to l2tp_tunnel_idr */ spinlock_t l2tp_tunnel_idr_lock; struct idr l2tp_tunnel_idr; - /* Lock for write access to l2tp_v3_session_idr/htable */ + /* Lock for write access to l2tp_v[23]_session_idr/htable */ spinlock_t l2tp_session_idr_lock; + struct idr l2tp_v2_session_idr; struct idr l2tp_v3_session_idr; struct hlist_head l2tp_v3_session_htable[16]; }; +static inline u32 l2tp_v2_session_key(u16 tunnel_id, u16 session_id) +{ + return ((u32)tunnel_id) << 16 | session_id; +} + static inline unsigned long l2tp_v3_session_hashkey(struct sock *sk, u32 session_id) { return ((unsigned long)sk) + session_id; @@ -292,6 +298,24 @@ struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, } EXPORT_SYMBOL_GPL(l2tp_v3_session_get); +struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id) +{ + u32 session_key = l2tp_v2_session_key(tunnel_id, session_id); + const struct l2tp_net *pn = l2tp_pernet(net); + struct l2tp_session *session; + + rcu_read_lock_bh(); + session = idr_find(&pn->l2tp_v2_session_idr, session_key); + if (session && refcount_inc_not_zero(&session->ref_count)) { + rcu_read_unlock_bh(); + return session; + } + rcu_read_unlock_bh(); + + return NULL; +} +EXPORT_SYMBOL_GPL(l2tp_v2_session_get); + struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth) { int hash; @@ -477,23 +501,32 @@ int l2tp_session_register(struct l2tp_session *session, err = l2tp_session_collision_add(pn, session, session2); } spin_unlock_bh(&pn->l2tp_session_idr_lock); - if (err == -ENOSPC) - err = -EEXIST; + } else { + session_key = l2tp_v2_session_key(tunnel->tunnel_id, + session->session_id); + spin_lock_bh(&pn->l2tp_session_idr_lock); + err = idr_alloc_u32(&pn->l2tp_v2_session_idr, NULL, + &session_key, session_key, GFP_ATOMIC); + spin_unlock_bh(&pn->l2tp_session_idr_lock); } - if (err) + if (err) { + if (err == -ENOSPC) + err = -EEXIST; goto err_tlock; + } l2tp_tunnel_inc_refcount(tunnel); hlist_add_head_rcu(&session->hlist, head); spin_unlock_bh(&tunnel->hlist_lock); - if (tunnel->version == L2TP_HDR_VER_3) { - spin_lock_bh(&pn->l2tp_session_idr_lock); + spin_lock_bh(&pn->l2tp_session_idr_lock); + if (tunnel->version == L2TP_HDR_VER_3) idr_replace(&pn->l2tp_v3_session_idr, session, session_key); - spin_unlock_bh(&pn->l2tp_session_idr_lock); - } + else + idr_replace(&pn->l2tp_v2_session_idr, session, session_key); + spin_unlock_bh(&pn->l2tp_session_idr_lock); trace_register_session(session); @@ -1321,25 +1354,30 @@ static void l2tp_session_unhash(struct l2tp_session *session) /* Remove the session from core hashes */ if (tunnel) { + struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net); + struct l2tp_session *removed = session; + /* Remove from the per-tunnel hash */ spin_lock_bh(&tunnel->hlist_lock); hlist_del_init_rcu(&session->hlist); spin_unlock_bh(&tunnel->hlist_lock); - /* For L2TPv3 we have a per-net IDR: remove from there, too */ + /* Remove from per-net IDR */ + spin_lock_bh(&pn->l2tp_session_idr_lock); if (tunnel->version == L2TP_HDR_VER_3) { - struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net); - struct l2tp_session *removed = session; - - spin_lock_bh(&pn->l2tp_session_idr_lock); if (hash_hashed(&session->hlist)) l2tp_session_collision_del(pn, session); else removed = idr_remove(&pn->l2tp_v3_session_idr, session->session_id); - WARN_ON_ONCE(removed && removed != session); - spin_unlock_bh(&pn->l2tp_session_idr_lock); + } else { + u32 session_key = l2tp_v2_session_key(tunnel->tunnel_id, + session->session_id); + removed = idr_remove(&pn->l2tp_v2_session_idr, + session_key); } + WARN_ON_ONCE(removed && removed != session); + spin_unlock_bh(&pn->l2tp_session_idr_lock); synchronize_rcu(); } @@ -1802,6 +1840,7 @@ static __net_init int l2tp_init_net(struct net *net) idr_init(&pn->l2tp_tunnel_idr); spin_lock_init(&pn->l2tp_tunnel_idr_lock); + idr_init(&pn->l2tp_v2_session_idr); idr_init(&pn->l2tp_v3_session_idr); spin_lock_init(&pn->l2tp_session_idr_lock); @@ -1825,6 +1864,7 @@ static __net_exit void l2tp_exit_net(struct net *net) flush_workqueue(l2tp_wq); rcu_barrier(); + idr_destroy(&pn->l2tp_v2_session_idr); idr_destroy(&pn->l2tp_v3_session_idr); idr_destroy(&pn->l2tp_tunnel_idr); } diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index bfccc4ca2644..d80f15f5b9fc 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -231,6 +231,7 @@ struct l2tp_session *l2tp_tunnel_get_session(struct l2tp_tunnel *tunnel, u32 session_id); struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id); +struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id); struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth); struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, const char *ifname); -- cgit v1.2.3-58-ga151 From ff6a2ac23cb027ff9980d633412db17d5f7a1e7c Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 20 Jun 2024 12:22:40 +0100 Subject: l2tp: refactor udp recv to lookup to not use sk_user_data Modify UDP decap to not use the tunnel pointer which comes from the sock's sk_user_data when parsing the L2TP header. By looking up the destination session using only the packet contents we avoid potential UDP 5-tuple aliasing issues which arise from depending on the socket that received the packet. Drop the useless error messages on short packet or on failing to find a session since the tunnel pointer might point to a different tunnel if multiple sockets use the same 5-tuple. Short packets (those not big enough to contain an L2TP header) are no longer counted in the tunnel's invalid counter because we can't derive the tunnel until we parse the l2tp header to lookup the session. l2tp_udp_encap_recv was a small wrapper around l2tp_udp_recv_core which used sk_user_data to derive a tunnel pointer in an RCU-safe way. But we no longer need the tunnel pointer, so remove that code and combine the two functions. Signed-off-by: James Chapman Reviewed-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 96 ++++++++++++---------------------------------------- 1 file changed, 21 insertions(+), 75 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 6f30b347fd46..2c6378a9f384 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -926,19 +926,14 @@ static void l2tp_session_queue_purge(struct l2tp_session *session) } } -/* Internal UDP receive frame. Do the real work of receiving an L2TP data frame - * here. The skb is not on a list when we get here. - * Returns 0 if the packet was a data packet and was successfully passed on. - * Returns 1 if the packet was not a good data packet and could not be - * forwarded. All such packets are passed up to userspace to deal with. - */ -static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb) +/* UDP encapsulation receive handler. See net/ipv4/udp.c for details. */ +int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) { struct l2tp_session *session = NULL; - struct l2tp_tunnel *orig_tunnel = tunnel; + struct l2tp_tunnel *tunnel = NULL; + struct net *net = sock_net(sk); unsigned char *ptr, *optr; u16 hdrflags; - u32 tunnel_id, session_id; u16 version; int length; @@ -948,11 +943,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb) __skb_pull(skb, sizeof(struct udphdr)); /* Short packet? */ - if (!pskb_may_pull(skb, L2TP_HDR_SIZE_MAX)) { - pr_debug_ratelimited("%s: recv short packet (len=%d)\n", - tunnel->name, skb->len); - goto invalid; - } + if (!pskb_may_pull(skb, L2TP_HDR_SIZE_MAX)) + goto pass; /* Point to L2TP header */ optr = skb->data; @@ -975,6 +967,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb) ptr += 2; if (version == L2TP_HDR_VER_2) { + u16 tunnel_id, session_id; + /* If length is present, skip it */ if (hdrflags & L2TP_HDRFLAG_L) ptr += 2; @@ -982,49 +976,35 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb) /* Extract tunnel and session ID */ tunnel_id = ntohs(*(__be16 *)ptr); ptr += 2; - - if (tunnel_id != tunnel->tunnel_id) { - /* We are receiving trafic for another tunnel, probably - * because we have several tunnels between the same - * IP/port quadruple, look it up. - */ - struct l2tp_tunnel *alt_tunnel; - - alt_tunnel = l2tp_tunnel_get(tunnel->l2tp_net, tunnel_id); - if (!alt_tunnel) - goto pass; - tunnel = alt_tunnel; - } - session_id = ntohs(*(__be16 *)ptr); ptr += 2; + + session = l2tp_v2_session_get(net, tunnel_id, session_id); } else { + u32 session_id; + ptr += 2; /* skip reserved bits */ - tunnel_id = tunnel->tunnel_id; session_id = ntohl(*(__be32 *)ptr); ptr += 4; - } - /* Check protocol version */ - if (version != tunnel->version) { - pr_debug_ratelimited("%s: recv protocol version mismatch: got %d expected %d\n", - tunnel->name, version, tunnel->version); - goto invalid; + session = l2tp_v3_session_get(net, sk, session_id); } - /* Find the session context */ - session = l2tp_tunnel_get_session(tunnel, session_id); if (!session || !session->recv_skb) { if (session) l2tp_session_dec_refcount(session); /* Not found? Pass to userspace to deal with */ - pr_debug_ratelimited("%s: no session found (%u/%u). Passing up.\n", - tunnel->name, tunnel_id, session_id); goto pass; } - if (tunnel->version == L2TP_HDR_VER_3 && + tunnel = session->tunnel; + + /* Check protocol version */ + if (version != tunnel->version) + goto invalid; + + if (version == L2TP_HDR_VER_3 && l2tp_v3_ensure_opt_in_linear(session, skb, &ptr, &optr)) { l2tp_session_dec_refcount(session); goto invalid; @@ -1033,9 +1013,6 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb) l2tp_recv_common(session, skb, ptr, optr, hdrflags, length); l2tp_session_dec_refcount(session); - if (tunnel != orig_tunnel) - l2tp_tunnel_dec_refcount(tunnel); - return 0; invalid: @@ -1045,42 +1022,11 @@ pass: /* Put UDP header back */ __skb_push(skb, sizeof(struct udphdr)); - if (tunnel != orig_tunnel) - l2tp_tunnel_dec_refcount(tunnel); - - return 1; -} - -/* UDP encapsulation receive and error receive handlers. - * See net/ipv4/udp.c for details. - * - * Note that these functions are called from inside an - * RCU-protected region, but without the socket being locked. - * - * Hence we use rcu_dereference_sk_user_data to access the - * tunnel data structure rather the usual l2tp_sk_to_tunnel - * accessor function. - */ -int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) -{ - struct l2tp_tunnel *tunnel; - - tunnel = rcu_dereference_sk_user_data(sk); - if (!tunnel) - goto pass_up; - if (WARN_ON(tunnel->magic != L2TP_TUNNEL_MAGIC)) - goto pass_up; - - if (l2tp_udp_recv_core(tunnel, skb)) - goto pass_up; - - return 0; - -pass_up: return 1; } EXPORT_SYMBOL_GPL(l2tp_udp_encap_recv); +/* UDP encapsulation receive error handler. See net/ipv4/udp.c for details. */ static void l2tp_udp_encap_err_recv(struct sock *sk, struct sk_buff *skb, int err, __be16 port, u32 info, u8 *payload) { -- cgit v1.2.3-58-ga151 From c37e0138ca5f3be6b69c3020470aecb94eb5d773 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 20 Jun 2024 12:22:41 +0100 Subject: l2tp: don't use sk_user_data in l2tp_udp_encap_err_recv If UDP sockets are aliased, sk might be the wrong socket. There's no benefit to using sk_user_data to do some checks on the associated tunnel context. Just report the error anyway, like udp core does. Signed-off-by: James Chapman Reviewed-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 2c6378a9f384..cbc5de1373cd 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1030,12 +1030,6 @@ EXPORT_SYMBOL_GPL(l2tp_udp_encap_recv); static void l2tp_udp_encap_err_recv(struct sock *sk, struct sk_buff *skb, int err, __be16 port, u32 info, u8 *payload) { - struct l2tp_tunnel *tunnel; - - tunnel = rcu_dereference_sk_user_data(sk); - if (!tunnel || tunnel->fd < 0) - return; - sk->sk_err = err; sk_error_report(sk); -- cgit v1.2.3-58-ga151 From 5f77c18ea55601822f9c495135a5b5d4b499d647 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 20 Jun 2024 12:22:42 +0100 Subject: l2tp: use IDR for all session lookups Add generic session getter which uses IDR. Replace all users of l2tp_tunnel_get_session which uses the per-tunnel session list to use the generic getter. Signed-off-by: James Chapman Reviewed-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 10 ++++++++++ net/l2tp/l2tp_core.h | 2 ++ net/l2tp/l2tp_netlink.c | 6 ++++-- net/l2tp/l2tp_ppp.c | 6 ++++-- 4 files changed, 20 insertions(+), 4 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index cbc5de1373cd..0e826a0260fe 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -316,6 +316,16 @@ struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u } EXPORT_SYMBOL_GPL(l2tp_v2_session_get); +struct l2tp_session *l2tp_session_get(const struct net *net, struct sock *sk, int pver, + u32 tunnel_id, u32 session_id) +{ + if (pver == L2TP_HDR_VER_2) + return l2tp_v2_session_get(net, tunnel_id, session_id); + else + return l2tp_v3_session_get(net, sk, session_id); +} +EXPORT_SYMBOL_GPL(l2tp_session_get); + struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth) { int hash; diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index d80f15f5b9fc..0e7c9b0bcc1e 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -232,6 +232,8 @@ struct l2tp_session *l2tp_tunnel_get_session(struct l2tp_tunnel *tunnel, struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id); struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id); +struct l2tp_session *l2tp_session_get(const struct net *net, struct sock *sk, int pver, + u32 tunnel_id, u32 session_id); struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth); struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, const char *ifname); diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c index a901fd14fe3b..d105030520f9 100644 --- a/net/l2tp/l2tp_netlink.c +++ b/net/l2tp/l2tp_netlink.c @@ -61,7 +61,8 @@ static struct l2tp_session *l2tp_nl_session_get(struct genl_info *info) session_id = nla_get_u32(info->attrs[L2TP_ATTR_SESSION_ID]); tunnel = l2tp_tunnel_get(net, tunnel_id); if (tunnel) { - session = l2tp_tunnel_get_session(tunnel, session_id); + session = l2tp_session_get(net, tunnel->sock, tunnel->version, + tunnel_id, session_id); l2tp_tunnel_dec_refcount(tunnel); } } @@ -635,7 +636,8 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf &cfg); if (ret >= 0) { - session = l2tp_tunnel_get_session(tunnel, session_id); + session = l2tp_session_get(net, tunnel->sock, tunnel->version, + tunnel_id, session_id); if (session) { ret = l2tp_session_notify(&l2tp_nl_family, info, session, L2TP_CMD_SESSION_CREATE); diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 6146e4e67bbb..3596290047b2 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -753,7 +753,8 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, if (tunnel->peer_tunnel_id == 0) tunnel->peer_tunnel_id = info.peer_tunnel_id; - session = l2tp_tunnel_get_session(tunnel, info.session_id); + session = l2tp_session_get(sock_net(sk), tunnel->sock, tunnel->version, + info.tunnel_id, info.session_id); if (session) { drop_refcnt = true; @@ -1045,7 +1046,8 @@ static int pppol2tp_tunnel_copy_stats(struct pppol2tp_ioc_stats *stats, /* If session_id is set, search the corresponding session in the * context of this tunnel and record the session's statistics. */ - session = l2tp_tunnel_get_session(tunnel, stats->session_id); + session = l2tp_session_get(tunnel->l2tp_net, tunnel->sock, tunnel->version, + tunnel->tunnel_id, stats->session_id); if (!session) return -EBADR; -- cgit v1.2.3-58-ga151 From 8c6245af4fc5b6d244fb0f953d493e848d1e1387 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 20 Jun 2024 12:22:43 +0100 Subject: l2tp: drop the now unused l2tp_tunnel_get_session All users of l2tp_tunnel_get_session are now gone so it can be removed. Signed-off-by: James Chapman Reviewed-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 22 ---------------------- net/l2tp/l2tp_core.h | 2 -- 2 files changed, 24 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 0e826a0260fe..3ce689331542 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -241,28 +241,6 @@ struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth) } EXPORT_SYMBOL_GPL(l2tp_tunnel_get_nth); -struct l2tp_session *l2tp_tunnel_get_session(struct l2tp_tunnel *tunnel, - u32 session_id) -{ - struct hlist_head *session_list; - struct l2tp_session *session; - - session_list = l2tp_session_id_hash(tunnel, session_id); - - rcu_read_lock_bh(); - hlist_for_each_entry_rcu(session, session_list, hlist) - if (session->session_id == session_id) { - l2tp_session_inc_refcount(session); - rcu_read_unlock_bh(); - - return session; - } - rcu_read_unlock_bh(); - - return NULL; -} -EXPORT_SYMBOL_GPL(l2tp_tunnel_get_session); - struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id) { const struct l2tp_net *pn = l2tp_pernet(net); diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 0e7c9b0bcc1e..bfff69f2e0a2 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -227,8 +227,6 @@ void l2tp_session_dec_refcount(struct l2tp_session *session); */ struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id); struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth); -struct l2tp_session *l2tp_tunnel_get_session(struct l2tp_tunnel *tunnel, - u32 session_id); struct l2tp_session *l2tp_v3_session_get(const struct net *net, struct sock *sk, u32 session_id); struct l2tp_session *l2tp_v2_session_get(const struct net *net, u16 tunnel_id, u16 session_id); -- cgit v1.2.3-58-ga151 From d18d3f0a24fc4a3513495892ab1a3753628b341b Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 20 Jun 2024 12:22:44 +0100 Subject: l2tp: replace hlist with simple list for per-tunnel session list The per-tunnel session list is no longer used by the datapath. However, we still need a list of sessions in the tunnel for l2tp_session_get_nth, which is used by management code. (An alternative might be to walk each session IDR list, matching only sessions of a given tunnel.) Replace the per-tunnel hlist with a per-tunnel list. In functions which walk a list of sessions of a tunnel, walk this list instead. Signed-off-by: James Chapman Reviewed-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 109 +++++++++++++++++------------------------------- net/l2tp/l2tp_core.h | 19 ++++----- net/l2tp/l2tp_debugfs.c | 13 +++--- 3 files changed, 50 insertions(+), 91 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 3ce689331542..be4bcbf291a1 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -39,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -137,18 +136,6 @@ static inline struct l2tp_net *l2tp_pernet(const struct net *net) return net_generic(net, l2tp_net_id); } -/* Session hash list. - * The session_id SHOULD be random according to RFC2661, but several - * L2TP implementations (Cisco and Microsoft) use incrementing - * session_ids. So we do a real hash on the session_id, rather than a - * simple bitmask. - */ -static inline struct hlist_head * -l2tp_session_id_hash(struct l2tp_tunnel *tunnel, u32 session_id) -{ - return &tunnel->session_hlist[hash_32(session_id, L2TP_HASH_BITS)]; -} - static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel) { trace_free_tunnel(tunnel); @@ -306,21 +293,17 @@ EXPORT_SYMBOL_GPL(l2tp_session_get); struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth) { - int hash; struct l2tp_session *session; int count = 0; rcu_read_lock_bh(); - for (hash = 0; hash < L2TP_HASH_SIZE; hash++) { - hlist_for_each_entry_rcu(session, &tunnel->session_hlist[hash], hlist) { - if (++count > nth) { - l2tp_session_inc_refcount(session); - rcu_read_unlock_bh(); - return session; - } + list_for_each_entry_rcu(session, &tunnel->session_list, list) { + if (++count > nth) { + l2tp_session_inc_refcount(session); + rcu_read_unlock_bh(); + return session; } } - rcu_read_unlock_bh(); return NULL; @@ -334,21 +317,23 @@ struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, const char *ifname) { struct l2tp_net *pn = l2tp_pernet(net); - unsigned long session_id, tmp; + unsigned long tunnel_id, tmp; struct l2tp_session *session; + struct l2tp_tunnel *tunnel; rcu_read_lock_bh(); - idr_for_each_entry_ul(&pn->l2tp_v3_session_idr, session, tmp, session_id) { - if (session) { - if (!strcmp(session->ifname, ifname)) { - l2tp_session_inc_refcount(session); - rcu_read_unlock_bh(); - - return session; + idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) { + if (tunnel) { + list_for_each_entry_rcu(session, &tunnel->session_list, list) { + if (!strcmp(session->ifname, ifname)) { + l2tp_session_inc_refcount(session); + rcu_read_unlock_bh(); + + return session; + } } } } - rcu_read_unlock_bh(); return NULL; @@ -452,25 +437,15 @@ int l2tp_session_register(struct l2tp_session *session, struct l2tp_tunnel *tunnel) { struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net); - struct l2tp_session *session_walk; - struct hlist_head *head; u32 session_key; int err; - head = l2tp_session_id_hash(tunnel, session->session_id); - - spin_lock_bh(&tunnel->hlist_lock); + spin_lock_bh(&tunnel->list_lock); if (!tunnel->acpt_newsess) { err = -ENODEV; goto err_tlock; } - hlist_for_each_entry(session_walk, head, hlist) - if (session_walk->session_id == session->session_id) { - err = -EEXIST; - goto err_tlock; - } - if (tunnel->version == L2TP_HDR_VER_3) { session_key = session->session_id; spin_lock_bh(&pn->l2tp_session_idr_lock); @@ -506,8 +481,8 @@ int l2tp_session_register(struct l2tp_session *session, l2tp_tunnel_inc_refcount(tunnel); - hlist_add_head_rcu(&session->hlist, head); - spin_unlock_bh(&tunnel->hlist_lock); + list_add(&session->list, &tunnel->session_list); + spin_unlock_bh(&tunnel->list_lock); spin_lock_bh(&pn->l2tp_session_idr_lock); if (tunnel->version == L2TP_HDR_VER_3) @@ -521,7 +496,7 @@ int l2tp_session_register(struct l2tp_session *session, return 0; err_tlock: - spin_unlock_bh(&tunnel->hlist_lock); + spin_unlock_bh(&tunnel->list_lock); return err; } @@ -1275,20 +1250,19 @@ end: return; } -/* Remove an l2tp session from l2tp_core's hash lists. */ +/* Remove an l2tp session from l2tp_core's lists. */ static void l2tp_session_unhash(struct l2tp_session *session) { struct l2tp_tunnel *tunnel = session->tunnel; - /* Remove the session from core hashes */ if (tunnel) { struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net); struct l2tp_session *removed = session; - /* Remove from the per-tunnel hash */ - spin_lock_bh(&tunnel->hlist_lock); - hlist_del_init_rcu(&session->hlist); - spin_unlock_bh(&tunnel->hlist_lock); + /* Remove from the per-tunnel list */ + spin_lock_bh(&tunnel->list_lock); + list_del_init(&session->list); + spin_unlock_bh(&tunnel->list_lock); /* Remove from per-net IDR */ spin_lock_bh(&pn->l2tp_session_idr_lock); @@ -1316,28 +1290,19 @@ static void l2tp_session_unhash(struct l2tp_session *session) static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) { struct l2tp_session *session; - int hash; + struct list_head __rcu *pos; + struct list_head *tmp; - spin_lock_bh(&tunnel->hlist_lock); + spin_lock_bh(&tunnel->list_lock); tunnel->acpt_newsess = false; - for (hash = 0; hash < L2TP_HASH_SIZE; hash++) { -again: - hlist_for_each_entry_rcu(session, &tunnel->session_hlist[hash], hlist) { - hlist_del_init_rcu(&session->hlist); - - spin_unlock_bh(&tunnel->hlist_lock); - l2tp_session_delete(session); - spin_lock_bh(&tunnel->hlist_lock); - - /* Now restart from the beginning of this hash - * chain. We always remove a session from the - * list so we are guaranteed to make forward - * progress. - */ - goto again; - } + list_for_each_safe(pos, tmp, &tunnel->session_list) { + session = list_entry(pos, struct l2tp_session, list); + list_del_init(&session->list); + spin_unlock_bh(&tunnel->list_lock); + l2tp_session_delete(session); + spin_lock_bh(&tunnel->list_lock); } - spin_unlock_bh(&tunnel->hlist_lock); + spin_unlock_bh(&tunnel->list_lock); } /* Tunnel socket destroy hook for UDP encapsulation */ @@ -1531,8 +1496,9 @@ int l2tp_tunnel_create(int fd, int version, u32 tunnel_id, u32 peer_tunnel_id, tunnel->magic = L2TP_TUNNEL_MAGIC; sprintf(&tunnel->name[0], "tunl %u", tunnel_id); - spin_lock_init(&tunnel->hlist_lock); + spin_lock_init(&tunnel->list_lock); tunnel->acpt_newsess = true; + INIT_LIST_HEAD(&tunnel->session_list); tunnel->encap = encap; @@ -1732,6 +1698,7 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn session->hlist_key = l2tp_v3_session_hashkey(tunnel->sock, session->session_id); INIT_HLIST_NODE(&session->hlist); INIT_LIST_HEAD(&session->clist); + INIT_LIST_HEAD(&session->list); if (cfg) { session->pwtype = cfg->pw_type; diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index bfff69f2e0a2..8ac81bc1bc6f 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -19,10 +19,6 @@ #define L2TP_TUNNEL_MAGIC 0x42114DDA #define L2TP_SESSION_MAGIC 0x0C04EB7D -/* Per tunnel session hash table size */ -#define L2TP_HASH_BITS 4 -#define L2TP_HASH_SIZE BIT(L2TP_HASH_BITS) - struct sk_buff; struct l2tp_stats { @@ -65,8 +61,7 @@ struct l2tp_session_coll_list { /* Represents a session (pseudowire) instance. * Tracks runtime state including cookies, dataplane packet sequencing, and IO statistics. - * Is linked into a per-tunnel session hashlist; and in the case of an L2TPv3 session into - * an additional per-net ("global") hashlist. + * Is linked into a per-tunnel session list and a per-net ("global") IDR tree. */ #define L2TP_SESSION_NAME_MAX 32 struct l2tp_session { @@ -90,6 +85,7 @@ struct l2tp_session { u32 nr_oos; /* NR of last OOS packet */ int nr_oos_count; /* for OOS recovery */ int nr_oos_count_max; + struct list_head list; /* per-tunnel list node */ refcount_t ref_count; struct hlist_node hlist; /* per-net session hlist */ unsigned long hlist_key; /* key for session hlist */ @@ -118,7 +114,7 @@ struct l2tp_session { /* Session close handler. * Each pseudowire implementation may implement this callback in order to carry * out pseudowire-specific shutdown actions. - * The callback is called by core after unhashing the session and purging its + * The callback is called by core after unlisting the session and purging its * reorder queue. */ void (*session_close)(struct l2tp_session *session); @@ -154,7 +150,7 @@ struct l2tp_tunnel_cfg { /* Represents a tunnel instance. * Tracks runtime state including IO statistics. * Holds the tunnel socket (either passed from userspace or directly created by the kernel). - * Maintains a hashlist of sessions belonging to the tunnel instance. + * Maintains a list of sessions belonging to the tunnel instance. * Is linked into a per-net list of tunnels. */ #define L2TP_TUNNEL_NAME_MAX 20 @@ -164,12 +160,11 @@ struct l2tp_tunnel { unsigned long dead; struct rcu_head rcu; - spinlock_t hlist_lock; /* write-protection for session_hlist */ + spinlock_t list_lock; /* write-protection for session_list */ bool acpt_newsess; /* indicates whether this tunnel accepts - * new sessions. Protected by hlist_lock. + * new sessions. Protected by list_lock. */ - struct hlist_head session_hlist[L2TP_HASH_SIZE]; - /* hashed list of sessions, hashed by id */ + struct list_head session_list; /* list of sessions */ u32 tunnel_id; u32 peer_tunnel_id; int version; /* 2=>L2TPv2, 3=>L2TPv3 */ diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c index 4595b56d175d..8755ae521154 100644 --- a/net/l2tp/l2tp_debugfs.c +++ b/net/l2tp/l2tp_debugfs.c @@ -123,17 +123,14 @@ static void l2tp_dfs_seq_tunnel_show(struct seq_file *m, void *v) struct l2tp_tunnel *tunnel = v; struct l2tp_session *session; int session_count = 0; - int hash; rcu_read_lock_bh(); - for (hash = 0; hash < L2TP_HASH_SIZE; hash++) { - hlist_for_each_entry_rcu(session, &tunnel->session_hlist[hash], hlist) { - /* Session ID of zero is a dummy/reserved value used by pppol2tp */ - if (session->session_id == 0) - continue; + list_for_each_entry_rcu(session, &tunnel->session_list, list) { + /* Session ID of zero is a dummy/reserved value used by pppol2tp */ + if (session->session_id == 0) + continue; - session_count++; - } + session_count++; } rcu_read_unlock_bh(); -- cgit v1.2.3-58-ga151 From a8a8d89dbd2bd2b762b6d0226a1201ec33f7aeac Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 24 Jun 2024 09:29:45 +0100 Subject: l2tp: remove incorrect __rcu attribute This fixes a sparse warning. Fixes: d18d3f0a24fc ("l2tp: replace hlist with simple list for per-tunnel session list") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202406220754.evK8Hrjw-lkp@intel.com/ Signed-off-by: James Chapman Link: https://patch.msgid.link/20240624082945.1925009-1-jchapman@katalix.com Signed-off-by: Jakub Kicinski --- net/l2tp/l2tp_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index be4bcbf291a1..64f446f0930b 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1290,7 +1290,7 @@ static void l2tp_session_unhash(struct l2tp_session *session) static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) { struct l2tp_session *session; - struct list_head __rcu *pos; + struct list_head *pos; struct list_head *tmp; spin_lock_bh(&tunnel->list_lock); -- cgit v1.2.3-58-ga151 From 47c130130de2fd68d9e4f591b0ea25975bdad68a Mon Sep 17 00:00:00 2001 From: Thorsten Blum Date: Wed, 3 Jul 2024 08:11:48 +0200 Subject: l2tp: Remove duplicate included header file trace.h Remove duplicate included header file trace.h and the following warning reported by make includecheck: trace.h is included more than once Compile-tested only. Signed-off-by: Thorsten Blum Reviewed-by: Michal Kubiak Link: https://patch.msgid.link/20240703061147.691973-2-thorsten.blum@toblux.com Signed-off-by: Paolo Abeni --- net/l2tp/l2tp_core.c | 1 - 1 file changed, 1 deletion(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 64f446f0930b..a99032076e04 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -60,7 +60,6 @@ #include #include "l2tp_core.h" -#include "trace.h" #define CREATE_TRACE_POINTS #include "trace.h" -- cgit v1.2.3-58-ga151 From f8ad00f3fb2af98f29aacd7ceb4ecdd5ad3c9a7f Mon Sep 17 00:00:00 2001 From: James Chapman Date: Thu, 4 Jul 2024 16:25:08 +0100 Subject: l2tp: fix possible UAF when cleaning up tunnels syzbot reported a UAF caused by a race when the L2TP work queue closes a tunnel at the same time as a userspace thread closes a session in that tunnel. Tunnel cleanup is handled by a work queue which iterates through the sessions contained within a tunnel, and closes them in turn. Meanwhile, a userspace thread may arbitrarily close a session via either netlink command or by closing the pppox socket in the case of l2tp_ppp. The race condition may occur when l2tp_tunnel_closeall walks the list of sessions in the tunnel and deletes each one. Currently this is implemented using list_for_each_safe, but because the list spinlock is dropped in the loop body it's possible for other threads to manipulate the list during list_for_each_safe's list walk. This can lead to the list iterator being corrupted, leading to list_for_each_safe spinning. One sequence of events which may lead to this is as follows: * A tunnel is created, containing two sessions A and B. * A thread closes the tunnel, triggering tunnel cleanup via the work queue. * l2tp_tunnel_closeall runs in the context of the work queue. It removes session A from the tunnel session list, then drops the list lock. At this point the list_for_each_safe temporary variable is pointing to the other session on the list, which is session B, and the list can be manipulated by other threads since the list lock has been released. * Userspace closes session B, which removes the session from its parent tunnel via l2tp_session_delete. Since l2tp_tunnel_closeall has released the tunnel list lock, l2tp_session_delete is able to call list_del_init on the session B list node. * Back on the work queue, l2tp_tunnel_closeall resumes execution and will now spin forever on the same list entry until the underlying session structure is freed, at which point UAF occurs. The solution is to iterate over the tunnel's session list using list_first_entry_not_null to avoid the possibility of the list iterator pointing at a list item which may be removed during the walk. Also, have l2tp_tunnel_closeall ref each session while it processes it to prevent another thread from freeing it. cpu1 cpu2 --- --- pppol2tp_release() spin_lock_bh(&tunnel->list_lock); for (;;) { session = list_first_entry_or_null(&tunnel->session_list, struct l2tp_session, list); if (!session) break; list_del_init(&session->list); spin_unlock_bh(&tunnel->list_lock); l2tp_session_delete(session); l2tp_session_delete(session); spin_lock_bh(&tunnel->list_lock); } spin_unlock_bh(&tunnel->list_lock); Calling l2tp_session_delete on the same session twice isn't a problem per-se, but if cpu2 manages to destruct the socket and unref the session to zero before cpu1 progresses then it would lead to UAF. Reported-by: syzbot+b471b7c936301a59745b@syzkaller.appspotmail.com Reported-by: syzbot+c041b4ce3a6dfd1e63e2@syzkaller.appspotmail.com Fixes: d18d3f0a24fc ("l2tp: replace hlist with simple list for per-tunnel session list") Signed-off-by: James Chapman Signed-off-by: Tom Parkin Link: https://patch.msgid.link/20240704152508.1923908-1-jchapman@katalix.com Signed-off-by: Paolo Abeni --- net/l2tp/l2tp_core.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index a99032076e04..29dfbd70c79c 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1289,17 +1289,20 @@ static void l2tp_session_unhash(struct l2tp_session *session) static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) { struct l2tp_session *session; - struct list_head *pos; - struct list_head *tmp; spin_lock_bh(&tunnel->list_lock); tunnel->acpt_newsess = false; - list_for_each_safe(pos, tmp, &tunnel->session_list) { - session = list_entry(pos, struct l2tp_session, list); + for (;;) { + session = list_first_entry_or_null(&tunnel->session_list, + struct l2tp_session, list); + if (!session) + break; + l2tp_session_inc_refcount(session); list_del_init(&session->list); spin_unlock_bh(&tunnel->list_lock); l2tp_session_delete(session); spin_lock_bh(&tunnel->list_lock); + l2tp_session_dec_refcount(session); } spin_unlock_bh(&tunnel->list_lock); } -- cgit v1.2.3-58-ga151 From 2146b7dd354c2a1384381ca3cd5751bfff6137d6 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Tue, 9 Jul 2024 17:28:39 +0100 Subject: l2tp: fix l2tp_session_register with colliding l2tpv3 IDs When handling colliding L2TPv3 session IDs, we use the existing session IDR entry and link the new session on that using session->coll_list. However, when using an existing IDR entry, we must not do the idr_replace step. Fixes: aa5e17e1f5ec ("l2tp: store l2tpv3 sessions in per-net IDR") Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'net/l2tp') diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 29dfbd70c79c..1c1decce7f06 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -436,6 +436,7 @@ int l2tp_session_register(struct l2tp_session *session, struct l2tp_tunnel *tunnel) { struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net); + struct l2tp_session *other_session = NULL; u32 session_key; int err; @@ -456,11 +457,10 @@ int l2tp_session_register(struct l2tp_session *session, * support existing userspace which depends on it. */ if (err == -ENOSPC && tunnel->encap == L2TP_ENCAPTYPE_UDP) { - struct l2tp_session *session2; - - session2 = idr_find(&pn->l2tp_v3_session_idr, - session_key); - err = l2tp_session_collision_add(pn, session, session2); + other_session = idr_find(&pn->l2tp_v3_session_idr, + session_key); + err = l2tp_session_collision_add(pn, session, + other_session); } spin_unlock_bh(&pn->l2tp_session_idr_lock); } else { @@ -484,10 +484,12 @@ int l2tp_session_register(struct l2tp_session *session, spin_unlock_bh(&tunnel->list_lock); spin_lock_bh(&pn->l2tp_session_idr_lock); - if (tunnel->version == L2TP_HDR_VER_3) - idr_replace(&pn->l2tp_v3_session_idr, session, session_key); - else + if (tunnel->version == L2TP_HDR_VER_3) { + if (!other_session) + idr_replace(&pn->l2tp_v3_session_idr, session, session_key); + } else { idr_replace(&pn->l2tp_v2_session_idr, session, session_key); + } spin_unlock_bh(&pn->l2tp_session_idr_lock); trace_register_session(session); -- cgit v1.2.3-58-ga151