Re: -Warray-bounds fun again

From: Sven Schnelle <svens@linux.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
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: Fri, 22 Apr 2022 15:36:52 +0200	[thread overview]
Message-ID: <yt9dilr1kznv.fsf@linux.ibm.com> (raw)
In-Reply-To: <CAHk-=wgv4NLtEowsmX+0jq_nBWXFp8jtruX6U3SDm52N=ftkgg@mail.gmail.com> (Linus Torvalds's message of "Thu, 21 Apr 2022 09:10:33 -0700")

Linus Torvalds <torvalds@linux-foundation.org> writes:

> 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".

Yes.

> 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.

Looks like gcc also catches that, i get the same warning.

> 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.

That's what i'll do for now. I'll complain in the bugtracker entry
mentioned above, but for now it is propably easier to just disable
the warning until we have a fix.

Thanks
Sven