* [PATCH][5.15.y] gpio: pca953x: fix pca953x_irq_bus_sync_unlock race
@ 2024-12-13 10:31 guocai.he.cn
2024-12-13 10:45 ` Harshit Mogalapalli
2024-12-13 15:13 ` Sasha Levin
0 siblings, 2 replies; 4+ messages in thread
From: guocai.he.cn @ 2024-12-13 10:31 UTC (permalink / raw)
To: stable; +Cc: gregkh, ian.ray, bartosz.golaszewski
From: Ian Ray <ian.ray@gehealthcare.com>
[ Upstream commit bfc6444b57dc7186b6acc964705d7516cbaf3904 ]
Ensure that `i2c_lock' is held when setting interrupt latch and mask in
pca953x_irq_bus_sync_unlock() in order to avoid races.
The other (non-probe) call site pca953x_gpio_set_multiple() ensures the
lock is held before calling pca953x_write_regs().
The problem occurred when a request raced against irq_bus_sync_unlock()
approximately once per thousand reboots on an i.MX8MP based system.
* Normal case
0-0022: write register AI|3a {03,02,00,00,01} Input latch P0
0-0022: write register AI|49 {fc,fd,ff,ff,fe} Interrupt mask P0
0-0022: write register AI|08 {ff,00,00,00,00} Output P3
0-0022: write register AI|12 {fc,00,00,00,00} Config P3
* Race case
0-0022: write register AI|08 {ff,00,00,00,00} Output P3
0-0022: write register AI|08 {03,02,00,00,01} *** Wrong register ***
0-0022: write register AI|12 {fc,00,00,00,00} Config P3
0-0022: write register AI|49 {fc,fd,ff,ff,fe} Interrupt mask P0
Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
Link: https://lore.kernel.org/r/20240620042915.2173-1-ian.ray@gehealthcare.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Guocai He <guocai.he.cn@windriver.com>
---
This commit is to solve the CVE-2024-42253. Please merge this commit to linux-5.15.y.
drivers/gpio/gpio-pca953x.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 4860bf3b7e00..4e97b6ae4f72 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -672,6 +672,8 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
int level;
if (chip->driver_data & PCA_PCAL) {
+ guard(mutex)(&chip->i2c_lock);
+
/* Enable latch on interrupt-enabled inputs */
pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH][5.15.y] gpio: pca953x: fix pca953x_irq_bus_sync_unlock race
2024-12-13 10:31 [PATCH][5.15.y] gpio: pca953x: fix pca953x_irq_bus_sync_unlock race guocai.he.cn
@ 2024-12-13 10:45 ` Harshit Mogalapalli
2024-12-13 11:08 ` Greg KH
2024-12-13 15:13 ` Sasha Levin
1 sibling, 1 reply; 4+ messages in thread
From: Harshit Mogalapalli @ 2024-12-13 10:45 UTC (permalink / raw)
To: guocai.he.cn, stable; +Cc: gregkh, ian.ray, bartosz.golaszewski
Hi Guocai,
On 13/12/24 16:01, guocai.he.cn@windriver.com wrote:
> From: Ian Ray <ian.ray@gehealthcare.com>
>
> [ Upstream commit bfc6444b57dc7186b6acc964705d7516cbaf3904 ]
>
> Ensure that `i2c_lock' is held when setting interrupt latch and mask in
> pca953x_irq_bus_sync_unlock() in order to avoid races.
>
> The other (non-probe) call site pca953x_gpio_set_multiple() ensures the
> lock is held before calling pca953x_write_regs().
>
> The problem occurred when a request raced against irq_bus_sync_unlock()
> approximately once per thousand reboots on an i.MX8MP based system.
>
> * Normal case
>
> 0-0022: write register AI|3a {03,02,00,00,01} Input latch P0
> 0-0022: write register AI|49 {fc,fd,ff,ff,fe} Interrupt mask P0
> 0-0022: write register AI|08 {ff,00,00,00,00} Output P3
> 0-0022: write register AI|12 {fc,00,00,00,00} Config P3
>
> * Race case
>
> 0-0022: write register AI|08 {ff,00,00,00,00} Output P3
> 0-0022: write register AI|08 {03,02,00,00,01} *** Wrong register ***
> 0-0022: write register AI|12 {fc,00,00,00,00} Config P3
> 0-0022: write register AI|49 {fc,fd,ff,ff,fe} Interrupt mask P0
>
> Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> Link: https://lore.kernel.org/r/20240620042915.2173-1-ian.ray@gehealthcare.com
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Guocai He <guocai.he.cn@windriver.com>
> ---
> This commit is to solve the CVE-2024-42253. Please merge this commit to linux-5.15.y.
>
> drivers/gpio/gpio-pca953x.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 4860bf3b7e00..4e97b6ae4f72 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -672,6 +672,8 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
> int level;
>
> if (chip->driver_data & PCA_PCAL) {
> + guard(mutex)(&chip->i2c_lock);
This wouldn't compile on 5.15.y
We don't have scope based locking backported to 5.15.y.
drivers/gpio/gpio-pca953x.c: In function ‘pca953x_irq_bus_sync_unlock’:
drivers/gpio/gpio-pca953x.c:675:17: error: implicit declaration of
function ‘guard’ [-Werror=implicit-function-declaration]
675 | guard(mutex)(&chip->i2c_lock);
| ^~~~~
drivers/gpio/gpio-pca953x.c:675:23: error: ‘mutex’ undeclared (first use
in this function)
675 | guard(mutex)(&chip->i2c_lock);
| ^~~~~
drivers/gpio/gpio-pca953x.c:675:23: note: each undeclared identifier is
reported only once for each function it appears in
cc1: all warnings being treated as errors
Thanks,
Harshit
> +
> /* Enable latch on interrupt-enabled inputs */
> pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][5.15.y] gpio: pca953x: fix pca953x_irq_bus_sync_unlock race
2024-12-13 10:45 ` Harshit Mogalapalli
@ 2024-12-13 11:08 ` Greg KH
0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2024-12-13 11:08 UTC (permalink / raw)
To: Harshit Mogalapalli, guocai.he.cn, stable, ian.ray,
bartosz.golaszewski
On Fri, Dec 13, 2024 at 04:15:09PM +0530, Harshit Mogalapalli wrote:
> Hi Guocai,
>
> On 13/12/24 16:01, guocai.he.cn@windriver.com wrote:
> > From: Ian Ray <ian.ray@gehealthcare.com>
> >
> > [ Upstream commit bfc6444b57dc7186b6acc964705d7516cbaf3904 ]
> >
> > Ensure that `i2c_lock' is held when setting interrupt latch and mask in
> > pca953x_irq_bus_sync_unlock() in order to avoid races.
> >
> > The other (non-probe) call site pca953x_gpio_set_multiple() ensures the
> > lock is held before calling pca953x_write_regs().
> >
> > The problem occurred when a request raced against irq_bus_sync_unlock()
> > approximately once per thousand reboots on an i.MX8MP based system.
> >
> > * Normal case
> >
> > 0-0022: write register AI|3a {03,02,00,00,01} Input latch P0
> > 0-0022: write register AI|49 {fc,fd,ff,ff,fe} Interrupt mask P0
> > 0-0022: write register AI|08 {ff,00,00,00,00} Output P3
> > 0-0022: write register AI|12 {fc,00,00,00,00} Config P3
> >
> > * Race case
> >
> > 0-0022: write register AI|08 {ff,00,00,00,00} Output P3
> > 0-0022: write register AI|08 {03,02,00,00,01} *** Wrong register ***
> > 0-0022: write register AI|12 {fc,00,00,00,00} Config P3
> > 0-0022: write register AI|49 {fc,fd,ff,ff,fe} Interrupt mask P0
> >
> > Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> > Link: https://lore.kernel.org/r/20240620042915.2173-1-ian.ray@gehealthcare.com
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > Signed-off-by: Guocai He <guocai.he.cn@windriver.com>
> > ---
> > This commit is to solve the CVE-2024-42253. Please merge this commit to linux-5.15.y.
> >
> > drivers/gpio/gpio-pca953x.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> > index 4860bf3b7e00..4e97b6ae4f72 100644
> > --- a/drivers/gpio/gpio-pca953x.c
> > +++ b/drivers/gpio/gpio-pca953x.c
> > @@ -672,6 +672,8 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
> > int level;
> > if (chip->driver_data & PCA_PCAL) {
> > + guard(mutex)(&chip->i2c_lock);
>
> This wouldn't compile on 5.15.y
Which means that no one is actually testing these backports.
Ok, I'm frustrated enough. No more windriver backports for stable trees
will now be accepted until you all get your act together and figure out
how to do this properly.
As to "how" you prove that you all know what you are doing, I will
leave that up to you to come up with a proper proposal and proof.
ugh.
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][5.15.y] gpio: pca953x: fix pca953x_irq_bus_sync_unlock race
2024-12-13 10:31 [PATCH][5.15.y] gpio: pca953x: fix pca953x_irq_bus_sync_unlock race guocai.he.cn
2024-12-13 10:45 ` Harshit Mogalapalli
@ 2024-12-13 15:13 ` Sasha Levin
1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-12-13 15:13 UTC (permalink / raw)
To: stable; +Cc: guocai.he.cn, Sasha Levin
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: bfc6444b57dc7186b6acc964705d7516cbaf3904
WARNING: Author mismatch between patch and upstream commit:
Backport author: guocai.he.cn@windriver.com
Commit author: Ian Ray <ian.ray@gehealthcare.com>
Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: e2ecdddca80d)
6.1.y | Present (different SHA1: 58a5c93bd1a6)
5.15.y | Not found
Note: The patch differs from the upstream commit:
---
1: bfc6444b57dc7 ! 1: 3812c0bc93e5e gpio: pca953x: fix pca953x_irq_bus_sync_unlock race
@@ Metadata
## Commit message ##
gpio: pca953x: fix pca953x_irq_bus_sync_unlock race
+ [ Upstream commit bfc6444b57dc7186b6acc964705d7516cbaf3904 ]
+
Ensure that `i2c_lock' is held when setting interrupt latch and mask in
pca953x_irq_bus_sync_unlock() in order to avoid races.
@@ Commit message
Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
Link: https://lore.kernel.org/r/20240620042915.2173-1-ian.ray@gehealthcare.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
+ Signed-off-by: Guocai He <guocai.he.cn@windriver.com>
## drivers/gpio/gpio-pca953x.c ##
@@ drivers/gpio/gpio-pca953x.c: static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y | Success | Failed |
Build Errors:
Build error for stable/linux-5.15.y:
drivers/gpio/gpio-pca953x.c: In function 'pca953x_irq_bus_sync_unlock':
drivers/gpio/gpio-pca953x.c:675:17: error: implicit declaration of function 'guard' [-Werror=implicit-function-declaration]
675 | guard(mutex)(&chip->i2c_lock);
| ^~~~~
drivers/gpio/gpio-pca953x.c:675:23: error: 'mutex' undeclared (first use in this function)
675 | guard(mutex)(&chip->i2c_lock);
| ^~~~~
drivers/gpio/gpio-pca953x.c:675:23: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:289: drivers/gpio/gpio-pca953x.o] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [scripts/Makefile.build:552: drivers/gpio] Error 2
make[1]: Target '__build' not remade because of errors.
make: *** [Makefile:1906: drivers] Error 2
make: Target '__all' not remade because of errors.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-12-13 15:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-13 10:31 [PATCH][5.15.y] gpio: pca953x: fix pca953x_irq_bus_sync_unlock race guocai.he.cn
2024-12-13 10:45 ` Harshit Mogalapalli
2024-12-13 11:08 ` Greg KH
2024-12-13 15:13 ` Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox