bpf: Check the offset in bpf_skb_generic_{push,pop}()

To make room for len bytes at offset off, bpf_skb_generic_push() moves the
first off bytes and clears len bytes after them. It assumes that the first
off bytes are in the linear part. But in bpf_skb_adjust_room() off can be
the end of the IP header, and this header can be in a frag. Then the
function reads and writes after skb->tail, and can damage skb_shared_info.

bpf_skb_generic_pop() checks off + len, but this u32 sum can wrap to a
small value. bpf_skb_adjust_room() and bpf_skb_change_proto() calculate off
from skb_mac_header_len(). This length is a very large u32 when
network_header < mac_header. Then the check can pass, and memmove() reads
and writes far outside the skb.

Return -ENOMEM from both functions, like the present check, when
off > skb->len. Thus the sum cannot wrap. In bpf_skb_generic_push() also
pull the first off bytes into the linear part. Now bpf_skb_adjust_room()
fails when a prog grows an skb that ends before off. network_header stays
wrong.

Fixes: 6578171a7f ("bpf: add bpf_skb_change_proto helper")
Fixes: 2be7e212d5 ("bpf: add bpf_skb_adjust_room helper")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261101190535.1454146-28-alexei.starovoitov@gmail.com
This commit is contained in:
Chris Mason 2026-11-01 19:05:35 +00:00 committed by Alexei Starovoitov
parent 33be492c2e
commit ee36c9aa44
No known key found for this signature in database

View File

@ -3369,6 +3369,10 @@ static const struct bpf_func_proto bpf_skb_vlan_pop_proto = {
static int bpf_skb_generic_push(struct sk_buff *skb, u32 off, u32 len)
{
/* The bytes in front of @off are moved with skb->data. */
if (unlikely(off > skb->len || !pskb_may_pull(skb, off)))
return -ENOMEM;
/* Caller already did skb_cow() with meta_len+len as headroom,
* so no need to do it here.
*/
@ -3389,9 +3393,10 @@ static int bpf_skb_generic_pop(struct sk_buff *skb, u32 off, u32 len)
void *old_data;
/* skb_ensure_writable() is not needed here, as we're
* already working on an uncloned skb.
* already working on an uncloned skb. The first test
* keeps off + len from wrapping.
*/
if (unlikely(!pskb_may_pull(skb, off + len)))
if (unlikely(off > skb->len || !pskb_may_pull(skb, off + len)))
return -ENOMEM;
old_data = skb->data;