All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	dja@axtens.net,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linuxppc-dev@lists.ozlabs.org,
	Christophe Leroy <christophe.leroy@c-s.fr>,
	linux-arch <linux-arch@vger.kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Segher Boessenkool <segher@kernel.crashing.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Christian Borntraeger <borntraeger@de.ibm.com>
Subject: Re: READ_ONCE() + STACKPROTECTOR_STRONG == :/ (was Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops))
Date: Tue, 17 Dec 2019 17:07:19 +0000	[thread overview]
Message-ID: <20191217170719.GA869@willie-the-truck> (raw)
In-Reply-To: <CAHk-=wiMuHmWzQ7-CRQB6o+SHtA-u-Rp6VZwPcqDbjAaug80rQ@mail.gmail.com>

On Thu, Dec 12, 2019 at 12:49:52PM -0800, Linus Torvalds wrote:
> On Thu, Dec 12, 2019 at 11:34 AM Will Deacon <will@kernel.org> wrote:
> >
> > The root of my concern in all of this, and what started me looking at it in
> > the first place, is the interaction with 'typeof()'. Inheriting 'volatile'
> > for a pointer means that local variables in macros declared using typeof()
> > suddenly start generating *hideous* code, particularly when pointless stack
> > spills get stackprotector all excited.
> 
> Yeah, removing volatile can be a bit annoying.
> 
> For the particular case of the bitops, though, it's not an issue.
> Since you know the type there, you can just cast it.
> 
> And if we had the rule that READ_ONCE() was an arithmetic type, you could do
> 
>     typeof(0+(*p)) __var;
> 
> since you might as well get the integer promotion anyway (on the
> non-volatile result).
> 
> But that doesn't work with structures or unions, of course.
> 
> I'm not entirely sure we have READ_ONCE() with a struct. I do know we
> have it with 64-bit entities on 32-bit machines, but that's ok with
> the "0+" trick.

Other than the two trivial examples Arnd and I spotted, it looks like
we're in for some fun with the page-table types such as pud_t but that
/should/ be fixable with enough effort.

However, I'm really banging my head against the compiler trying to get
your trick above to work for pointer types when the pointed-to-type is
not defined. As a very cut down example (I pulled this back out of the
preprocessor and cleaned it up a bit):


struct dentry {
	struct inode *d_inode;
};

static inline struct inode *d_inode_rcu(struct dentry *dentry)
{
	return ({
		typeof(0 + dentry->d_inode) __x = (*(volatile typeof(dentry->d_inode) *)&(dentry->d_inode));
		(typeof(dentry->d_inode))__x;
	});
}


Trying to compile this results in:

  | In function 'd_inode_rcu':
  | error: invalid use of undefined type 'struct inode'

whereas it compiles fine if you remove the '0 +' from the first typeof.

What am I missing? Perhaps the compiler wants the size information of
'struct inode' before it will contemplate the arithmetic, but if so then
I don't think we can use this trick after all. Hmm.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-arch <linux-arch@vger.kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	linuxppc-dev@lists.ozlabs.org, dja@axtens.net
Subject: Re: READ_ONCE() + STACKPROTECTOR_STRONG == :/ (was Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops))
Date: Tue, 17 Dec 2019 17:07:19 +0000	[thread overview]
Message-ID: <20191217170719.GA869@willie-the-truck> (raw)
In-Reply-To: <CAHk-=wiMuHmWzQ7-CRQB6o+SHtA-u-Rp6VZwPcqDbjAaug80rQ@mail.gmail.com>

On Thu, Dec 12, 2019 at 12:49:52PM -0800, Linus Torvalds wrote:
> On Thu, Dec 12, 2019 at 11:34 AM Will Deacon <will@kernel.org> wrote:
> >
> > The root of my concern in all of this, and what started me looking at it in
> > the first place, is the interaction with 'typeof()'. Inheriting 'volatile'
> > for a pointer means that local variables in macros declared using typeof()
> > suddenly start generating *hideous* code, particularly when pointless stack
> > spills get stackprotector all excited.
> 
> Yeah, removing volatile can be a bit annoying.
> 
> For the particular case of the bitops, though, it's not an issue.
> Since you know the type there, you can just cast it.
> 
> And if we had the rule that READ_ONCE() was an arithmetic type, you could do
> 
>     typeof(0+(*p)) __var;
> 
> since you might as well get the integer promotion anyway (on the
> non-volatile result).
> 
> But that doesn't work with structures or unions, of course.
> 
> I'm not entirely sure we have READ_ONCE() with a struct. I do know we
> have it with 64-bit entities on 32-bit machines, but that's ok with
> the "0+" trick.

Other than the two trivial examples Arnd and I spotted, it looks like
we're in for some fun with the page-table types such as pud_t but that
/should/ be fixable with enough effort.

However, I'm really banging my head against the compiler trying to get
your trick above to work for pointer types when the pointed-to-type is
not defined. As a very cut down example (I pulled this back out of the
preprocessor and cleaned it up a bit):


struct dentry {
	struct inode *d_inode;
};

static inline struct inode *d_inode_rcu(struct dentry *dentry)
{
	return ({
		typeof(0 + dentry->d_inode) __x = (*(volatile typeof(dentry->d_inode) *)&(dentry->d_inode));
		(typeof(dentry->d_inode))__x;
	});
}


Trying to compile this results in:

  | In function 'd_inode_rcu':
  | error: invalid use of undefined type 'struct inode'

whereas it compiles fine if you remove the '0 +' from the first typeof.

