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);
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);
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
[ 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.
linux-stable-mirror@lists.linaro.org