mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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.
 > 
 > 



      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®