Re: [PATCH bpf v1 1/2] mptcp, bpf: reject bpf_sk_release() on msk
From: Kalpan Jani <hidden>
Date: 2026-09-17 11:07:28
Also in:
bpf, linux-kselftest, lkml, mptcp
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>, <redacted>, <redacted> Cc: "VEGA"<redacted>, "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"<redacted>, "Tejun Heo"<tj@kernel.org>, <redacted>, <redacted>, <redacted>, "Kalpan Jani"<redacted>, "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 (local) > [2] https://lore.kernel.org/e039e866-fe7e-41ef-ad41-92a76a123713@redhat.com (local) > > Cheers, > Matt > -- > Sponsored by the NGI0 Core fund. > >