Re: [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()

Hi Matt,

Could you follow up on the comments by Sashiko ?

On Mon, May 4, 2026 at 9:14 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] NULL pointer dereferences in bpf_mptcp_sock_from_subflow due to lockless concurrent accesses
> - [High] Use-After-Free in BPF programs returning freed mptcp socket during teardown
> --
>
> commit 52dbcdaad7d49ff5db2faacf11c47b1c581024a8
> Author: Matthieu Baerts (NGI0) <[email protected]>
> mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()
>
> This commit fixes a type confusion issue in bpf_mptcp_sock_from_subflow() by
> checking if the socket is actually a TCP socket before calling sk_is_mptcp().
> Previously, raw sockets could bypass the IPPROTO_TCP check and cause incorrect
> assumptions about the socket type.
>
> > diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> > index 8a16672b94e2..4cc16cbeb328 100644
> > --- a/net/mptcp/bpf.c
> > +++ b/net/mptcp/bpf.c
> > @@ -14,7 +14,7 @@
> >
> >  struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
> >  {
> > -     if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> > +     if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
> >               return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
>
> Could this cause a regression by dereferencing a NULL pointer when accessed
> concurrently?
>
> In subflow_ulp_fallback(), icsk->icsk_ulp_data is set to NULL prior to
> clearing tcp_sk(sk)->is_mptcp. If a BPF program triggers in this window,
> sk_is_mptcp() would return true but mptcp_subflow_ctx(sk) would return NULL,
> leading to a NULL pointer dereference.
>
> Additionally, could ctx->conn be NULL when read here?
>
> During subflow_ulp_init(), the context is allocated and is_mptcp is set to 1,
> but ctx->conn is initially NULL. Passing NULL to mptcp_sk(ctx->conn) triggers
> a kernel panic on CONFIG_DEBUG_NET builds. On non-debug builds, it returns a
> non-NULL offset pointer, which bypasses BPF verifier NULL checks and causes
> invalid memory accesses.
>
> Could this introduce a regression with a use-after-free of the mptcp_sock
> during teardown?
>
> During subflow_ulp_release(), sock_put(ctx->conn) drops the reference to the
> parent mptcp_sock and may free it. However, ctx->conn is not cleared, and the
> subflow context remains valid until the RCU grace period ends.
>
> If a BPF tracing program triggers after sock_put(), could this retrieve
> and return the dangling ctx->conn pointer, enabling the BPF program to read
> from freed kernel slab memory?
>
> >
> >       return NULL;
> >  }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3