linux-kernel - Re: [PATCH v2 1/3] spi: spi-fsl-dspi: Fix external abort on interrupt in exit paths

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, 15 Jun 2020 16:57:11 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Mark Brown <broonie@...nel.org>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vladimir Oltean <vladimir.oltean@....com>,
        linux-spi <linux-spi@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Wolfram Sang <wsa@...nel.org>, stable@...r.kernel.org,
        Pengutronix Kernel Team <kernel@...gutronix.de>
Subject: Re: [PATCH v2 1/3] spi: spi-fsl-dspi: Fix external abort on
 interrupt in exit paths

On Mon, Jun 15, 2020 at 05:23:28PM +0300, Vladimir Oltean wrote:
> On Mon, 15 Jun 2020 at 16:41, Krzysztof Kozlowski <krzk@...nel.org> wrote:
> >
> > On Mon, Jun 15, 2020 at 04:12:28PM +0300, Vladimir Oltean wrote:
> > > On Mon, 15 Jun 2020 at 16:10, Mark Brown <broonie@...nel.org> wrote:
> > >
> > > >
> > > > It's a bit unusual to need to actually free the IRQ over suspend -
> > > > what's driving that requirement here?
> > >
> > > clk_disable_unprepare(dspi->clk); is driving the requirement - same as
> > > in dspi_remove case, the module will fault when its registers are
> > > accessed without a clock.
> >
> > In few cases when I have shared interrupt in different drivers, they
> > were just disabling it during suspend. Why it has to be freed?
> >
> > Best regards,
> > Krzysztof
> >
> 
> Not saying it _has_ to be freed, just to be prevented from running
> concurrently with us disabling the clock.
> But if we can get away in dspi_suspend with just disable_irq, can't we
> also get away in dspi_remove with just devm_free_irq?

One reason why they have to be different could be following scenario:
1. Device could be unbound any time and disabling IRQ in remove() would
   effectively disable the IRQ also for other devices using this shared
   line. First disable_irq() really disables it, the latter just
   increases the counter.
2. However during system suspend, it is expected that all drivers in
   their suspend (and later resume) callbacks will do the same - disable
   the shared IRQ line. And finally the system disables interrupts
   globally so the line will be balanced.

Freeing IRQ solves the case #1 without causing any imbalance between
enables/disables or requests/frees.  Disabling IRQ solves the #2, also
without any imbalance.

Best regards,
Krzysztof