From 2e7a280692bf43940728b7f6ca6a52ebf641a6f5 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:00 +0100 Subject: l2tp: lookup tunnel from socket without using sk_user_data l2tp_sk_to_tunnel derives the tunnel from sk_user_data. Instead, lookup the tunnel by walking the tunnel IDR for a tunnel using the indicated sock. This is slow but l2tp_sk_to_tunnel is not used in the datapath so performance isn't critical. l2tp_tunnel_destruct needs a variant of l2tp_sk_to_tunnel which does not bump the tunnel refcount since the tunnel refcount is already 0. Change l2tp_sk_to_tunnel sk arg to const since it does not modify sk. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 52 +++++++++++++++++++++++++++++++++++++++++++--------- net/l2tp/l2tp_core.h | 5 +---- net/l2tp/l2tp_ip.c | 7 +++++-- net/l2tp/l2tp_ip6.c | 7 +++++-- 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index c80ab3f26084..c97cd0fd8514 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -150,15 +150,43 @@ static void l2tp_session_free(struct l2tp_session *session) kfree(session); } -struct l2tp_tunnel *l2tp_sk_to_tunnel(struct sock *sk) +static struct l2tp_tunnel *__l2tp_sk_to_tunnel(const struct sock *sk) { - struct l2tp_tunnel *tunnel = sk->sk_user_data; + const struct net *net = sock_net(sk); + unsigned long tunnel_id, tmp; + struct l2tp_tunnel *tunnel; + struct l2tp_net *pn; + + WARN_ON_ONCE(!rcu_read_lock_bh_held()); + pn = l2tp_pernet(net); + idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) { + if (tunnel && tunnel->sock == sk) + return tunnel; + } + + return NULL; +} - if (tunnel) - if (WARN_ON(tunnel->magic != L2TP_TUNNEL_MAGIC)) - return NULL; +struct l2tp_tunnel *l2tp_sk_to_tunnel(const struct sock *sk) +{ + const struct net *net = sock_net(sk); + unsigned long tunnel_id, tmp; + struct l2tp_tunnel *tunnel; + struct l2tp_net *pn; + + rcu_read_lock_bh(); + pn = l2tp_pernet(net); + idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) { + if (tunnel && + tunnel->sock == sk && + refcount_inc_not_zero(&tunnel->ref_count)) { + rcu_read_unlock_bh(); + return tunnel; + } + } + rcu_read_unlock_bh(); - return tunnel; + return NULL; } EXPORT_SYMBOL_GPL(l2tp_sk_to_tunnel); @@ -1213,8 +1241,10 @@ EXPORT_SYMBOL_GPL(l2tp_xmit_skb); */ static void l2tp_tunnel_destruct(struct sock *sk) { - struct l2tp_tunnel *tunnel = l2tp_sk_to_tunnel(sk); + struct l2tp_tunnel *tunnel; + rcu_read_lock_bh(); + tunnel = __l2tp_sk_to_tunnel(sk); if (!tunnel) goto end; @@ -1242,6 +1272,7 @@ static void l2tp_tunnel_destruct(struct sock *sk) kfree_rcu(tunnel, rcu); end: + rcu_read_unlock_bh(); return; } @@ -1308,10 +1339,13 @@ static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) /* Tunnel socket destroy hook for UDP encapsulation */ static void l2tp_udp_encap_destroy(struct sock *sk) { - struct l2tp_tunnel *tunnel = l2tp_sk_to_tunnel(sk); + struct l2tp_tunnel *tunnel; - if (tunnel) + tunnel = l2tp_sk_to_tunnel(sk); + if (tunnel) { l2tp_tunnel_delete(tunnel); + l2tp_tunnel_dec_refcount(tunnel); + } } static void l2tp_tunnel_remove(struct net *net, struct l2tp_tunnel *tunnel) diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 8ac81bc1bc6f..a41cf6795df0 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -273,10 +273,7 @@ void l2tp_nl_unregister_ops(enum l2tp_pwtype pw_type); /* IOCTL helper for IP encap modules. */ int l2tp_ioctl(struct sock *sk, int cmd, int *karg); -/* Extract the tunnel structure from a socket's sk_user_data pointer, - * validating the tunnel magic feather. - */ -struct l2tp_tunnel *l2tp_sk_to_tunnel(struct sock *sk); +struct l2tp_tunnel *l2tp_sk_to_tunnel(const struct sock *sk); static inline int l2tp_get_l2specific_len(struct l2tp_session *session) { diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index e48aa177d74c..78243f993cda 100644 --- a/net/l2tp/l2tp_ip.c +++ b/net/l2tp/l2tp_ip.c @@ -235,14 +235,17 @@ static void l2tp_ip_close(struct sock *sk, long timeout) static void l2tp_ip_destroy_sock(struct sock *sk) { - struct l2tp_tunnel *tunnel = l2tp_sk_to_tunnel(sk); + struct l2tp_tunnel *tunnel; struct sk_buff *skb; while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL) kfree_skb(skb); - if (tunnel) + tunnel = l2tp_sk_to_tunnel(sk); + if (tunnel) { l2tp_tunnel_delete(tunnel); + l2tp_tunnel_dec_refcount(tunnel); + } } static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c index d217ff1f229e..3b0465f2d60d 100644 --- a/net/l2tp/l2tp_ip6.c +++ b/net/l2tp/l2tp_ip6.c @@ -246,14 +246,17 @@ static void l2tp_ip6_close(struct sock *sk, long timeout) static void l2tp_ip6_destroy_sock(struct sock *sk) { - struct l2tp_tunnel *tunnel = l2tp_sk_to_tunnel(sk); + struct l2tp_tunnel *tunnel; lock_sock(sk); ip6_flush_pending_frames(sk); release_sock(sk); - if (tunnel) + tunnel = l2tp_sk_to_tunnel(sk); + if (tunnel) { l2tp_tunnel_delete(tunnel); + l2tp_tunnel_dec_refcount(tunnel); + } } static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) -- cgit From 4ff8863419cdc40f2c6e1ad99436e375b9b86b68 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:01 +0100 Subject: ipv4: export ip_flush_pending_frames To avoid protocol modules implementing their own, export ip_flush_pending_frames. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/ipv4/ip_output.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index b90d0f78ac80..8a10a7c67834 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -1534,6 +1534,7 @@ void ip_flush_pending_frames(struct sock *sk) { __ip_flush_pending_frames(sk, &sk->sk_write_queue, &inet_sk(sk)->cork.base); } +EXPORT_SYMBOL_GPL(ip_flush_pending_frames); struct sk_buff *ip_make_skb(struct sock *sk, struct flowi4 *fl4, -- cgit From ed8ebee6def7b7b760bd4fd90c03b9e86622701c Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:02 +0100 Subject: l2tp: have l2tp_ip_destroy_sock use ip_flush_pending_frames Use the recently exported ip_flush_pending_frames instead of a free-coded version and lock the socket while we call it. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_ip.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index 78243f993cda..f21dcbf3efd5 100644 --- a/net/l2tp/l2tp_ip.c +++ b/net/l2tp/l2tp_ip.c @@ -236,10 +236,10 @@ static void l2tp_ip_close(struct sock *sk, long timeout) static void l2tp_ip_destroy_sock(struct sock *sk) { struct l2tp_tunnel *tunnel; - struct sk_buff *skb; - while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL) - kfree_skb(skb); + lock_sock(sk); + ip_flush_pending_frames(sk); + release_sock(sk); tunnel = l2tp_sk_to_tunnel(sk); if (tunnel) { -- cgit From eeb11209e000797d555aefd642e24ed6f4e70140 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:03 +0100 Subject: l2tp: don't use tunnel socket sk_user_data in ppp procfs output l2tp's ppp procfs output can be used to show internal state of pppol2tp. It includes a 'user-data-ok' field, which is derived from the tunnel socket's sk_user_data being non-NULL. Use tunnel->sock being non-NULL to indicate this instead. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_ppp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 3596290047b2..0844b86cd0a6 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -1513,7 +1513,7 @@ static void pppol2tp_seq_tunnel_show(struct seq_file *m, void *v) seq_printf(m, "\nTUNNEL '%s', %c %d\n", tunnel->name, - (tunnel == tunnel->sock->sk_user_data) ? 'Y' : 'N', + tunnel->sock ? 'Y' : 'N', refcount_read(&tunnel->ref_count) - 1); seq_printf(m, " %08x %ld/%ld/%ld %ld/%ld/%ld\n", 0, -- cgit From 4a4cd70369f162f819b7855b0eabcb2db21f01f4 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:04 +0100 Subject: l2tp: don't set sk_user_data in tunnel socket l2tp no longer uses the tunnel socket's sk_user_data so drop the code which sets it. In l2tp_validate_socket use l2tp_sk_to_tunnel to check whether a given socket is already attached to an l2tp tunnel since we can no longer use non-null sk_user_data to indicate this. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index c97cd0fd8514..59a171fa1a39 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1263,7 +1263,6 @@ static void l2tp_tunnel_destruct(struct sock *sk) /* Remove hooks into tunnel socket */ write_lock_bh(&sk->sk_callback_lock); sk->sk_destruct = tunnel->old_sk_destruct; - sk->sk_user_data = NULL; write_unlock_bh(&sk->sk_callback_lock); /* Call the original destructor */ @@ -1554,6 +1553,8 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_create); static int l2tp_validate_socket(const struct sock *sk, const struct net *net, enum l2tp_encap_type encap) { + struct l2tp_tunnel *tunnel; + if (!net_eq(sock_net(sk), net)) return -EINVAL; @@ -1567,8 +1568,11 @@ static int l2tp_validate_socket(const struct sock *sk, const struct net *net, (encap == L2TP_ENCAPTYPE_IP && sk->sk_protocol != IPPROTO_L2TP)) return -EPROTONOSUPPORT; - if (sk->sk_user_data) + tunnel = l2tp_sk_to_tunnel(sk); + if (tunnel) { + l2tp_tunnel_dec_refcount(tunnel); return -EBUSY; + } return 0; } @@ -1607,12 +1611,10 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net, ret = l2tp_validate_socket(sk, net, tunnel->encap); if (ret < 0) goto err_inval_sock; - rcu_assign_sk_user_data(sk, tunnel); write_unlock_bh(&sk->sk_callback_lock); if (tunnel->encap == L2TP_ENCAPTYPE_UDP) { struct udp_tunnel_sock_cfg udp_cfg = { - .sk_user_data = tunnel, .encap_type = UDP_ENCAP_L2TPINUDP, .encap_rcv = l2tp_udp_encap_recv, .encap_err_rcv = l2tp_udp_encap_err_recv, -- cgit From 0fa51a7c6f54f1c0c23bb256eb9b2d31a8b9c544 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:05 +0100 Subject: l2tp: remove unused tunnel magic field Since l2tp no longer derives tunnel pointers directly via sk_user_data, it is no longer useful for l2tp to check tunnel pointers using a magic feather. Drop the tunnel's magic field. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 1 - net/l2tp/l2tp_core.h | 3 --- 2 files changed, 4 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 59a171fa1a39..1ef14f99e78c 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1527,7 +1527,6 @@ int l2tp_tunnel_create(int fd, int version, u32 tunnel_id, u32 peer_tunnel_id, tunnel->tunnel_id = tunnel_id; tunnel->peer_tunnel_id = peer_tunnel_id; - tunnel->magic = L2TP_TUNNEL_MAGIC; sprintf(&tunnel->name[0], "tunl %u", tunnel_id); spin_lock_init(&tunnel->list_lock); tunnel->acpt_newsess = true; diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index a41cf6795df0..50107531fe3b 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -16,7 +16,6 @@ #endif /* Random numbers used for internal consistency checks of tunnel and session structures */ -#define L2TP_TUNNEL_MAGIC 0x42114DDA #define L2TP_SESSION_MAGIC 0x0C04EB7D struct sk_buff; @@ -155,8 +154,6 @@ struct l2tp_tunnel_cfg { */ #define L2TP_TUNNEL_NAME_MAX 20 struct l2tp_tunnel { - int magic; /* Should be L2TP_TUNNEL_MAGIC */ - unsigned long dead; struct rcu_head rcu; -- cgit From 29717a4fb7fcbbcfa5f0d5e8969b8ce0929276be Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:06 +0100 Subject: l2tp: simplify tunnel and socket cleanup When the l2tp tunnel socket used sk_user_data to point to its associated l2tp tunnel, socket and tunnel cleanup had to make use of the socket's destructor to free the tunnel only when the socket could no longer be accessed. Now that sk_user_data is no longer used, we can simplify socket and tunnel cleanup: * If the tunnel closes first, it cleans up and drops its socket ref when the tunnel refcount drops to zero. If its socket was provided by userspace, the socket is closed and freed asynchronously, when userspace closes it. If its socket is a kernel socket, the tunnel closes the socket itself during cleanup and drops its socket ref when the tunnel's refcount drops to zero. * If the socket closes first, we initiate the closing of its associated tunnel. For UDP sockets, this is via the socket's encap_destroy hook. For L2TPIP sockets, this is via the socket's destroy callback. The tunnel holds a socket ref while it references the sock. When the tunnel is freed, it drops its socket ref and the socket will be cleaned up when its own refcount drops to zero, asynchronous to the tunnel free. * The tunnel socket destructor is no longer needed since the tunnel is no longer freed through the socket destructor. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 82 ++++++++++++++-------------------------------------- net/l2tp/l2tp_core.h | 1 - 2 files changed, 21 insertions(+), 62 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 1ef14f99e78c..a01dd891639b 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -137,9 +137,28 @@ static inline struct l2tp_net *l2tp_pernet(const struct net *net) static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel) { + struct sock *sk = tunnel->sock; + trace_free_tunnel(tunnel); - sock_put(tunnel->sock); - /* the tunnel is freed in the socket destructor */ + + if (sk) { + /* Disable udp encapsulation */ + switch (tunnel->encap) { + case L2TP_ENCAPTYPE_UDP: + /* No longer an encapsulation socket. See net/ipv4/udp.c */ + WRITE_ONCE(udp_sk(sk)->encap_type, 0); + udp_sk(sk)->encap_rcv = NULL; + udp_sk(sk)->encap_destroy = NULL; + break; + case L2TP_ENCAPTYPE_IP: + break; + } + + tunnel->sock = NULL; + sock_put(sk); + } + + kfree_rcu(tunnel, rcu); } static void l2tp_session_free(struct l2tp_session *session) @@ -150,23 +169,6 @@ static void l2tp_session_free(struct l2tp_session *session) kfree(session); } -static struct l2tp_tunnel *__l2tp_sk_to_tunnel(const struct sock *sk) -{ - const struct net *net = sock_net(sk); - unsigned long tunnel_id, tmp; - struct l2tp_tunnel *tunnel; - struct l2tp_net *pn; - - WARN_ON_ONCE(!rcu_read_lock_bh_held()); - pn = l2tp_pernet(net); - idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) { - if (tunnel && tunnel->sock == sk) - return tunnel; - } - - return NULL; -} - struct l2tp_tunnel *l2tp_sk_to_tunnel(const struct sock *sk) { const struct net *net = sock_net(sk); @@ -1235,46 +1237,6 @@ EXPORT_SYMBOL_GPL(l2tp_xmit_skb); * Tinnel and session create/destroy. *****************************************************************************/ -/* Tunnel socket destruct hook. - * The tunnel context is deleted only when all session sockets have been - * closed. - */ -static void l2tp_tunnel_destruct(struct sock *sk) -{ - struct l2tp_tunnel *tunnel; - - rcu_read_lock_bh(); - tunnel = __l2tp_sk_to_tunnel(sk); - if (!tunnel) - goto end; - - /* Disable udp encapsulation */ - switch (tunnel->encap) { - case L2TP_ENCAPTYPE_UDP: - /* No longer an encapsulation socket. See net/ipv4/udp.c */ - WRITE_ONCE(udp_sk(sk)->encap_type, 0); - udp_sk(sk)->encap_rcv = NULL; - udp_sk(sk)->encap_destroy = NULL; - break; - case L2TP_ENCAPTYPE_IP: - break; - } - - /* Remove hooks into tunnel socket */ - write_lock_bh(&sk->sk_callback_lock); - sk->sk_destruct = tunnel->old_sk_destruct; - write_unlock_bh(&sk->sk_callback_lock); - - /* Call the original destructor */ - if (sk->sk_destruct) - (*sk->sk_destruct)(sk); - - kfree_rcu(tunnel, rcu); -end: - rcu_read_unlock_bh(); - return; -} - /* Remove an l2tp session from l2tp_core's lists. */ static void l2tp_session_unhash(struct l2tp_session *session) { @@ -1623,8 +1585,6 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net, setup_udp_tunnel_sock(net, sock, &udp_cfg); } - tunnel->old_sk_destruct = sk->sk_destruct; - sk->sk_destruct = &l2tp_tunnel_destruct; sk->sk_allocation = GFP_ATOMIC; release_sock(sk); diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 50107531fe3b..6c62d02a0ae6 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -173,7 +173,6 @@ struct l2tp_tunnel { struct net *l2tp_net; /* the net we belong to */ refcount_t ref_count; - void (*old_sk_destruct)(struct sock *sk); struct sock *sock; /* parent socket */ int fd; /* parent fd, if tunnel socket was created * by userspace -- cgit From fc7ec7f554d7d0a27ba339fcf48df11d14413329 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:07 +0100 Subject: l2tp: delete sessions using work queue When a tunnel is closed, l2tp_tunnel_closeall closes all sessions in the tunnel. Move the work of deleting each session to the work queue so that sessions are deleted using the same codepath whether they are closed by user API request or their parent tunnel is closing. This also avoids the locking dance in l2tp_tunnel_closeall where the tunnel's session list lock was unlocked and relocked in the loop. In l2tp_exit_net, use drain_workqueue instead of flush_workqueue because the processing of tunnel_delete work may queue session_delete work items which must also be processed. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 36 ++++++++++++++++++++---------------- net/l2tp/l2tp_core.h | 1 + 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index a01dd891639b..f6ae18c180bf 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1282,18 +1282,8 @@ static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) spin_lock_bh(&tunnel->list_lock); tunnel->acpt_newsess = false; - 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); + list_for_each_entry(session, &tunnel->session_list, list) l2tp_session_delete(session); - spin_lock_bh(&tunnel->list_lock); - l2tp_session_dec_refcount(session); - } spin_unlock_bh(&tunnel->list_lock); } @@ -1631,18 +1621,31 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_delete); void l2tp_session_delete(struct l2tp_session *session) { - if (test_and_set_bit(0, &session->dead)) - return; + if (!test_and_set_bit(0, &session->dead)) { + trace_delete_session(session); + l2tp_session_inc_refcount(session); + queue_work(l2tp_wq, &session->del_work); + } +} +EXPORT_SYMBOL_GPL(l2tp_session_delete); + +/* Workqueue session deletion function */ +static void l2tp_session_del_work(struct work_struct *work) +{ + struct l2tp_session *session = container_of(work, struct l2tp_session, + del_work); - trace_delete_session(session); l2tp_session_unhash(session); l2tp_session_queue_purge(session); if (session->session_close) (*session->session_close)(session); + /* drop initial ref */ + l2tp_session_dec_refcount(session); + + /* drop workqueue ref */ l2tp_session_dec_refcount(session); } -EXPORT_SYMBOL_GPL(l2tp_session_delete); /* We come here whenever a session's send_seq, cookie_len or * l2specific_type parameters are set. @@ -1694,6 +1697,7 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn INIT_HLIST_NODE(&session->hlist); INIT_LIST_HEAD(&session->clist); INIT_LIST_HEAD(&session->list); + INIT_WORK(&session->del_work, l2tp_session_del_work); if (cfg) { session->pwtype = cfg->pw_type; @@ -1751,7 +1755,7 @@ static __net_exit void l2tp_exit_net(struct net *net) rcu_read_unlock_bh(); if (l2tp_wq) - flush_workqueue(l2tp_wq); + drain_workqueue(l2tp_wq); rcu_barrier(); idr_destroy(&pn->l2tp_v2_session_idr); diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 6c62d02a0ae6..8d7a589ccd2a 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -102,6 +102,7 @@ struct l2tp_session { int reorder_skip; /* set if skip to next nr */ enum l2tp_pwtype pwtype; struct l2tp_stats stats; + struct work_struct del_work; /* Session receive handler for data packets. * Each pseudowire implementation should implement this callback in order to -- cgit From d17e89999574aca143dd4ede43e4382d32d98724 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:08 +0100 Subject: l2tp: free sessions using rcu l2tp sessions may be accessed under an rcu read lock. Have them freed via rcu and remove the now unneeded synchronize_rcu when a session is removed. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 4 +--- net/l2tp/l2tp_core.h | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index f6ae18c180bf..4cf4aa271353 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -166,7 +166,7 @@ static void l2tp_session_free(struct l2tp_session *session) trace_free_session(session); if (session->tunnel) l2tp_tunnel_dec_refcount(session->tunnel); - kfree(session); + kfree_rcu(session, rcu); } struct l2tp_tunnel *l2tp_sk_to_tunnel(const struct sock *sk) @@ -1269,8 +1269,6 @@ static void l2tp_session_unhash(struct l2tp_session *session) spin_unlock_bh(&pn->l2tp_session_idr_lock); spin_unlock_bh(&tunnel->list_lock); - - synchronize_rcu(); } } diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 8d7a589ccd2a..58d3977870de 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -66,6 +66,7 @@ struct l2tp_session_coll_list { struct l2tp_session { int magic; /* should be L2TP_SESSION_MAGIC */ long dead; + struct rcu_head rcu; struct l2tp_tunnel *tunnel; /* back pointer to tunnel context */ u32 session_id; -- cgit From c5cbaef992d6420d8bcebea1b1fcc23302a67c57 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:09 +0100 Subject: l2tp: refactor ppp socket/session relationship Each l2tp ppp session has an associated pppox socket. l2tp_ppp uses the session's pppox socket refcount to manage session lifetimes; the pppox socket holds a ref on the session which is dropped by the socket destructor. This complicates session cleanup. Given l2tp sessions are refcounted, it makes more sense to reverse this relationship such that the session keeps the socket alive, not the other way around. So refactor l2tp_ppp to have the session hold a ref on its socket while it references it. When the session is closed, it drops its socket ref when it detaches from its socket. If the socket is closed first, it initiates the closing of its session, if one is attached. The socket/session can then be freed asynchronously when their refcounts drop to 0. Use the session's session_close callback to detach the pppox socket since this will be done on the work queue together with the rest of the session cleanup via l2tp_session_delete. Also, since l2tp_ppp uses the pppox socket's sk_user_data, use the rcu sk_user_data access helpers when accessing it and set the socket's SOCK_RCU_FREE flag to have pppox sockets freed by rcu. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_ppp.c | 94 ++++++++++++++++++++++------------------------------- 1 file changed, 39 insertions(+), 55 deletions(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 0844b86cd0a6..12a0a7162870 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -119,7 +119,6 @@ struct pppol2tp_session { struct mutex sk_lock; /* Protects .sk */ struct sock __rcu *sk; /* Pointer to the session PPPoX socket */ struct sock *__sk; /* Copy of .sk, for cleanup */ - struct rcu_head rcu; /* For asynchronous release */ }; static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb); @@ -157,20 +156,16 @@ static inline struct l2tp_session *pppol2tp_sock_to_session(struct sock *sk) if (!sk) return NULL; - sock_hold(sk); - session = (struct l2tp_session *)(sk->sk_user_data); - if (!session) { - sock_put(sk); - goto out; - } - if (WARN_ON(session->magic != L2TP_SESSION_MAGIC)) { - session = NULL; - sock_put(sk); - goto out; + rcu_read_lock(); + session = rcu_dereference_sk_user_data(sk); + if (session && refcount_inc_not_zero(&session->ref_count)) { + rcu_read_unlock(); + WARN_ON_ONCE(session->magic != L2TP_SESSION_MAGIC); + return session; } + rcu_read_unlock(); -out: - return session; + return NULL; } /***************************************************************************** @@ -318,12 +313,12 @@ static int pppol2tp_sendmsg(struct socket *sock, struct msghdr *m, l2tp_xmit_skb(session, skb); local_bh_enable(); - sock_put(sk); + l2tp_session_dec_refcount(session); return total_len; error_put_sess: - sock_put(sk); + l2tp_session_dec_refcount(session); error: return error; } @@ -377,12 +372,12 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb) l2tp_xmit_skb(session, skb); local_bh_enable(); - sock_put(sk); + l2tp_session_dec_refcount(session); return 1; abort_put_sess: - sock_put(sk); + l2tp_session_dec_refcount(session); abort: /* Free the original skb */ kfree_skb(skb); @@ -393,28 +388,31 @@ abort: * Session (and tunnel control) socket create/destroy. *****************************************************************************/ -static void pppol2tp_put_sk(struct rcu_head *head) -{ - struct pppol2tp_session *ps; - - ps = container_of(head, typeof(*ps), rcu); - sock_put(ps->__sk); -} - /* Really kill the session socket. (Called from sock_put() if * refcnt == 0.) */ static void pppol2tp_session_destruct(struct sock *sk) { - struct l2tp_session *session = sk->sk_user_data; - skb_queue_purge(&sk->sk_receive_queue); skb_queue_purge(&sk->sk_write_queue); +} - if (session) { - sk->sk_user_data = NULL; - if (WARN_ON(session->magic != L2TP_SESSION_MAGIC)) - return; +static void pppol2tp_session_close(struct l2tp_session *session) +{ + struct pppol2tp_session *ps; + + ps = l2tp_session_priv(session); + mutex_lock(&ps->sk_lock); + ps->__sk = rcu_dereference_protected(ps->sk, + lockdep_is_held(&ps->sk_lock)); + RCU_INIT_POINTER(ps->sk, NULL); + mutex_unlock(&ps->sk_lock); + if (ps->__sk) { + /* detach socket */ + rcu_assign_sk_user_data(ps->__sk, NULL); + sock_put(ps->__sk); + + /* drop ref taken when we referenced socket via sk_user_data */ l2tp_session_dec_refcount(session); } } @@ -444,30 +442,13 @@ static int pppol2tp_release(struct socket *sock) session = pppol2tp_sock_to_session(sk); if (session) { - struct pppol2tp_session *ps; - l2tp_session_delete(session); - - ps = l2tp_session_priv(session); - mutex_lock(&ps->sk_lock); - ps->__sk = rcu_dereference_protected(ps->sk, - lockdep_is_held(&ps->sk_lock)); - RCU_INIT_POINTER(ps->sk, NULL); - mutex_unlock(&ps->sk_lock); - call_rcu(&ps->rcu, pppol2tp_put_sk); - - /* Rely on the sock_put() call at the end of the function for - * dropping the reference held by pppol2tp_sock_to_session(). - * The last reference will be dropped by pppol2tp_put_sk(). - */ + /* drop ref taken by pppol2tp_sock_to_session */ + l2tp_session_dec_refcount(session); } release_sock(sk); - /* This will delete the session context via - * pppol2tp_session_destruct() if the socket's refcnt drops to - * zero. - */ sock_put(sk); return 0; @@ -506,6 +487,7 @@ static int pppol2tp_create(struct net *net, struct socket *sock, int kern) goto out; sock_init_data(sock, sk); + sock_set_flag(sk, SOCK_RCU_FREE); sock->state = SS_UNCONNECTED; sock->ops = &pppol2tp_ops; @@ -542,6 +524,7 @@ static void pppol2tp_session_init(struct l2tp_session *session) struct pppol2tp_session *ps; session->recv_skb = pppol2tp_recv; + session->session_close = pppol2tp_session_close; if (IS_ENABLED(CONFIG_L2TP_DEBUGFS)) session->show = pppol2tp_show; @@ -830,12 +813,13 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, out_no_ppp: /* This is how we get the session context from the socket. */ - sk->sk_user_data = session; + sock_hold(sk); + rcu_assign_sk_user_data(sk, session); rcu_assign_pointer(ps->sk, sk); mutex_unlock(&ps->sk_lock); /* Keep the reference we've grabbed on the session: sk doesn't expect - * the session to disappear. pppol2tp_session_destruct() is responsible + * the session to disappear. pppol2tp_session_close() is responsible * for dropping it. */ drop_refcnt = false; @@ -1002,7 +986,7 @@ static int pppol2tp_getname(struct socket *sock, struct sockaddr *uaddr, error = len; - sock_put(sk); + l2tp_session_dec_refcount(session); end: return error; } @@ -1274,7 +1258,7 @@ static int pppol2tp_setsockopt(struct socket *sock, int level, int optname, err = pppol2tp_session_setsockopt(sk, session, optname, val); } - sock_put(sk); + l2tp_session_dec_refcount(session); end: return err; } @@ -1395,7 +1379,7 @@ static int pppol2tp_getsockopt(struct socket *sock, int level, int optname, err = 0; end_put_sess: - sock_put(sk); + l2tp_session_dec_refcount(session); end: return err; } -- cgit From 24256415d18695b46da06c93135f5b51c548b950 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:10 +0100 Subject: l2tp: prevent possible tunnel refcount underflow 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 Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- 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 4cf4aa271353..cbf117683fab 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -279,7 +279,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; @@ -507,6 +514,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) { @@ -822,7 +830,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. @@ -843,7 +852,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); @@ -1648,7 +1658,8 @@ static void l2tp_session_del_work(struct work_struct *work) /* 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; @@ -1657,7 +1668,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; } } @@ -1671,7 +1682,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; @@ -1710,7 +1720,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 58d3977870de..c907687705b9 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -258,7 +258,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 d105030520f9..fc43ecbd128c 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 12a0a7162870..1b79a36d5756 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -1189,7 +1189,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: -- cgit From 89b768ec2dfefaeba5212de14fc71368e12d06ba Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:11 +0100 Subject: l2tp: use rcu list add/del when updating lists l2tp_v3_session_htable and tunnel->session_list are read by lockless getters using RCU. Use rcu list variants when adding or removing list items. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index cbf117683fab..cd9b157e8b4d 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -419,12 +419,12 @@ static int l2tp_session_collision_add(struct l2tp_net *pn, /* 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); + hash_add_rcu(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); + hash_add_rcu(pn->l2tp_v3_session_htable, &session1->hlist, + session1->hlist_key); refcount_inc(&clist->ref_count); l2tp_session_coll_list_add(clist, session1); @@ -440,7 +440,7 @@ static void l2tp_session_collision_del(struct l2tp_net *pn, lockdep_assert_held(&pn->l2tp_session_idr_lock); - hash_del(&session->hlist); + hash_del_rcu(&session->hlist); if (clist) { /* Remove session from its collision list. If there @@ -515,7 +515,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); + list_add_rcu(&session->list, &tunnel->session_list); if (tunnel->version == L2TP_HDR_VER_3) { if (!other_session) -- cgit From 0aa45570c3241e3bdba1a8b5b30d200c819b5b15 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:12 +0100 Subject: l2tp: add idr consistency check in session_register l2tp_session_register uses an idr_alloc then idr_replace pattern to insert sessions into the session IDR. To catch invalid locking, add a WARN_ON_ONCE if the IDR entry is modified by another thread between alloc and replace steps. Also add comments to make expectations clear. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index cd9b157e8b4d..fd03c17dd20c 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -474,6 +474,7 @@ int l2tp_session_register(struct l2tp_session *session, { struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net); struct l2tp_session *other_session = NULL; + void *old = NULL; u32 session_key; int err; @@ -517,13 +518,19 @@ int l2tp_session_register(struct l2tp_session *session, WRITE_ONCE(session->tunnel, tunnel); list_add_rcu(&session->list, &tunnel->session_list); + /* this makes session available to lockless getters */ if (tunnel->version == L2TP_HDR_VER_3) { if (!other_session) - idr_replace(&pn->l2tp_v3_session_idr, session, session_key); + old = idr_replace(&pn->l2tp_v3_session_idr, session, session_key); } else { - idr_replace(&pn->l2tp_v2_session_idr, session, session_key); + old = idr_replace(&pn->l2tp_v2_session_idr, session, session_key); } + /* old should be NULL, unless something removed or modified + * the IDR entry after our idr_alloc_32 above (which shouldn't + * happen). + */ + WARN_ON_ONCE(old); out: spin_unlock_bh(&pn->l2tp_session_idr_lock); spin_unlock_bh(&tunnel->list_lock); -- cgit From d93b8a63f011f252dcfb101d5b90367bd8f42db3 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:13 +0100 Subject: l2tp: cleanup eth/ppp pseudowire setup code l2tp eth/ppp pseudowire setup/cleanup uses kfree() in some error paths. Drop the refcount instead such that the session object is always freed when the refcount reaches 0. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_eth.c | 2 +- net/l2tp/l2tp_ppp.c | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c index 8ba00ad433c2..cc8a3ce716e9 100644 --- a/net/l2tp/l2tp_eth.c +++ b/net/l2tp/l2tp_eth.c @@ -322,7 +322,7 @@ err_sess_dev: l2tp_session_dec_refcount(session); free_netdev(dev); err_sess: - kfree(session); + l2tp_session_dec_refcount(session); err: return rc; } diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 1b79a36d5756..90bf3a8ccab6 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -770,6 +770,8 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, goto end; } + drop_refcnt = true; + pppol2tp_session_init(session); ps = l2tp_session_priv(session); l2tp_session_inc_refcount(session); @@ -778,10 +780,10 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, error = l2tp_session_register(session, tunnel); if (error < 0) { mutex_unlock(&ps->sk_lock); - kfree(session); + l2tp_session_dec_refcount(session); goto end; } - drop_refcnt = true; + new_session = true; } @@ -875,7 +877,7 @@ static int pppol2tp_session_create(struct net *net, struct l2tp_tunnel *tunnel, return 0; err_sess: - kfree(session); + l2tp_session_dec_refcount(session); err: return error; } -- cgit From 5dfa598b249c5bd8fa620843cf9ece1ad16929d9 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 29 Jul 2024 16:38:14 +0100 Subject: l2tp: use pre_exit pernet hook to avoid rcu_barrier Move the work of closing all tunnels from the pernet exit hook to pre_exit since the core does rcu synchronisation between these steps and we can therefore remove rcu_barrier from l2tp code. Signed-off-by: James Chapman Signed-off-by: Tom Parkin Signed-off-by: David S. Miller --- net/l2tp/l2tp_core.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index fd03c17dd20c..5d2068b6c778 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1756,7 +1756,7 @@ static __net_init int l2tp_init_net(struct net *net) return 0; } -static __net_exit void l2tp_exit_net(struct net *net) +static __net_exit void l2tp_pre_exit_net(struct net *net) { struct l2tp_net *pn = l2tp_pernet(net); struct l2tp_tunnel *tunnel = NULL; @@ -1771,7 +1771,11 @@ static __net_exit void l2tp_exit_net(struct net *net) if (l2tp_wq) drain_workqueue(l2tp_wq); - rcu_barrier(); +} + +static __net_exit void l2tp_exit_net(struct net *net) +{ + struct l2tp_net *pn = l2tp_pernet(net); idr_destroy(&pn->l2tp_v2_session_idr); idr_destroy(&pn->l2tp_v3_session_idr); @@ -1781,6 +1785,7 @@ static __net_exit void l2tp_exit_net(struct net *net) static struct pernet_operations l2tp_net_ops = { .init = l2tp_init_net, .exit = l2tp_exit_net, + .pre_exit = l2tp_pre_exit_net, .id = &l2tp_net_id, .size = sizeof(struct l2tp_net), }; -- cgit