* [GIT PULL] gpio: fixes for v5.18-rc2
@ 2022-04-09 20:51 Bartosz Golaszewski
2022-04-10 4:26 ` Linus Torvalds
2022-04-10 4:53 ` pr-tracker-bot
0 siblings, 2 replies; 4+ messages in thread
From: Bartosz Golaszewski @ 2022-04-09 20:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Shevchenko, Linus Walleij, linux-gpio, linux-kernel,
Bartosz Golaszewski
Linus,
Here's a single fix for a race condition between the GPIO core and consumers of
GPIO IRQ chips.
Please pull,
Bartosz Golaszewski
The following changes since commit 3123109284176b1532874591f7c81f3837bbdc17:
Linux 5.18-rc1 (2022-04-03 14:08:21 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/gpio-fixes-for-v5.18-rc2
for you to fetch changes up to 5467801f1fcbdc46bc7298a84dbf3ca1ff2a7320:
gpio: Restrict usage of GPIO chip irq members before initialization (2022-04-04 14:41:34 +0200)
----------------------------------------------------------------
gpio fixes for v5.18-rc2
- fix a race condition with consumers accessing the fields of GPIO IRQ chips
before they're fully initialized
----------------------------------------------------------------
Shreeya Patel (1):
gpio: Restrict usage of GPIO chip irq members before initialization
drivers/gpio/gpiolib.c | 19 +++++++++++++++++++
include/linux/gpio/driver.h | 9 +++++++++
2 files changed, 28 insertions(+)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [GIT PULL] gpio: fixes for v5.18-rc2
2022-04-09 20:51 [GIT PULL] gpio: fixes for v5.18-rc2 Bartosz Golaszewski
@ 2022-04-10 4:26 ` Linus Torvalds
2022-04-11 10:49 ` Bartosz Golaszewski
2022-04-10 4:53 ` pr-tracker-bot
1 sibling, 1 reply; 4+ messages in thread
From: Linus Torvalds @ 2022-04-10 4:26 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andy Shevchenko, Linus Walleij, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List
On Sat, Apr 9, 2022 at 10:51 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> Here's a single fix for a race condition between the GPIO core and consumers of
> GPIO IRQ chips.
I've pulled this, but it's horribly broken.
You can't just use a compiler barrier to make sure the compiler orders
the data at initialization time.
That doesn't take care of CPU re-ordering, but it also doesn't take
care of re-ordering reads on the other side of the equation.
Every write barrier needs to pair with a read barrier.
And "barrier()" is only a barrier on that CPU, since it is only a
barrier for code generation, not for data.
There are multiple ways to do proper hand-off of data, but the best
one is likely
- on the initialization side, do
.. initialize all the data, then do ..
smp_store_release(&initialized, 1);
- on the reading side, do
if (!smp_load_acquire(&initialized))
return -EAGAIN;
.. you can now rely on all the data having been initialized ..
But honestly, the fact that you got this race condition so wrong makes
me suggest you use proper locks. Because the above gives you proper
ordering between the two sequences, but the sequences in question
still have to have a *lot* of guarantees about the accesses actually
then being valid in a lock-free environment (the only obviously safe
situation is a "initialize things once, everything afterwards is only
a read" - otherwise y ou need to make sure all the *updates* are
safely done too).
With locking, all these issues go away. The lock will take care of
ordering, but also data consistency at updates.
Without locking, you need to do the above kinds of careful things for
_all_ the accesses that can race, not just that "initialized" flag.
Linus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [GIT PULL] gpio: fixes for v5.18-rc2
2022-04-09 20:51 [GIT PULL] gpio: fixes for v5.18-rc2 Bartosz Golaszewski
2022-04-10 4:26 ` Linus Torvalds
@ 2022-04-10 4:53 ` pr-tracker-bot
1 sibling, 0 replies; 4+ messages in thread
From: pr-tracker-bot @ 2022-04-10 4:53 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Torvalds, Andy Shevchenko, Linus Walleij, linux-gpio,
linux-kernel, Bartosz Golaszewski
The pull request you sent on Sat, 9 Apr 2022 22:51:34 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/gpio-fixes-for-v5.18-rc2
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/fa3b895da8e06d6e3dcf3e6941a3fd428343e3d7
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [GIT PULL] gpio: fixes for v5.18-rc2
2022-04-10 4:26 ` Linus Torvalds
@ 2022-04-11 10:49 ` Bartosz Golaszewski
0 siblings, 0 replies; 4+ messages in thread
From: Bartosz Golaszewski @ 2022-04-11 10:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Shevchenko, Linus Walleij, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, Shreeya Patel
On Sun, Apr 10, 2022 at 6:27 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sat, Apr 9, 2022 at 10:51 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > Here's a single fix for a race condition between the GPIO core and consumers of
> > GPIO IRQ chips.
>
> I've pulled this, but it's horribly broken.
>
> You can't just use a compiler barrier to make sure the compiler orders
> the data at initialization time.
>
> That doesn't take care of CPU re-ordering, but it also doesn't take
> care of re-ordering reads on the other side of the equation.
>
> Every write barrier needs to pair with a read barrier.
>
> And "barrier()" is only a barrier on that CPU, since it is only a
> barrier for code generation, not for data.
>
> There are multiple ways to do proper hand-off of data, but the best
> one is likely
>
> - on the initialization side, do
>
> .. initialize all the data, then do ..
> smp_store_release(&initialized, 1);
>
> - on the reading side, do
>
> if (!smp_load_acquire(&initialized))
> return -EAGAIN;
>
> .. you can now rely on all the data having been initialized ..
>
> But honestly, the fact that you got this race condition so wrong makes
> me suggest you use proper locks. Because the above gives you proper
> ordering between the two sequences, but the sequences in question
> still have to have a *lot* of guarantees about the accesses actually
> then being valid in a lock-free environment (the only obviously safe
> situation is a "initialize things once, everything afterwards is only
> a read" - otherwise y ou need to make sure all the *updates* are
> safely done too).
>
> With locking, all these issues go away. The lock will take care of
> ordering, but also data consistency at updates.
>
> Without locking, you need to do the above kinds of careful things for
> _all_ the accesses that can race, not just that "initialized" flag.
>
> Linus
Cc'ing Shreeya
Thanks, we'll see about a follow-up with a better solution.
Bart
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-04-11 10:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-09 20:51 [GIT PULL] gpio: fixes for v5.18-rc2 Bartosz Golaszewski
2022-04-10 4:26 ` Linus Torvalds
2022-04-11 10:49 ` Bartosz Golaszewski
2022-04-10 4:53 ` pr-tracker-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).