linux-hardening - RE: -Warray-bounds fun again

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 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:   Fri, 22 Apr 2022 08:24:37 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Linus Torvalds' <torvalds@...ux-foundation.org>,
        Sven Schnelle <svens@...ux.ibm.com>
CC:     Kees Cook <keescook@...omium.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>,
        "krebbel@...ux.ibm.com" <krebbel@...ux.ibm.com>,
        Ilya Leoshkevich <iii@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>
Subject: RE: -Warray-bounds fun again

From: Linus Torvalds
> Sent: 21 April 2022 17:11
> 
> On Thu, Apr 21, 2022 at 7:02 AM Sven Schnelle <svens@...ux.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.

I think you might be mixing up two problems.

Accessing ((foo *)0)->member is problematic because NULL might not be zero.
In which case an unexpected address is generated.
I think this is why clang really doesn't like you doing that.
Using ((foo *)(sizeof (foo))[-1].member might get around that.

I suspect the array bounds issue is caused by gcc using a size of 0
for 'I don't know the size' and then assuming it is real size later on.
That seems like a real gcc bug.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)