Re: -Warray-bounds fun again

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Sven Schnelle <svens@linux.ibm.com>
Cc: Kees Cook <keescook@chromium.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-hardening@vger.kernel.org, krebbel@linux.ibm.com,
	Ilya Leoshkevich <iii@linux.ibm.com>,
	Heiko Carstens <hca@linux.ibm.com>
Subject: Re: -Warray-bounds fun again
Date: Thu, 21 Apr 2022 09:10:33 -0700	[thread overview]
Message-ID: <CAHk-=wgv4NLtEowsmX+0jq_nBWXFp8jtruX6U3SDm52N=ftkgg@mail.gmail.com> (raw)
In-Reply-To: <yt9dzgkelelc.fsf@linux.ibm.com>

On Thu, Apr 21, 2022 at 7:02 AM Sven Schnelle <svens@linux.ibm.com> wrote:
>
> The obvious 'fix' is to use absolute_pointer():
>
> #define S390_lowcore (*((struct lowcore *)absolute_pointer(0)))
>
> That makes the warning go away, but unfortunately the compiler no longer
> knows that the memory access is fitting into a load/store with a 12 bit
> displacement.

In the gcc bugzilla for us needing to do these games:

    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578

one of the suggestions was "I recommend suppressing the warning either
by #pragma GCC diagnostic or by making the pointer volatile".

But READ_ONCE() should already be doing that volatile thing, so that
suggestion may not work with gcc-12 any more.

It is *possible* that gcc-12 has now special-cased the very special
issue of a cast of the constant zero. That is how NULL was
traditionally defined.

So just out of a perverse curiosity, what happens if you do something like this:

   #define S390_lowcore_end ((struct lowcore *)sizeof(struct lowcore))
   #define S390_lowcore (S390_lowcore_end[-1])

instead? It should get the same value in the end, but it doesn't have
that special case of "cast an integer constant 0 to a pointer".

I suspect it probably doesn't help, because gcc will still see "oh,
you're basing this off address zero".

Another thing to try might be to remove the initial 16 bytes of
padding from 'struct lowcore' (it looks like the first 20 bytes are
not used - so leave 4 bytes of padding still), and use

   #define S390_lowcore (*((struct lowcore_nopad *)16))

instead. Then gcc will never see that 0, and hopefully the "he's
accessing based off a NULL pointer" logic will go away.

Because right now, our absolute_pointer() protection against this
horrible gcc mis-feature is literally based on hiding the value from
the compiler with an inline asm, and by virtue of hiding the value
then yes, gcc will have to go through a register base pointer and
cannot see that it fits in 12 bits.

.. or you just need to disable -Warray-bounds on s390.

              Linus