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.