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 <[email protected]> To: "Jiayuan Chen"<[email protected]>, <[email protected]>, <[email protected]> Cc: "VEGA"<[email protected]>, "Alexei Starovoitov"<[email protected]>, "Daniel Borkmann"<[email protected]>, "John Fastabend"<[email protected]>, "Andrii Nakryiko"<[email protected]>, "Eduard Zingerman"<[email protected]>, "Kumar Kartikeya Dwivedi"<[email protected]>, "Martin KaFai Lau"<[email protected]>, "Song Liu"<[email protected]>, "Yonghong Song"<[email protected]>, "Jiri Olsa"<[email protected]>, "Emil Tsalapatis"<[email protected]>, "Ihor Solodrai"<[email protected]>, "Shuah Khan"<[email protected]>, "Mat Martineau"<[email protected]>, "Geliang Tang"<[email protected]>, "Matt Bobrowski"<[email protected]>, "Tejun Heo"<[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, "Kalpan Jani"<[email protected]>, "Paolo Abeni"<[email protected]> 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/[email protected] > [2] https://lore.kernel.org/[email protected] > > Cheers, > Matt > -- > Sponsored by the NGI0 Core fund. > >
