mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Linus Torvalds' <torvalds@linux-foundation.org>,
	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"
	<linux-hardening@vger.kernel.org>,
	"krebbel@linux.ibm.com" <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 08:24:37 +0000	[thread overview]
Message-ID: <fd6b4de14d944680b5a5674edfe34654@AcuMS.aculab.com> (raw)
In-Reply-To: <CAHk-=wgv4NLtEowsmX+0jq_nBWXFp8jtruX6U3SDm52N=ftkgg@mail.gmail.com>

From: Linus Torvalds
> Sent: 21 April 2022 17:11
> 
> 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.

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)

  reply	other threads:[~2022-04-22  8:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-21 14:02 Sven Schnelle
2022-04-21 16:10 ` Linus Torvalds
2022-04-22  8:24   ` David Laight [this message]
2022-04-22 13:36   ` Sven Schnelle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fd6b4de14d944680b5a5674edfe34654@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=hca@linux.ibm.com \
    --cc=iii@linux.ibm.com \
    --cc=keescook@chromium.org \
    --cc=krebbel@linux.ibm.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=svens@linux.ibm.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®