Thread (15 messages) 15 messages, 4 authors, 8d ago

Re: [PATCH bpf v8 1/2] net: Validate protocol in skb_steal_sock() for BPF-assigned sockets

From: Kuniyuki Iwashima <kuniyu@google.com>
Date: 2026-06-09 17:14:53
Also in: bpf, linux-kselftest, lkml

On Tue, Jun 9, 2026 at 6:55 AM Jiayuan Chen [off-list ref] wrote:

On 6/9/26 6:34 AM, Kuniyuki Iwashima wrote:
quoted
On Mon, Jun 8, 2026 at 3:16 PM Alexei Starovoitov
[off-list ref] wrote:
quoted
On Mon Jun 8, 2026 at 2:35 PM PDT, Kuniyuki Iwashima wrote:
quoted
quoted
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)

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

quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help