| Message ID | 20220318011245.43678-1-liujian56@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v2] net: Enlarge offset check value from 0xffff to 0x7fffffff in bpf_skb_load_bytes | expand |
On 3/18/22 2:12 AM, Liu Jian wrote: > The data length of skb frags + frag_list may be greater than 0xffff, > and skb_header_pointer can not handle negative offset and negative len. > So here 0x7ffffff is used to check the validity of offset and len. > > Fixes: 05c74e5e53f6 ("bpf: add bpf_skb_load_bytes helper") > Signed-off-by: Liu Jian <liujian56@huawei.com> > --- > v1->v2: sorry for this, change 0x7ffffffff to 0x7fffffff > net/core/filter.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/filter.c b/net/core/filter.c > index 9eb785842258..17865b896f7d 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -1722,7 +1722,7 @@ BPF_CALL_4(bpf_skb_load_bytes, const struct sk_buff *, skb, u32, offset, > { > void *ptr; > > - if (unlikely(offset > 0xffff)) > + if (unlikely(offset > 0x7fffffff || len > 0x7fffffff)) > goto err_clear; What are those magic numbers (and why not < 0 check)? Also, it's ugly you're adding these for skb_load_bytes but not skb_store_bytes, both are used in combination from tc BPF side. Can we come up with something better that works for both? Given you had to change this between v1 -> v2 from 0x7ffffffff to 0x7fffffff, please also add BPF selftests with corner cases so this gets properly tested. > ptr = skb_header_pointer(skb, offset, len, to); > Thanks, Daniel
diff --git a/net/core/filter.c b/net/core/filter.c index 9eb785842258..17865b896f7d 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1722,7 +1722,7 @@ BPF_CALL_4(bpf_skb_load_bytes, const struct sk_buff *, skb, u32, offset, { void *ptr; - if (unlikely(offset > 0xffff)) + if (unlikely(offset > 0x7fffffff || len > 0x7fffffff)) goto err_clear; ptr = skb_header_pointer(skb, offset, len, to);
The data length of skb frags + frag_list may be greater than 0xffff, and skb_header_pointer can not handle negative offset and negative len. So here 0x7ffffff is used to check the validity of offset and len. Fixes: 05c74e5e53f6 ("bpf: add bpf_skb_load_bytes helper") Signed-off-by: Liu Jian <liujian56@huawei.com> --- v1->v2: sorry for this, change 0x7ffffffff to 0x7fffffff net/core/filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)