linux-kernel - Re: [PATCH v3] genirq: Check irq disabled & masked states in irq

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: <alpine.DEB.2.20.1705271237280.2329@nanos>
Date:   Sat, 27 May 2017 13:12:38 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Jeffy Chen <jeffy.chen@...k-chips.com>
cc:     linux-kernel@...r.kernel.org, briannorris@...omium.org,
        dianders@...omium.org, tfiga@...omium.org
Subject: Re: [PATCH v3] genirq: Check irq disabled & masked states in
 irq_shutdown

On Sat, 27 May 2017, Jeffy Chen wrote:

> If a irq is already disabled, irq_shutdown may try to disable it again,
> for example:
> devm_request_irq->irq_startup->irq_enable
> disable_irq					<-- disabled
> devm_free_irq->irq_shutdown			<-- disable it again
> 
> This would confuse some chips which require balanced irq enable and
> disable, for example pinctrl-rockchip & pinctrl-nomadik.

"would confuse" is an interesting technical term. Can you please start to
explain things in coherent sentences so people who do not have detailed
knowledge of the problem at hand can understand it?

> Add a state check before calling irq_disable to prevent that.

So you analyzed in half an hour that all of this is correct and fixes all
possible imbalances, right?  Really impressive!

You just forgot to mention this in the changelog.

> v2: Rewrite commit message.
> v3: Rewrite commit message and not skip irq_shutdown.

This does not belong into the changelog and having that information twice
is more than pointless.

Before you send yet another version of this within 5 minutes, can you
please sit down and analyze all the possible imbalance scenarios?

I'm not going to apply hastiliy cobbled together workarounds which "fix"
just half of the problem space. This is not random driver code which breaks
ONE type of machine. This is core code which has the potential to break ALL
machines out there in one go.

Either you come up with a properly analyzed solution which addresses all
possible imbalanced invocations or you have to wait until I find some time
to look at it myself.

Thanks,

	tglx