[PATCH bpf v1 1/2] mptcp, bpf: reject bpf_sk_release() on msk

[PATCH bpf v1 1/2] mptcp, bpf: reject bpf_sk_release() on msk

[Jiayuan Chen]

A bpf prog can do this today:

	subflow = bpf_skc_lookup_tcp(...);
	msk = bpf_skc_to_mptcp_sock(subflow);
	bpf_sk_release(msk);

bpf_skc_to_mptcp_sock() returns subflow->conn without taking any
reference, so bpf_sk_release() drops a refcount nobody took on the msk,
and the subflow reference is leaked:

refcount_t: underflow; use-after-free.
WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xdf/0x120, CPU#2: mptcp_pair/440
Call Trace:
 <IRQ>
 sock_gen_put+0xda/0x100
 bpf_sk_release+0x5e/0xd0
 bpf_prog_780c70b94862636c_rel_msk+0x127/0x132
 __dev_queue_xmit+0x104b/0x3c90
 ip_finish_output2+0x9af/0x1c40
 __ip_finish_output+0x510/0x7e0
 ip_finish_output+0x2f/0x320
 ip_output+0x17a/0x3f0
 ip_local_out+0x12f/0x170
 __ip_queue_xmit+0x81d/0x1d50
 ip_queue_xmit+0x4a/0x80
 __tcp_transmit_skb+0x2f6f/0x5110
 __tcp_send_ack.part.0+0x385/0x740
 tcp_send_ack+0x70/0x90
 __tcp_ack_snd_check+0x1c9/0x8d0
 tcp_rcv_established+0xa1e/0x44f0
 tcp_v4_do_rcv+0x4b8/0xb30
 tcp_v4_rcv+0x27af/0x3e60
 ip_protocol_deliver_rcu+0x95/0x410
 ip_local_deliver_finish+0x357/0x5b0
 ip_local_deliver+0x15c/0x1c0
 ip_rcv+0x284/0x320

bpf_skc_to_mptcp_sock() is listed in is_ptr_cast_function(), so if the
subflow is a referenced obj (returned by bpf_skc_lookup_tcp()), the msk
becomes a referenced obj too, which lets bpf_sk_release() take it. That
list is for helpers casting a sock to another type at the same address,
which does not hold here: msk and subflow are two different socks.

Drop it from is_ptr_cast_function(). To keep the msk from outliving the
subflow it was derived from, tie the two together with the existing
parent_id, so this gets rejected as well:

	subflow = bpf_skc_lookup_tcp(...);
	msk = bpf_skc_to_mptcp_sock(subflow);
	bpf_sk_release(subflow);
	msk->token;			/* rejected now */

The other option was to reject bpf_skc_to_mptcp_sock() on a referenced
subflow altogether, but that breaks progs which only read msk fields, so
go with parent_id.

Reported-by: VEGA <vega@nebusec.ai>
Fixes: 3bc253c2e652 ("bpf: Add bpf_skc_to_mptcp_sock_proto")
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 kernel/bpf/verifier.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9e79750e2480..5db49a0c346f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -522,11 +522,21 @@ static bool is_ptr_cast_function(enum bpf_func_id func_id)
 		func_id == BPF_FUNC_skc_to_tcp_sock ||
 		func_id == BPF_FUNC_skc_to_tcp6_sock ||
 		func_id == BPF_FUNC_skc_to_udp6_sock ||
-		func_id == BPF_FUNC_skc_to_mptcp_sock ||
 		func_id == BPF_FUNC_skc_to_tcp_timewait_sock ||
 		func_id == BPF_FUNC_skc_to_tcp_request_sock;
 }
 
+/*
+ * bpf_skc_to_mptcp_sock() does not cast its argument. It returns the parent
+ * MPTCP socket of the subflow that was passed in, so the return value must not
+ * inherit the argument's reference, or bpf_sk_release() would put the wrong
+ * socket.
+ */
+static bool is_ptr_derive_function(enum bpf_func_id func_id)
+{
+	return func_id == BPF_FUNC_skc_to_mptcp_sock;
+}
+
 static bool is_sync_callback_calling_kfunc(u32 btf_id);
 static bool is_async_callback_calling_kfunc(u32 btf_id);
 static bool is_callback_calling_kfunc(u32 btf_id);
@@ -11369,6 +11379,14 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 		bpf_diag_mod_begin(env, &regs[BPF_REG_0], NULL, BPF_DIAG_MOD_WRITE);
 		regs[BPF_REG_0].type &= ~PTR_MAYBE_NULL;
 		regs[BPF_REG_0].id = meta.ref_obj.id;
