linux-kernel - Re: [PATCH] s390: disable -Warray-bounds

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: <CAHk-=wiFn-_OaWKY=nXt3YSiy=obrNoQW_u7zKO7qoArez=GUw@mail.gmail.com>
Date:   Wed, 8 Jun 2022 16:59:29 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Kees Cook <keescook@...omium.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        David Howells <dhowells@...hat.com>
Cc:     Sven Schnelle <svens@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] s390: disable -Warray-bounds

On Wed, Jun 8, 2022 at 2:33 PM Kees Cook <keescook@...omium.org> wrote:
>
> I and others have been working through a bunch of them, though yes,
> they're not all fixed yet. I've been trying to track it here[1], but
> many of those fixes are only in -next.

Hmm. Even with that disabled, I get a few warnings I *really* would
want to get rid of.

The one in ipuv3-crtc.c seems valid about "address used as boolean is
always true".

The 'dangling-pointer' warning does seem interesting, but not when the
compiler does as bad a job as gcc seems to do.

See the attached patch for

 (a) make the s390 "use -Wno-array-bounds for gcc-12" be generic

 (b) fix the ipuv3-crtc.c one. IMX people?

 (c) disable -Wdangling-pointer entirely for now

but that still leaves the netfs_i_context games, which gcc-12 is very
unhappy about:

  In function ‘fortify_memset_chk’,
      inlined from ‘netfs_i_context_init’ at ./include/linux/netfs.h:327:2,
      inlined from ‘afs_set_netfs_context’ at fs/afs/inode.c:61:2,
      inlined from ‘afs_inode_init_from_status’ at fs/afs/inode.c:139:2:
  ./include/linux/fortify-string.h:258:25: error: call to
‘__write_overflow_field’ declared with attribute warning: detected
write beyond size of field (1st parameter); maybe use struct_group()?
[-Werror=attribute-warning]

and I do kind of agree with the compiler in that case. That code
should have some kind of

  struct container {
        struct inode inode;
        struct netfs_i_context ctx;
   };

thing, and aim to do that instead of the pointer arithmetic games.

Ceph seems to trigger the exact same thing.

There's also an annoying mlx5 issue, with gcc apparently not tracking
the usage of

        struct lag_tracker tracker;

well enough (it's never used if do_bond is false, but probably some
inlining change means that gcc doesn't see that).

DavidH - mind looking at the netfs_i_context_init() thing?

I'd like to use something more surgical than
CONFIG_CC_NO_ARRAY_BOUNDS, but considering the s390 issues, it may not
even be worth it. Kees, just how far away are we from that being ok on
x86-64?

I did consider making CONFIG_CC_NO_ARRAY_BOUNDS be more akin to

 config CC_NO_ARRAY_BOUNDS
        bool
        depends on CC_IS_GCC
        depends on GCC_VERSION >= 120000 && GCC_VERSION < 130000
        default GCC12_NO_ARRAY_BOUNDS

and then s390 and any subsystem that triggers the -Warray-bounds problem can do

        select GCC12_NO_ARRAY_BOUNDS

to show that they have issues with the new gcc12 rules.

That would be at least a bit more surgical..

                  Linus

View attachment "patch.diff" of type "text/x-patch" (3073 bytes)