netdev - Re: [PATCH v4 net-next 12/12] mlx5: support BIG TCP packets

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]

Date:   Sat, 7 May 2022 00:23:29 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Paolo Abeni <pabeni@...hat.com>,
        netdev <netdev@...r.kernel.org>, Coco Li <lixiaoyan@...gle.com>,
        Tariq Toukan <tariqt@...dia.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Leon Romanovsky <leon@...nel.org>
Subject: Re: [PATCH v4 net-next 12/12] mlx5: support BIG TCP packets

On Fri, May 06, 2022 at 07:37:34PM -0700, Jakub Kicinski wrote:
> On Fri, 6 May 2022 19:10:48 -0700 Eric Dumazet wrote:
> > On Fri, May 6, 2022 at 6:54 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > > Without our patches drivers/net/ethernet/mellanox/mlx5/core/ builds
> > > cleanly. Gotta be the new W=1 filed overflow warnings, let's bother
> > > Kees.  
> > 
> > Note that inline_hdr.start is a 2 byte array.
> > 
> > Obviously mlx5 driver copies more than 2 bytes of inlined headers.
> > 
> > mlx5e_insert_vlan(eseg->inline_hdr.start, skb, attr->ihs)
> > is called already with attr->ihs > 2
> > 
> > So it should already complain ?
> 
> It's a static checker, I presume it ignores attr->ihs because 
> it can't prove its value is indeed > 2. Unpleasant :/

I think it's actually the reverse. GCC keeps getting better about tracking
potential variable value ranges. In this case it thinks ihs WILL be > 2.
And this is bumping up against the kernel's lack of "intentional overflow"
annotations in source (i.e. structure layouts). But we can't protect
against unintentional overflow unless we've been able to explicitly
describe to the compiler what is intended.

-- 
Kees Cook