lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 16 Apr 2022 10:45:30 +0000 From: "liujian (CE)" <liujian56@...wei.com> To: Daniel Borkmann <daniel@...earbox.net>, "ast@...nel.org" <ast@...nel.org>, "andrii@...nel.org" <andrii@...nel.org>, "kafai@...com" <kafai@...com>, "songliubraving@...com" <songliubraving@...com>, "yhs@...com" <yhs@...com>, "john.fastabend@...il.com" <john.fastabend@...il.com>, "kpsingh@...nel.org" <kpsingh@...nel.org>, "davem@...emloft.net" <davem@...emloft.net>, "kuba@...nel.org" <kuba@...nel.org>, "sdf@...gle.com" <sdf@...gle.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>, "pabeni@...hat.com" <pabeni@...hat.com> Subject: RE: [PATCH bpf-next v3 1/3] net: Enlarge offset check value from 0xffff to INT_MAX in bpf_skb_load_bytes > -----Original Message----- > From: Daniel Borkmann [mailto:daniel@...earbox.net] > Sent: Saturday, April 16, 2022 3:32 AM > To: liujian (CE) <liujian56@...wei.com>; ast@...nel.org; andrii@...nel.org; > kafai@...com; songliubraving@...com; yhs@...com; > john.fastabend@...il.com; kpsingh@...nel.org; davem@...emloft.net; > kuba@...nel.org; sdf@...gle.com; netdev@...r.kernel.org; > bpf@...r.kernel.org; pabeni@...hat.com > Subject: Re: [PATCH bpf-next v3 1/3] net: Enlarge offset check value from > 0xffff to INT_MAX in bpf_skb_load_bytes > > 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@...wei.com> > > Acked-by: Song Liu <songliubraving@...com> > > --- > > 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.) > I only need change "offset > 0xffff". Delete " || len > INT_MAX " in v4. Thank you~ > > 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); > >
Powered by blists - more mailing lists