netdev - RE: [PATCH bpf-next v3 1/3] net: Enlarge offset check value from 0xffff to INT_MAX in bpf_skb_load

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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);
> >