linux-kernel - Re: [PATCH] genirq: Deduplicate WARN_ON_ONCE() in generic_handle_domain

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]

Date:   Mon, 16 May 2022 08:53:29 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Lukas Wunner <lukas@...ner.de>
Cc:     Marc Zyngier <maz@...nel.org>, Mark Rutland <mark.rutland@....com>,
        Jakub Kicinski <kuba@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        linux-gpio@...r.kernel.org,
        Octavian Purdila <octavian.purdila@....com>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] genirq: Deduplicate WARN_ON_ONCE() in
 generic_handle_domain_irq()

On Mon, May 16 2022 at 08:29, Sebastian Andrzej Siewior wrote:

> On 2022-05-10 09:56:05 [+0200], Lukas Wunner wrote:
>> An example for irqchips where the warning is false positive are
>> USB-attached GPIO controllers such as drivers/gpio/gpio-dln2.c:
>
> They are not false positives because…
>
>> USB gadgets are incapable of directly signaling an interrupt because
>> they cannot initiate a bus transaction by themselves.  All communication
>> on the bus is initiated by the host controller, which polls a gadget's
>> Interrupt Endpoint in regular intervals.  If an interrupt is pending,
>> that information is passed up the stack in softirq context, from which
>> a hardirq is synthesized via generic_handle_domain_irq().
>
> they tell you that the context is wrong.

Why? These handlers can be called from any context, really. Yes. They
need to be called with interrupts disabled, but that's it. The warning
is checking hard interrupt context unconditionally.

> From looking at gpio-dln2 this is called from USB URB's callback which
> is softirq. In the end dln2_gpio_event() is invoked while
> dln2_dev::event_cb_lock is acquired.  That lock is acquired by
> disabling interrupts which is what gets the locking right for
> generic_handle_domain_irq(). If that lock lifted to spin_lock_bh()
> (because it is always in urb's calback context and all HCDs complete
> in one context unlike now) then this breaks.

Yes, but that's a different problem.

> And PREEMPT_RT is broken already. Therefore, last week, I've been
> promoting generic_handle_domain_irq_safe()
> https://lkml.kernel.org/r/YnkfWFzvusFFktSt@linutronix.de

Well, that's just a wrapper which adds the local_irq_save(), so it's not
any different from having the local_irq_save() at the callsite, unless
I'm missing something.

Thanks,

        tglx