From: Chris Mason 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: 6578171a7ff0 ("bpf: add bpf_skb_change_proto helper") Fixes: 2be7e212d541 ("bpf: add bpf_skb_adjust_room helper") Signed-off-by: Chris Mason Signed-off-by: Alexei Starovoitov --- net/core/filter.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/net/core/filter.c b/net/core/filter.c index 21fcdc7cd950..9245009d9f1e 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -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; -- 2.55.0