lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c75a89f8-9eb7-4300-979e-e11159dc6888@collabora.com>
Date: Thu, 9 Oct 2025 08:17:31 +0100
From: Martyn Welch <martyn.welch@...labora.com>
To: Linus Walleij <linus.walleij@...aro.org>,
 Francesco Lavra <flavra@...libre.com>,
 Hugo Villeneuve <hvilleneuve@...onoff.com>,
 Maria Garcia <mariagarcia7293@...il.com>,
 Sascha Hauer <s.hauer@...gutronix.de>,
 Emanuele Ghidoli <emanuele.ghidoli@...adex.com>,
 Potin Lai <potin.lai.pt@...il.com>,
 Mark Tomlinson <mark.tomlinson@...iedtelesis.co.nz>,
 Fabio Estevam <festevam@...x.de>, Ian Ray <ian.ray@...ealthcare.com>
Cc: Bartosz Golaszewski <brgl@...ev.pl>, linux-gpio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] gpio: pca953x: enable latch only on edge-triggered inputs

On 09/10/2025 07:03, Linus Walleij wrote:
> Hi Francesco,
> 
> thanks for your patch!
> 
> On Wed, Oct 8, 2025 at 12:43 PM Francesco Lavra <flavra@...libre.com> wrote:
> 
> 
>> The latched input feature of the pca953x GPIO controller is useful
>> when an input is configured to trigger interrupts on rising or
>> falling edges, because it allows retrieving which edge type caused
>> a given interrupt even if the pin state changes again before the
>> interrupt handler has a chance to run. But for level-triggered
>> interrupts, reading the latched input state can cause an active
>> interrupt condition to be missed, e.g. if an active-low signal (for
>> which an IRQ_TYPE_LEVEL_LOW interrupt has been configured) triggers
>> an interrupt when switching to the inactive state, but then becomes
>> active again before the interrupt handler has a chance to run: in
>> this case, if the interrupt handler reads the latched input state,
>> it will wrongly assume that the interrupt is not pending.
>> Fix the above issue by enabling the latch only on edge-triggered
>> inputs, instead of all interrupt-enabled inputs.
>>
>> Signed-off-by: Francesco Lavra <flavra@...libre.com>
>> ---
>>   drivers/gpio/gpio-pca953x.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
>> index e80a96f39788..e87ef2c3ff82 100644
>> --- a/drivers/gpio/gpio-pca953x.c
>> +++ b/drivers/gpio/gpio-pca953x.c
>> @@ -761,10 +761,13 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
>>          int level;
>>
>>          if (chip->driver_data & PCA_PCAL) {
>> +               DECLARE_BITMAP(latched_inputs, MAX_LINE);
>>                  guard(mutex)(&chip->i2c_lock);
>>
>> -               /* Enable latch on interrupt-enabled inputs */
>> -               pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
>> +               /* Enable latch on edge-triggered interrupt-enabled inputs */
>> +               bitmap_or(latched_inputs, chip->irq_trig_fall, chip->irq_trig_raise, gc->ngpio);
>> +               bitmap_and(latched_inputs, latched_inputs, chip->irq_mask, gc->ngpio);
>> +               pca953x_write_regs(chip, PCAL953X_IN_LATCH, latched_inputs);
> 
> This driver is used by a *lot* of systems and people.
> 
> It is maybe the most used GPIO driver in the kernel.
> 
> So I added a lot of affected developers to the To: line of
> the mail so we can get a wider review and testing.
> 

I don't have access to the relevant hardware to test this anymore and 
it's been a while since I thought much about edge vs. level triggered 
interrupts. But if the state of the interrupt is unilaterally returning 
to an inactive state, it sounds like that should be configured as an 
edge triggered interrupt, not a level triggered one...

Martyn

> Yours,
> Linus Walleij


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