Skip to content

Commit 1eacc87

Browse files
author
Sabrina Dubroca
committed
tcp: drop secpath at the same time as we currently drop dst
JIRA: https://issues.redhat.com/browse/RHEL-69649 JIRA: https://issues.redhat.com/browse/RHEL-83224 CVE: CVE-2025-21864 commit 9b6412e Author: Sabrina Dubroca <sd@queasysnail.net> Date: Mon Feb 17 11:23:35 2025 +0100 tcp: drop secpath at the same time as we currently drop dst 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 <xmu@redhat.com> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/5055ba8f8f72bdcb602faa299faca73c280b7735.1739743613.git.sd@queasysnail.net Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Sabrina Dubroca <sdubroca@redhat.com>
1 parent ef2902a commit 1eacc87

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
@@ -40,6 +40,7 @@
4040
#include <net/inet_ecn.h>
4141
#include <net/dst.h>
4242
#include <net/mptcp.h>
43+
#include <net/xfrm.h>
4344

4445
#include <linux/seq_file.h>
4546
#include <linux/memcontrol.h>
@@ -640,6 +641,19 @@ void tcp_fin(struct sock *sk);
640641
void tcp_check_space(struct sock *sk);
641642
void tcp_sack_compress_send_ack(struct sock *sk);
642643

644+
static inline void tcp_cleanup_skb(struct sk_buff *skb)
645+
{
646+
skb_dst_drop(skb);
647+
secpath_reset(skb);
648+
}
649+
650+
static inline void tcp_add_receive_queue(struct sock *sk, struct sk_buff *skb)
651+
{
652+
DEBUG_NET_WARN_ON_ONCE(skb_dst(skb));
653+
DEBUG_NET_WARN_ON_ONCE(secpath_exists(skb));
654+
__skb_queue_tail(&sk->sk_receive_queue, skb);
655+
}
656+
643657
/* tcp_timer.c */
644658
void tcp_init_xmit_timers(struct sock *);
645659
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
@@ -194,7 +194,7 @@ void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
194194
if (!skb)
195195
return;
196196

197-
skb_dst_drop(skb);
197+
tcp_cleanup_skb(skb);
198198
/* segs_in has been initialized to 1 in tcp_create_openreq_child().
199199
* Hence, reset segs_in to 0 before calling tcp_segs_in()
200200
* to avoid double counting. Also, tcp_segs_in() expects
@@ -211,7 +211,7 @@ void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
211211
TCP_SKB_CB(skb)->tcp_flags &= ~TCPHDR_SYN;
212212

213213
tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
214-
__skb_queue_tail(&sk->sk_receive_queue, skb);
214+
tcp_add_receive_queue(sk, skb);
215215
tp->syn_data_acked = 1;
216216

217217
/* 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
@@ -4803,7 +4803,7 @@ static void tcp_ofo_queue(struct sock *sk)
48034803
tcp_rcv_nxt_update(tp, TCP_SKB_CB(skb)->end_seq);
48044804
fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
48054805
if (!eaten)
4806-
__skb_queue_tail(&sk->sk_receive_queue, skb);
4806+
tcp_add_receive_queue(sk, skb);
48074807
else
48084808
kfree_skb_partial(skb, fragstolen);
48094809

@@ -4994,7 +4994,7 @@ static int __must_check tcp_queue_rcv(struct sock *sk, struct sk_buff *skb,
49944994
skb, fragstolen)) ? 1 : 0;
49954995
tcp_rcv_nxt_update(tcp_sk(sk), TCP_SKB_CB(skb)->end_seq);
49964996
if (!eaten) {
4997-
__skb_queue_tail(&sk->sk_receive_queue, skb);
4997+
tcp_add_receive_queue(sk, skb);
49984998
skb_set_owner_r(skb, sk);
49994999
}
50005000
return eaten;
@@ -5077,7 +5077,7 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
50775077
__kfree_skb(skb);
50785078
return;
50795079
}
5080-
skb_dst_drop(skb);
5080+
tcp_cleanup_skb(skb);
50815081
__skb_pull(skb, tcp_hdr(skb)->doff * 4);
50825082

50835083
reason = SKB_DROP_REASON_NOT_SPECIFIED;
@@ -5988,7 +5988,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
59885988
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPHPHITS);
59895989

59905990
/* Bulk data transfer: receiver */
5991-
skb_dst_drop(skb);
5991+
tcp_cleanup_skb(skb);
59925992
__skb_pull(skb, tcp_header_len);
59935993
eaten = tcp_queue_rcv(sk, skb, &fragstolen);
59945994

net/ipv4/tcp_ipv4.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1773,7 +1773,7 @@ bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb,
17731773
*/
17741774
skb_condense(skb);
17751775

1776-
skb_dst_drop(skb);
1776+
tcp_cleanup_skb(skb);
17771777

17781778
if (unlikely(tcp_checksum_complete(skb))) {
17791779
bh_unlock_sock(sk);

0 commit comments

Comments
 (0)