What am I missing? Perhaps the compiler wants the size information of
'struct inode' before it will contemplate the arithmetic, but if so then
I don't think we can use this trick after all. Hmm.

Will

  parent reply	other threads:[~2019-12-17 17:07 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-06 12:46 [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops) Michael Ellerman
2019-12-06 12:46 ` Michael Ellerman
2019-12-06 13:16 ` Peter Zijlstra
2019-12-06 13:16   ` Peter Zijlstra
2019-12-10  5:38   ` Michael Ellerman
2019-12-10  5:38     ` Michael Ellerman
2019-12-10 10:15     ` Peter Zijlstra
2019-12-10 10:15       ` Peter Zijlstra
2019-12-11  0:29       ` Michael Ellerman
2019-12-11  0:29         ` Michael Ellerman
2019-12-12  5:42   ` READ_ONCE() + STACKPROTECTOR_STRONG == :/ (was Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops)) Michael Ellerman
2019-12-12  5:42     ` Michael Ellerman
2019-12-12  8:01     ` Peter Zijlstra
2019-12-12  8:01       ` Peter Zijlstra
2019-12-12 10:07       ` Will Deacon
2019-12-12 10:07         ` Will Deacon
2019-12-12 10:46         ` Peter Zijlstra
2019-12-12 10:46           ` Peter Zijlstra
2019-12-12 17:04           ` Will Deacon
2019-12-12 17:04             ` Will Deacon
2019-12-12 17:16             ` Will Deacon
2019-12-12 17:16               ` Will Deacon
2019-12-12 17:41           ` Linus Torvalds
2019-12-12 17:41             ` Linus Torvalds
2019-12-12 17:50             ` Segher Boessenkool
2019-12-12 17:50               ` Segher Boessenkool
2019-12-12 18:06             ` Will Deacon
2019-12-12 18:06               ` Will Deacon
2019-12-12 18:29               ` Christian Borntraeger
2019-12-12 18:29                 ` Christian Borntraeger
2019-12-12 18:43               ` Linus Torvalds
2019-12-12 18:43                 ` Linus Torvalds
2019-12-12 19:34                 ` Will Deacon
2019-12-12 19:34                   ` Will Deacon
2019-12-12 20:21                   ` Peter Zijlstra
2019-12-12 20:21                     ` Peter Zijlstra
2019-12-12 20:53                     ` Peter Zijlstra
2019-12-12 20:53                       ` Peter Zijlstra
2019-12-13 10:47                       ` Luc Van Oostenryck
2019-12-13 10:47                         ` Luc Van Oostenryck
2019-12-13 12:56                         ` Peter Zijlstra
2019-12-13 12:56                           ` Peter Zijlstra
2019-12-13 14:28                           ` Luc Van Oostenryck
2019-12-13 14:28                             ` Luc Van Oostenryck
2019-12-12 20:49                   ` Linus Torvalds
2019-12-12 20:49                     ` Linus Torvalds
2019-12-13 13:17                     ` Arnd Bergmann
2019-12-13 13:17                       ` Arnd Bergmann
2019-12-13 21:32                       ` Arnd Bergmann
2019-12-13 21:32                         ` Arnd Bergmann
2019-12-13 22:01                         ` Linus Torvalds
2019-12-13 22:01                           ` Linus Torvalds
2019-12-16 10:28                       ` Will Deacon
2019-12-16 10:28                         ` Will Deacon
2019-12-16 11:47                         ` Peter Zijlstra
2019-12-16 11:47                           ` Peter Zijlstra
2019-12-16 12:06                         ` Arnd Bergmann
2019-12-16 12:06                           ` Arnd Bergmann
2019-12-17 17:07                     ` Will Deacon [this message]
2019-12-17 17:07                       ` Will Deacon
2019-12-17 18:04                       ` Linus Torvalds
2019-12-17 18:04                         ` Linus Torvalds
2019-12-17 18:05                         ` Linus Torvalds
2019-12-17 18:05                           ` Linus Torvalds
2019-12-17 18:31                           ` Will Deacon
2019-12-17 18:31                             ` Will Deacon
2019-12-17 18:32                         ` Linus Torvalds
2019-12-17 18:32                           ` Linus Torvalds
2019-12-18 12:17                           ` Michael Ellerman
2019-12-18 12:17                             ` Michael Ellerman
2019-12-19 12:11                           ` Will Deacon
2019-12-19 12:11                             ` Will Deacon
2019-12-18 10:22                     ` Christian Borntraeger
2019-12-18 10:22                       ` Christian Borntraeger
2019-12-18 10:35                       ` Will Deacon
2019-12-18 10:35                         ` Will Deacon
2019-12-13 12:07           ` Michael Ellerman
2019-12-13 12:07             ` Michael Ellerman
2019-12-13 13:53             ` Segher Boessenkool
2019-12-13 13:53               ` Segher Boessenkool
2019-12-13 21:06               ` Michael Ellerman
2019-12-13 21:06                 ` Michael Ellerman
2019-12-12 15:10     ` Segher Boessenkool
2019-12-12 15:10       ` Segher Boessenkool
2019-12-06 22:15 ` [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops) pr-tracker-bot
2019-12-06 22:15   ` pr-tracker-bot

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=20191217170719.GA869@willie-the-truck \
    --to=will@kernel.org \
    --cc=arnd@arndb.de \
    --cc=borntraeger@de.ibm.com \
    --cc=christophe.leroy@c-s.fr \
    --cc=dja@axtens.net \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mark.rutland@arm.com \
    --cc=mpe@ellerman.id.au \
    --cc=peterz@infradead.org \
    --cc=segher@kernel.crashing.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.