linux-kernel - Re: [PATCH] gpio: siox: indicate exclusive support of threaded IRQs

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: <87eeojh5vh.fsf@nanos.tec.linutronix.de>
Date:   Thu, 06 Aug 2020 22:33:06 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Ahmad Fatoum <a.fatoum@...gutronix.de>,
        "open list\:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Thorsten Scherer <t.scherer@...elmann.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>
Subject: Re: [PATCH] gpio: siox: indicate exclusive support of threaded IRQs

Uwe Kleine-König <u.kleine-koenig@...gutronix.de> writes:
> On Thu, Aug 06, 2020 at 08:50:45PM +0200, Thomas Gleixner wrote:
>> handle_nested_irq() does not care. It cares about thread context,
>> external reentrancy protection for the same nested interrupt and that
>> the nested interrupt has a thread handler.
>> 
>> The latter is what goes belly up because w/o that threaded bit set the
>> GPIO core fails to set nested thread. So if a consumer requests an
>> interrupt with request_any_context_irq() then that fails to select
>> thread mode which means the threaded handler is not set causing
>> handle_nested_irq() to fail.
>
> For a caller of request_threaded_irq() that passes a relevant hardirq
> handler the hardirq handler is never called but request_threaded_irq()
> doesn't fail. The handler is just replaced by irq_nested_primary_handler
> in __setup_irq(). Is that a bug? (I didn't test, just read the code, so I
> might have missed something.)

Depends on what the threaded handler expects what the primary handler
has done. It might just work or not :)

> Trying to be constructive, here is my suggested changelog:
>
> 	gpio: siox: explicitly only support threaded irqs
>
> 	The gpio-siox driver uses handle_nested_irq() to implement its
> 	interrupt support. This is only capable to handle threaded irq
> 	actions. For a hardirq action it triggers a NULL pointer oops.
> 	(It calls action->thread_fn which is NULL then.)
>
> 	So prevent registration of a hardirq action by setting
> 	gpio_irq_chip::threaded to true.
>
> Does this address all your concerns?

LGTM

> Is this bad enough to justify sending this patch to stable?

Yes, a Cc: stable and a Fixes: tag is justified.

Thanks,

        tglx