Skip to content

Commit eaed1df

Browse files
qsnjfvogel
authored andcommitted
tcp: drop secpath at the same time as we currently drop dst
[ Upstream commit 9b6412e ] Xiumei reported hitting the WARN in xfrm6_tunnel_net_exit while running tests that boil down to: - create a pair of netns - run a basic TCP test over ipcomp6 - delete the pair of netns The xfrm_state found on spi_byaddr was not deleted at the time we delete the netns, because we still have a reference on it. This lingering reference comes from a secpath (which holds a ref on the xfrm_state), which is still attached to an skb. This skb is not leaked, it ends up on sk_receive_queue and then gets defer-free'd by skb_attempt_defer_free. The problem happens when we defer freeing an skb (push it on one CPU's defer_list), and don't flush that list before the netns is deleted. In that case, we still have a reference on the xfrm_state that we don't expect at this point. We already drop the skb's dst in the TCP receive path when it's no longer needed, so let's also drop the secpath. At this point, tcp_filter has already called into the LSM hooks that may require the secpath, so it should not be needed anymore. However, in some of those places, the MPTCP extension has just been attached to the skb, so we cannot simply drop all extensions. Fixes: 68822bd ("net: generalize skb freeing deferral to per-cpu lists") Reported-by: Xiumei Mu <[email protected]> Signed-off-by: Sabrina Dubroca <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> Link: https://patch.msgid.link/5055ba8f8f72bdcb602faa299faca73c280b7735.1739743613.git.sd@queasysnail.net Signed-off-by: Paolo Abeni <[email protected]> Signed-off-by: Sasha Levin <[email protected]> (cherry picked from commit cd34a07f744451e2ecf9005bb7d24d0b2fb83656) Signed-off-by: Jack Vogel <[email protected]>
1 parent a231df2 commit eaed1df

File tree

4 files changed

+21
-7
lines changed

4 files changed

+21
-7
lines changed

include/net/tcp.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include <net/inet_ecn.h>
4242
#include <net/dst.h>
4343
#include <net/mptcp.h>
44+
#include <net/xfrm.h>
4445

4546
#include <linux/seq_file.h>
4647
#include <linux/memcontrol.h>
@@ -683,6 +684,19 @@ void tcp_fin(struct sock *sk);
683684
void tcp_check_space(struct sock *sk);
684685
void tcp_sack_compress_send_ack(struct sock *sk);
685686

687+
static inline void tcp_cleanup_skb(struct sk_buff *skb)
688+
{
689+
skb_dst_drop(skb);
690+
secpath_reset(skb);
691+
}
692+
693+
static inline void tcp_add_receive_queue(struct sock *sk, struct sk_buff *skb)
694+
{
695+
DEBUG_NET_WARN_ON_ONCE(skb_dst(skb));
696+
DEBUG_NET_WARN_ON_ONCE(secpath_exists(skb));
697+
__skb_queue_tail(&sk->sk_receive_queue, skb);
698+
}
699+
686700
/* tcp_timer.c */
687701
void tcp_init_xmit_timers(struct sock *);
688702
static inline void tcp_clear_xmit_timers(struct sock *sk)

net/ipv4/tcp_fastopen.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
178178
if (!skb)
179179
return;
180180

181-
skb_dst_drop(skb);
181+
tcp_cleanup_skb(skb);
182182
/* segs_in has been initialized to 1 in tcp_create_openreq_child().
183183
* Hence, reset segs_in to 0 before calling tcp_segs_in()
184184
* to avoid double counting. Also, tcp_segs_in() expects
@@ -195,7 +195,7 @@ void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
195195
TCP_SKB_CB(skb)->tcp_flags &= ~TCPHDR_SYN;
196196

197197
tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
198-
__skb_queue_tail(&sk->sk_receive_queue, skb);
198+
tcp_add_receive_queue(sk, skb);
199199
tp->syn_data_acked = 1;
200200

201201
/* u64_stats_update_begin(&tp->syncp) not needed here,

net/ipv4/tcp_input.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4970,7 +4970,7 @@ static void tcp_ofo_queue(struct sock *sk)
49704970
tcp_rcv_nxt_update(tp, TCP_SKB_CB(skb)->end_seq);
49714971
fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
49724972
if (!eaten)
4973-
__skb_queue_tail(&sk->sk_receive_queue, skb);
4973+
tcp_add_receive_queue(sk, skb);
49744974
else
49754975
kfree_skb_partial(skb, fragstolen);
49764976

@@ -5162,7 +5162,7 @@ static int __must_check tcp_queue_rcv(struct sock *sk, struct sk_buff *skb,
51625162
skb, fragstolen)) ? 1 : 0;
51635163
tcp_rcv_nxt_update(tcp_sk(sk), TCP_SKB_CB(skb)->end_seq);
51645164
if (!eaten) {
5165-
__skb_queue_tail(&sk->sk_receive_queue, skb);
5165+
tcp_add_receive_queue(sk, skb);
51665166
skb_set_owner_r(skb, sk);
51675167
}
51685168
return eaten;
@@ -5245,7 +5245,7 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
52455245
__kfree_skb(skb);
52465246
return;
52475247
}
5248-
skb_dst_drop(skb);
5248+
tcp_cleanup_skb(skb);
52495249
__skb_pull(skb, tcp_hdr(skb)->doff * 4);
52505250

52515251
reason = SKB_DROP_REASON_NOT_SPECIFIED;
@@ -6214,7 +6214,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
62146214
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPHPHITS);
62156215

62166216
/* Bulk data transfer: receiver */
6217-
skb_dst_drop(skb);
6217+
tcp_cleanup_skb(skb);
62186218
__skb_pull(skb, tcp_header_len);
62196219
eaten = tcp_queue_rcv(sk, skb, &fragstolen);
62206220

net/ipv4/tcp_ipv4.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2025,7 +2025,7 @@ bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb,
20252025
*/
20262026
skb_condense(skb);
20272027

2028-
skb_dst_drop(skb);
2028+
tcp_cleanup_skb(skb);
20292029

20302030
if (unlikely(tcp_checksum_complete(skb))) {
20312031
bh_unlock_sock(sk);

0 commit comments

Comments
 (0)