netdev - Re: [PATCH bpf 1/3] bpf, sockmap: Check for any of tcp_bpf

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: <87h6wqyaq6.fsf@cloudflare.com>
Date:   Mon, 16 Jan 2023 12:31:11 +0100
From:   Jakub Sitnicki <jakub@...udflare.com>
To:     Dan Carpenter <error27@...il.com>
Cc:     oe-kbuild@...ts.linux.dev, netdev@...r.kernel.org, lkp@...el.com,
        oe-kbuild-all@...ts.linux.dev, kernel-team@...udflare.com,
        John Fastabend <john.fastabend@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        syzbot+04c21ed96d861dccc5cd@...kaller.appspotmail.com
Subject: Re: [PATCH bpf 1/3] bpf, sockmap: Check for any of tcp_bpf_prots
 when cloning a listener

On Mon, Jan 16, 2023 at 02:13 PM +03, Dan Carpenter wrote:
> On Mon, Jan 16, 2023 at 11:09:02AM +0100, Jakub Sitnicki wrote:

[...]

>> Smatch doesn't seem to graps the 2D array concept here. We can make it
>> happy by being explicit but harder on the eyes:
>> 
>> 	if (&tcp_bpf_prots[0][0] <= prot && prot < &tcp_bpf_prots[ARRAY_SIZE(tcp_bpf_prots)][0])
>> 		newsk->sk_prot = sk->sk_prot_creator;
>
> Huh.  I can silence this false positive in Smatch...  It never even
> occured to me that this was a two dimensional array (I only have the
> information in the email).
>

No need. Eric's macro helper makes Smatch happy. I'll use it in v2.

>> 
>> Clang can do pointer arithmetic on 2D arrays just fine :-)
>
> Heh.  I must have an older version of Clang.
>
>   CC      net/ipv4/tcp_bpf.o
> net/ipv4/tcp_bpf.c:644:41: warning: array index 2 is past the end of the array (that has type 'struct proto[2][4]') [-Warray-bounds]
>         if (tcp_bpf_prots[0] <= prot && prot < tcp_bpf_prots[ARRAY_SIZE(tcp_bpf_prots)])
>                                                ^             ~~~~~~~~~~~~~~~~~~~~~~~~~
> net/ipv4/tcp_bpf.c:544:1: note: array 'tcp_bpf_prots' declared here
> static struct proto tcp_bpf_prots[TCP_BPF_NUM_PROTS][TCP_BPF_NUM_CFGS];
> ^
> 1 warning generated.

FWIW, I've checked against:

$ clang --version
clang version 15.0.6 (Fedora 15.0.6-2.fc37)

Gotta keep it fresh to be able to build bpf selftests ;-)
But I sure don't want to break builds with older Clangs.

Thanks for pointing it out.