From: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
To: "Matthieu Baerts" <matttbe@kernel.org>
Cc: "Jiayuan Chen" <jiayuan.chen@linux.dev>,
"bpf" <bpf@vger.kernel.org>, "mptcp" <mptcp@lists.linux.dev>,
"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" <linux-kernel@vger.kernel.org>,
"linux-kselftest" <linux-kselftest@vger.kernel.org>,
"netdev" <netdev@vger.kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>
Subject: Re: [PATCH bpf v1 1/2] mptcp, bpf: reject bpf_sk_release() on msk
Date: Thu, 17 Sep 2026 16:36:44 +0530 [thread overview]
Message-ID: <1a0af0c25d8.352950d2114190.972790069861537729@mpiricsoftware.com> (raw)
In-Reply-To: <18f86b06-7477-4750-b57e-094df4310e4a@kernel.org>
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.
>
>
prev parent reply other threads:[~2026-09-17 11:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-17 8:11 Jiayuan Chen
2026-09-17 8:11 ` [PATCH bpf v1 2/2] selftests/bpf: add verifier tests for bpf_skc_to_mptcp_sock Jiayuan Chen
2026-09-17 9:19 ` bot+bpf-ci
2026-09-17 9:39 ` [PATCH bpf v1 1/2] mptcp, bpf: reject bpf_sk_release() on msk Matthieu Baerts
2026-09-17 11:06 ` Kalpan Jani [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1a0af0c25d8.352950d2114190.972790069861537729@mpiricsoftware.com \
--to=kalpan.jani@mpiricsoftware.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=geliang@kernel.org \
--cc=ihor.solodrai@linux.dev \
--cc=jiayuan.chen@linux.dev \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=martineau@kernel.org \
--cc=mattbobrowski@google.com \
--cc=matttbe@kernel.org \
--cc=memxor@gmail.com \
--cc=mptcp@lists.linux.dev \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=tj@kernel.org \
--cc=vega@nebusec.ai \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®