+	} else if (is_ptr_derive_function(func_id) &&
+		   find_reference_state(env->cur_state, meta.ref_obj.id)) {
+		err = validate_ref_obj(env, &meta.ref_obj);
+		if (err)
+			return err;
+
+		/* Ensures we don't access the object after a release_reference() */
+		regs[BPF_REG_0].parent_id = meta.ref_obj.id;
 	} else if (is_acquire_function(func_id, meta.map.ptr)) {
 		int id = acquire_reference(env, insn_idx, 0);
 
-- 
2.43.0

[PATCH bpf v1 2/2] selftests/bpf: add verifier tests for bpf_skc_to_mptcp_sock

[Jiayuan Chen]

Cover what the verifier has to get right for the msk returned by
bpf_skc_to_mptcp_sock(): it must not be accepted by bpf_sk_release(), it
must go away with the subflow it was derived from, and the subflow
reference still has to be released.

  # ./test_progs -t verifier_mptcp
  #658/1   verifier_mptcp/bpf_skc_to_mptcp_sock: release the acquired subflow:OK
  #658/2   verifier_mptcp/bpf_skc_to_mptcp_sock: msk is not the acquired subflow:OK
  #658/3   verifier_mptcp/bpf_skc_to_mptcp_sock: msk dies with the subflow it came from:OK
  #658/4   verifier_mptcp/bpf_skc_to_mptcp_sock: subflow is still owned after the cast:OK
  #658     verifier_mptcp:OK
  Summary: 1/4 PASSED, 0 SKIPPED, 0/0 FAILED

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../selftests/bpf/progs/verifier_mptcp.c      | 101 ++++++++++++++++++
 2 files changed, 103 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_mptcp.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index f7f94ccebce2..63316e4be6bb 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -74,6 +74,7 @@
 #include "verifier_mem_size_reg.skel.h"
 #include "verifier_meta_access.skel.h"
 #include "verifier_movsx.skel.h"
+#include "verifier_mptcp.skel.h"
 #include "verifier_mtu.skel.h"
 #include "verifier_mul.skel.h"
 #include "verifier_netfilter_ctx.skel.h"
@@ -238,6 +239,7 @@ void test_verifier_may_goto_2(void)           { RUN(verifier_may_goto_2); }
 void test_verifier_mem_size_reg(void)         { RUN(verifier_mem_size_reg); }
 void test_verifier_meta_access(void)          { RUN(verifier_meta_access); }
 void test_verifier_movsx(void)                 { RUN(verifier_movsx); }
+void test_verifier_mptcp(void)                { RUN(verifier_mptcp); }
 void test_verifier_mul(void)                  { RUN(verifier_mul); }
 void test_verifier_netfilter_ctx(void)        { RUN(verifier_netfilter_ctx); }
 void test_verifier_netfilter_retcode(void)    { RUN(verifier_netfilter_retcode); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_mptcp.c b/tools/testing/selftests/bpf/progs/verifier_mptcp.c
new file mode 100644
index 000000000000..1f679265e043
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_mptcp.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "bpf_tracing_net.h"
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+__u32 token;
+
+static __always_inline struct bpf_sock *lookup_subflow(struct __sk_buff *skb)
+{
+	struct bpf_sock_tuple tuple = {};
+
+	return bpf_skc_lookup_tcp(skb, &tuple, sizeof(tuple.ipv4),
+				  BPF_F_CURRENT_NETNS, 0);
+}
+
+SEC("tc")
+__description("bpf_skc_to_mptcp_sock: release the acquired subflow")
+__success
+int mptcp_cast_release_subflow(struct __sk_buff *skb)
+{
+	struct mptcp_sock *msk;
+	struct bpf_sock *sk;
+
+	sk = lookup_subflow(skb);
+	if (!sk)
+		return 0;
+
+	msk = bpf_skc_to_mptcp_sock(sk);
+	if (msk)
+		token = msk->token;
+
+	bpf_sk_release(sk);
+	return 0;
+}
+
+SEC("tc")
+__description("bpf_skc_to_mptcp_sock: msk is not the acquired subflow")
+__failure __msg("release helper bpf_sk_release expects referenced PTR_TO_BTF_ID")
+int mptcp_cast_release_msk(struct __sk_buff *skb)
+{
+	struct mptcp_sock *msk;
+	struct bpf_sock *sk;
+
+	sk = lookup_subflow(skb);
+	if (!sk)
+		return 0;
+
+	msk = bpf_skc_to_mptcp_sock(sk);
+	if (!msk) {
+		bpf_sk_release(sk);
+		return 0;
+	}
+
+	bpf_sk_release((struct bpf_sock *)msk);
+	return 0;
+}
+
+SEC("tc")
+__description("bpf_skc_to_mptcp_sock: msk dies with the subflow it came from")
+__failure __msg("invalid mem access 'scalar'")
+int mptcp_cast_use_after_release(struct __sk_buff *skb)
+{
+	struct mptcp_sock *msk;
+	struct bpf_sock *sk;
+
+	sk = lookup_subflow(skb);
+	if (!sk)
+		return 0;
+
+	msk = bpf_skc_to_mptcp_sock(sk);
+	if (!msk) {
+		bpf_sk_release(sk);
+		return 0;
+	}
+
+	bpf_sk_release(sk);
+	token = msk->token;
+	return 0;
+}
+
+SEC("tc")
+__description("bpf_skc_to_mptcp_sock: subflow is still owned after the cast")
+__failure __msg("Unreleased reference")
+int mptcp_cast_leak_subflow(struct __sk_buff *skb)
+{
+	struct mptcp_sock *msk;
+	struct bpf_sock *sk;
+
+	sk = lookup_subflow(skb);
+	if (!sk)
+		return 0;
+
+	msk = bpf_skc_to_mptcp_sock(sk);
+	if (msk)
+		token = msk->token;
+
+	return 0;
+}
-- 
2.43.0

Re: [PATCH bpf v1 2/2] selftests/bpf: add verifier tests for bpf_skc_to_mptcp_sock

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1688 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 8b439e194bcc5..8d5908e34c9b4 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/verifier_mptcp.c b/tools/testing/selftests/bpf/progs/verifier_mptcp.c
> new file mode 100644
> index 0000000000000..1f679265e0437
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_mptcp.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "bpf_tracing_net.h"
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"

This isn't a bug, but since nothing from bpf_tracing_net.h is used here,
would a plain #include "vmlinux.h" (as progs/verifier_sock.c does) be enough
for struct mptcp_sock?

The file doesn't use any AF_*/SOL_*/TCP_*/IPPROTO_* constants or
bpf_core_read/BPF_CORE_READ that bpf_tracing_net.h provides. It only needs
the transitive vmlinux.h for struct mptcp_sock. The 9 verifier_*.c files
that need BTF kernel types include vmlinux.h directly.

> +
> +char _license[] SEC("license") = "GPL";
> +
> +__u32 token;
> +
> +static __always_inline struct bpf_sock *lookup_subflow(struct __sk_buff *skb)
> +{
> +	struct bpf_sock_tuple tuple = {};
> +
> +	return bpf_skc_lookup_tcp(skb, &tuple, sizeof(tuple.ipv4),
> +				  BPF_F_CURRENT_NETNS, 0);
> +}

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35199493791

Re: [PATCH bpf v1 1/2] mptcp, bpf: reject bpf_sk_release() on msk

[Matthieu Baerts]

Hi Jiayuan,

+Cc Kalpan, Paolo.

On 17/09/2026 10:11, Jiayuan Chen wrote:
> A bpf prog can do this today:
> 
> 	subflow = bpf_skc_lookup_tcp(...);
> 	msk = bpf_skc_to_mptcp_sock(subflow);
> 	bpf_sk_release(msk);
> 
> bpf_skc_to_mptcp_sock() returns subflow->conn without taking any
> reference, so bpf_sk_release() drops a refcount nobody took on the msk,
> and the subflow reference is leaked:
Thank you for looking at this!

Note that Kalpan was looking at this [1], and Paolo suggested removing
the helper [2] (but we failed to review the last version so far, sorry
about that...)

I don't know if there are progs already using it. If yes, I guess your
approach is better (but I'm not comfortable reviewing verifier's code).

@Kalpan, WDYT?

[1] https://lore.kernel.org/20260818120437.3949686-1-kalpan.jani@mpiricsoftware.com
[2] https://lore.kernel.org/e039e866-fe7e-41ef-ad41-92a76a123713@redhat.com

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH bpf v1 1/2] mptcp, bpf: reject bpf_sk_release() on msk

[Kalpan Jani]

Hi Matt, Jiayuan,

Thanks for looping me in, and thanks Jiayuan for catching this.

This is a separate bug from what I was fixing. My patch hardens the
lockless read of ->conn (the NULL-deref/UAF from #622) with
rcu_dereference()/acquire-release/SOCK_RCU_FREE, but that doesn't
touch the helper's return-value ownership contract, so the refcount
mismatch you're describing exists independently of whether the read
itself is safe. Hardening the read fixes my bug; it doesn't and can't
fix this one.

Given that, I think Paolo's original suggestion (dropping the helper
from tracing_prog_func_proto()) is the right call. I don't see a way
to fix the refcounting issue while keeping the helper's current shape
(returning a different object than the input, with no reference taken)
without a bigger rework of what it hands back to a BPF program.

I checked which prog types can actually chain bpf_skc_lookup_tcp()
into bpf_skc_to_mptcp_sock() - it's broader than just tracing.
sock_addr, tc_cls_act, xdp, and sk_msg all have bpf_skc_lookup_tcp()
available and fall through to bpf_sk_base_func_proto(), which is
where bpf_skc_to_mptcp_sock() is registered, so the same pattern
Jiayuan showed is reachable from all four of them, not only from
tracing programs. sock_ops doesn't register bpf_skc_lookup_tcp() at
all, and cg_skb has the lookup helper but doesn't fall through to
bpf_sk_base_func_proto(), so neither of those two can chain into it
this way. I haven't traced the verifier's reference-tracking closely
enough to rule out some other path I'm not aware of.

I don't have visibility into whether any real tracing (or TC/XDP/etc.)
programs use this helper today. If that's a live concern for anyone,
I'll defer to whoever has that visibility.

I can send a version that drops bpf_skc_to_mptcp_sock() from
tracing_prog_func_proto() as Paolo suggested. Given what Jiayuan found
also applies to the non-tracing paths above, should that patch just
remove the BPF_FUNC_skc_to_mptcp_sock case from
bpf_sk_base_func_proto() as well, rather than leaving it there for
sock_addr/tc/xdp/sk_msg?

Cheers,
Kalpan Jani


From: Matthieu Baerts <matttbe@kernel.org>
To: "Jiayuan Chen"<jiayuan.chen@linux.dev>, <bpf@vger.kernel.org>, <mptcp@lists.linux.dev>
Cc: "VEGA"<vega@nebusec.ai>, "Alexei Starovoitov"<ast@kernel.org>, "Daniel Borkmann"<daniel@iogearbox.net>, "John Fastabend"<john.fastabend@gmail.com>, "Andrii Nakryiko"<andrii@kernel.org>, "Eduard Zingerman"<eddyz87@gmail.com>, "Kumar Kartikeya Dwivedi"<memxor@gmail.com>, "Martin KaFai Lau"<martin.lau@linux.dev>, "Song Liu"<song@kernel.org>, "Yonghong Song"<yonghong.song@linux.dev>, "Jiri Olsa"<jolsa@kernel.org>, "Emil Tsalapatis"<emil@etsalapatis.com>, "Ihor Solodrai"<ihor.solodrai@linux.dev>, "Shuah Khan"<shuah@kernel.org>, "Mat Martineau"<martineau@kernel.org>, "Geliang Tang"<geliang@kernel.org>, "Matt Bobrowski"<mattbobrowski@google.com>, "Tejun Heo"<tj@kernel.org>, <linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>, <netdev@vger.kernel.org>, "Kalpan Jani"<kalpan.jani@mpiricsoftware.com>, "Paolo Abeni"<pabeni@redhat.com>
Date: Thu, 17 Sep 2026 15:09:13 +0530
Subject: Re: [PATCH bpf v1 1/2] mptcp, bpf: reject bpf_sk_release() on msk

 > Hi Jiayuan,
 > 
 > +Cc Kalpan, Paolo.
 > 
 > On 17/09/2026 10:11, Jiayuan Chen wrote:
 > > A bpf prog can do this today:
 > > 
 > >     subflow = bpf_skc_lookup_tcp(...);
 > >     msk = bpf_skc_to_mptcp_sock(subflow);
 > >     bpf_sk_release(msk);
 > > 
 > > bpf_skc_to_mptcp_sock() returns subflow->conn without taking any
 > > reference, so bpf_sk_release() drops a refcount nobody took on the msk,
 > > and the subflow reference is leaked:
 > Thank you for looking at this!
 > 
 > Note that Kalpan was looking at this [1], and Paolo suggested removing
 > the helper [2] (but we failed to review the last version so far, sorry
 > about that...)
 > 
 > I don't know if there are progs already using it. If yes, I guess your
 > approach is better (but I'm not comfortable reviewing verifier's code).
 > 
 > @Kalpan, WDYT?
 > 
 > [1] https://lore.kernel.org/20260818120437.3949686-1-kalpan.jani@mpiricsoftware.com
 > [2] https://lore.kernel.org/e039e866-fe7e-41ef-ad41-92a76a123713@redhat.com
 > 
 > Cheers,
 > Matt
 > -- 
 > Sponsored by the NGI0 Core fund.
 > 
 >