linux-kernel - Re: [PATCH] regmap-irq: Set IRQCHIP_MASK_ON_SUSPEND if no wake

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1C2DB0DE-016C-4F19-9D6F-5C8A0C003273@sifive.com>
Date:   Mon, 17 Jul 2023 14:54:18 -0500
From:   Samuel Holland <samuel.holland@...ive.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Laxman Dewangan <ldewangan@...dia.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] regmap-irq: Set IRQCHIP_MASK_ON_SUSPEND if no wake_base

On Jul 17, 2023, at 2:22 PM, Mark Brown <broonie@...nel.org> wrote:
> 
> On Mon, Jul 17, 2023 at 12:16:54PM -0700, Samuel Holland wrote:
> 
>> If hardware provides no way to control which IRQs are wakeup enabled,
>> then software needs to mask non-wakeup-enabled IRQs when going to sleep.
> 
> This isn't an unambigiously clear statement, especially for MFDs where
> there might be a desire to have some function on the device be able to
> wake the system (eg, headset button press on an audio CODEC or a RTC on
> a PMIC) even if there's no control within the device...
> 
>> Fixes: 55ac85e942c6 ("regmap: irq: enable wake support by default")
> 
> ...as the commit log does hint at.  If there's a problem I think we need
> some finer grained control here.

The current problem is that if wakeup is enabled for any IRQ in the chip
(say, the PMIC's power button), then we enable wakeup for the parent IRQ,
and now suddenly all (enabled) IRQs on the PMIC are also inadvertently
wakeup-enabled.

But I realize this patch does not actually solve the issue, for a couple
of reasons:
1) regmap-irq does not implement .irq_mask, just .irq_disable.
2) The __disable_irq() call in suspend_device_irq() should be sufficient,
   except that we fail the irq_settings_is_nested_thread() check in
   suspend_device_irqs().

So maybe the real issue is that commit 3c646f2c6aa9 ("genirq: Don't
suspend nested_thread irqs over system suspend") missed the case where
the child IRQ should be suspended, but the parent IRQ should not.

If that is fixed, then suspend_device_irq() should do the right thing
without any changes to the regmap-irq code.

Sorry for the noise.