On 4/14/22 3:59 PM, 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 INT_MAX is used to check the validity of offset and len.
Add the same change to the related function skb_store_bytes.
Fixes: 05c74e5e53f6 ("bpf: add bpf_skb_load_bytes helper")
Signed-off-by: Liu Jian <liujian56@xxxxxxxxxx>
Acked-by: Song Liu <songliubraving@xxxxxx>
---
v2->v3: change nothing
net/core/filter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 64470a727ef7..1571b6bc51ea 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1687,7 +1687,7 @@ BPF_CALL_5(bpf_skb_store_bytes, struct sk_buff *, skb, u32, offset,
if (unlikely(flags & ~(BPF_F_RECOMPUTE_CSUM | BPF_F_INVALIDATE_HASH)))
return -EINVAL;
- if (unlikely(offset > 0xffff))
+ if (unlikely(offset > INT_MAX || len > INT_MAX))
One more follow-up question, len param is checked by the verifier for the provided buffer.
Why is it needed here? Were you able to create e.g. map values of size larger than INT_MAX?
Please provide details. (Other than that, rest looks good.)
return -EFAULT;
if (unlikely(bpf_try_make_writable(skb, offset + len)))
return -EFAULT;
@@ -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 > INT_MAX || len > INT_MAX))
goto err_clear;
ptr = skb_header_pointer(skb, offset, len, to);
