linux-kernel - Re: [PATCH v5] gpio: dwapb: mask/unmask IRQ when disable/enale it

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] [day] [month] [year] [list]

Message-ID: <CAMRc=Md7ODHUSe2fa37NazgJoV5aGXRh1Pp24LcFc49=UXgnwA@mail.gmail.com>
Date: Thu, 21 Dec 2023 11:19:55 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: xiongxin <xiongxin@...inos.cn>
Cc: fancer.lancer@...il.com, hoan@...amperecomputing.com, 
	linus.walleij@...aro.org, andy@...nel.org, linux-gpio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, stable@...nel.org, 
	Riwen Lu <luriwen@...inos.cn>
Subject: Re: [PATCH v5] gpio: dwapb: mask/unmask IRQ when disable/enale it

On Wed, Dec 20, 2023 at 3:29 AM xiongxin <xiongxin@...inos.cn> wrote:
>
> In the hardware implementation of the I2C HID driver based on DesignWare
> GPIO IRQ chip, when the user continues to use the I2C HID device in the
> suspend process, the I2C HID interrupt will be masked after the resume
> process is finished.
>
> This is because the disable_irq()/enable_irq() of the DesignWare GPIO
> driver does not synchronize the IRQ mask register state. In normal use
> of the I2C HID procedure, the GPIO IRQ irq_mask()/irq_unmask() functions
> are called in pairs. In case of an exception, i2c_hid_core_suspend()
> calls disable_irq() to disable the GPIO IRQ. With low probability, this
> causes irq_unmask() to not be called, which causes the GPIO IRQ to be
> masked and not unmasked in enable_irq(), raising an exception.
>
> Add synchronization to the masked register state in the
> dwapb_irq_enable()/dwapb_irq_disable() function. mask the GPIO IRQ
> before disabling it. After enabling the GPIO IRQ, unmask the IRQ.
>
> Fixes: 7779b3455697 ("gpio: add a driver for the Synopsys DesignWare APB GPIO block")
> Cc: stable@...nel.org
> Co-developed-by: Riwen Lu <luriwen@...inos.cn>
> Signed-off-by: Riwen Lu <luriwen@...inos.cn>
> Signed-off-by: xiongxin <xiongxin@...inos.cn>
> Acked-by: Serge Semin <fancer.lancer@...il.com>
> Reviewed-by: Andy Shevchenko <andy@...nel.org>
> ---
> v5:
>         * fix typo in patch description
> v4:
>         * Add patch tag information
> v3:
>         * Modify the submitter's information
> v2:
>         * Resubmit the patch to fix this exception from the DesignWare
>           GPIO driver side
> v1:
>         * Resolve the exception from the IRQ core layer. (key point not
>           found correctly)
> ---
>  drivers/gpio/gpio-dwapb.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 4a4f61bf6c58..8c59332429c2 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -282,13 +282,15 @@ static void dwapb_irq_enable(struct irq_data *d)
>  {
>         struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
>         struct dwapb_gpio *gpio = to_dwapb_gpio(gc);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(d);
>         unsigned long flags;
>         u32 val;
>
>         raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
> -       val = dwapb_read(gpio, GPIO_INTEN);
> -       val |= BIT(irqd_to_hwirq(d));
> +       val = dwapb_read(gpio, GPIO_INTEN) | BIT(hwirq);
>         dwapb_write(gpio, GPIO_INTEN, val);
> +       val = dwapb_read(gpio, GPIO_INTMASK) & ~BIT(hwirq);
> +       dwapb_write(gpio, GPIO_INTMASK, val);
>         raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>  }
>
> @@ -296,12 +298,14 @@ static void dwapb_irq_disable(struct irq_data *d)
>  {
>         struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
>         struct dwapb_gpio *gpio = to_dwapb_gpio(gc);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(d);
>         unsigned long flags;
>         u32 val;
>
>         raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
> -       val = dwapb_read(gpio, GPIO_INTEN);
> -       val &= ~BIT(irqd_to_hwirq(d));
> +       val = dwapb_read(gpio, GPIO_INTMASK) | BIT(hwirq);
> +       dwapb_write(gpio, GPIO_INTMASK, val);
> +       val = dwapb_read(gpio, GPIO_INTEN) & ~BIT(hwirq);
>         dwapb_write(gpio, GPIO_INTEN, val);
>         raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>  }
> --
> 2.34.1
>

Queued for fixes, thanks!

Bartosz