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:   Fri, 12 Jun 2020 12:39:49 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
Cc:     Wolfram Sang <wsa@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Oleksij Rempel <linux@...pel-privat.de>,
        NXP Linux Team <linux-imx@....com>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        linux-arm-kernel@...ts.infradead.org, linux-i2c@...r.kernel.org
Subject: Re: [PATCH] i2c: imx: Fix external abort on early interrupt

On Fri, Jun 12, 2020 at 12:31:49PM +0200, Oleksij Rempel wrote:
> On Fri, Jun 12, 2020 at 12:21:13PM +0200, Krzysztof Kozlowski wrote:
> > On Fri, Jun 12, 2020 at 11:56:04AM +0200, Wolfram Sang wrote:
> > > On Fri, Jun 12, 2020 at 11:29:41AM +0200, Krzysztof Kozlowski wrote:
> > > > On Fri, Jun 12, 2020 at 11:05:17AM +0200, Wolfram Sang wrote:
> > > > > On Wed, Jun 10, 2020 at 03:46:42PM +0200, Krzysztof Kozlowski wrote:
> > > > > > If interrupt comes early (could be triggered with CONFIG_DEBUG_SHIRQ),
> > > > > 
> > > > > That code is disabled since 2011 (6d83f94db95c ("genirq: Disable the
> > > > > SHIRQ_DEBUG call in request_threaded_irq for now"))? So, you had this
> > > > > without fake injection, I assume?
> > > > 
> > > > No, I observed it only after enabling DEBUG_SHIRQ (to a kernel with
> > > > some debugging options already).
> > > 
> > > Interesting. Maybe probe was deferred and you got the extra irq when
> > > deregistering?
> > 
> > Yes, good catch. The abort happens right after deferred probe exit.  It
> > could be then different reason than I thought - the interrupt is freed
> > through devm infrastructure quite late.  At this time, the clock might
> > be indeed disabled (error path of probe()).
> 
> This line looks suspicious to me:
>  Unhandled fault: external abort on non-linefetch (0x1008) at 0x8882d003
> 
> 0x8882d003 looks like not initialized pointer.
> The only not initialized value at devm_request_irq stage is i2c_imx->queue.

The queue should be good at this time because it is part of i2c_imx
which is allocated before interrupt (so freed after interrupt).

Like Wolfram suggested, the interrupt comes because of deferred probe.
The only solution would be to free the IRQ in error path... and in
driver remove.

This basically kills the concept of devm for interrupts. Some other
drivers experience exactly the same pattern. I now reproduced it on
unbind of dspi driver of VF5xx:

echo 4002d000.spi > /sys/devices/platform/soc/40000000.bus/4002d000.spi/driver/unbind
[  218.391867] Unhandled fault: external abort on non-linefetch (0x1008) at 0x8887f02c
...
[  218.754493] [<806185c4>] (regmap_mmio_read32le) from [<8061885c>] (regmap_mmio_read+0x48/0x68)
[  218.820049] [<80678c64>] (dspi_interrupt) from [<8017acec>] (free_irq+0x26c/0x3cc)
[  218.827853]  r5:86312200 r4:85a71d40
[  218.831602] [<8017aa80>] (free_irq) from [<8017dcec>] (devm_irq_release+0x1c/0x20)
[  218.839420]  r10:805f91fc r9:8630dac8 r8:8630dac8 r7:805f9214 r6:8630d810 r5:85a54780
[  218.847468]  r4:85a54800
[  218.850152] [<8017dcd0>] (devm_irq_release) from [<805f98ec>] (release_nodes+0x1e4/0x298)

Best regards,
Krzysztof

Powered by blists - more mailing lists