netdev - Re: [PATCH bpf-next] bpf, net: Introduce skb_pointer_if

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 linux-cve-announce PHC
Open Source and information security mailing list archives

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

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

Message-ID: <CAADnVQJ+NSeVOJROKHYY1VVRDwK8Gm8jgSoRTCmbrN8XG=y3JA@mail.gmail.com>
Date: Wed, 19 Jul 2023 09:30:22 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: David Laight <David.Laight@...lab.com>
Cc: "davem@...emloft.net" <davem@...emloft.net>, "kuba@...nel.org" <kuba@...nel.org>, 
	"edumazet@...gle.com" <edumazet@...gle.com>, "pabeni@...hat.com" <pabeni@...hat.com>, 
	"daniel@...earbox.net" <daniel@...earbox.net>, "andrii@...nel.org" <andrii@...nel.org>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>, 
	"kernel-team@...com" <kernel-team@...com>
Subject: Re: [PATCH bpf-next] bpf, net: Introduce skb_pointer_if_linear().

On Wed, Jul 19, 2023 at 6:10 AM David Laight <David.Laight@...lab.com> wrote:
>
> From: Alexei Starovoitov
> > Sent: 19 July 2023 00:40
> >
> > Network drivers always call skb_header_pointer() with non-null buffer.
> > Remove !buffer check to prevent accidental misuse of skb_header_pointer().
> > Introduce skb_pointer_if_linear() instead.
> >
> ...
> > +static inline void * __must_check
> > +skb_pointer_if_linear(const struct sk_buff *skb, int offset, int len)
> > +{
> > +     if (likely(skb_headlen(skb) - offset >= len))
> > +             return skb->data + offset;
> > +     return NULL;
> > +}
>
> Shouldn't both 'offset' and 'len' be 'unsigned int' ?
>
> The check should probably be written:
>                 offset + len <= skb_headlen(skb)
> so that it fails if 'offset' is also large.
> (Provided 'offset + len' itself doesn't wrap.)

I agree that this style is easier to read, but
consistency with skb_header_pointer() trumps all such considerations.