On Tue, Jun 9, 2026 at 6:55 AM Jiayuan Chen <[email protected]> wrote: > > > On 6/9/26 6:34 AM, Kuniyuki Iwashima wrote: > > On Mon, Jun 8, 2026 at 3:16 PM Alexei Starovoitov > > <[email protected]> wrote: > >> On Mon Jun 8, 2026 at 2:35 PM PDT, Kuniyuki Iwashima wrote: > >>>> btw is earlier sashiko TOCTOU concern real? > >>> Yes, the selftest in patch 2 should reproduce null-ptr-deref. > >>> (I tested one in a prior version) > >>> > >>> > >>>> iph->protocol = IPPROTO_TCP; > >>>> bpf_sk_assign_tcp_reqsk(udp_skb, tcp_sk); > >>>> iph->protocol = IPPROTO_UDP; > >>>> > >>>> as far as I can tell past bpf_sk_assign_tcp_reqsk() > >>>> the networking stack only looks at skb->protocol, > >>>> so modifying iph->protocol will only mess up > >>>> things on the wire (if this packet goes out). > >>> ip_rcv_finish_core() uses iph->protocol for early demux > >>> and ip_local_deliver_finish() also uses it for demux. > >> ok. Another idea... we can keep the checks on bpf side and > >> teach the verifier to invalidate packet pointers at > >> bpf_sk_assign_tcp_reqsk() > >> and introduce new concept of "readonly skb". > >> So after invalidation the reloaded sbk->data will be read only. > > Yes, this is exactly what I had in mind in the other thread :) > > > > ---8<--- > > On top of L4 validation in bpf_sk_assign() and bpf_sk_assign_tcp_reqsk(), > > can't we mark such an skb immutable after the helpers and catch > > subsequent writes to skb->data on the verifier ? > > ---8<--- > > > > > >> There is no such thing today. The verifier only understands PTR_TO_PACKET > >> as read/write. So it's not easy, but probably good thing to have > >> for this and other cases where bpf prog shouldn't touch the packet > >> once it called some kfunc/helper. > > This would be useful and we could (re)move some validations from > > the fast path. > > > Hi Kuniyuki and Alexei, > > > Thanks both for the discussion, it makes sense to me. > > I'll go back to fixing this in the helper itself, like the previous > version did.
I think even the previous version is the same type of change for intentional misuse. Both bpf_sk_assign() and bpf_sk_assign_tcp_reqsk() assumes sk_lookup() in advance, meaning the bpf must have dissected skb, and bpf_sk_assign() relies on it and has no protocol check. > > > Then, as a longer-term and more general solution, I'll try look into > marking the > skb as immutable after these helpers so the verifier can reject any > later writes > to the packet. >
