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.
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.
