linux-kernel - Re: [PATCH RESEND] serial: imx: Fix potential deadlock on sport->port.lock

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] [day] [month] [year] [list]

Message-ID: <20230928084436.fftw5sk4sixaedck@pengutronix.de>
Date:   Thu, 28 Sep 2023 10:44:36 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Vinod Koul <vkoul@...nel.org>
Cc:     festevam@...il.com, linux-serial@...r.kernel.org,
        gregkh@...uxfoundation.org, s.hauer@...gutronix.de,
        linux-kernel@...r.kernel.org, Chengfeng Ye <dg573847474@...il.com>,
        sorganov@...il.com, kernel@...gutronix.de,
        ilpo.jarvinen@...ux.intel.com, dmaengine@...r.kernel.org,
        shawnguo@...nel.org, jirislaby@...nel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH RESEND] serial: imx: Fix potential deadlock on
 sport->port.lock

Hello Vinod,

thanks for your quick answer!

On Thu, Sep 28, 2023 at 01:08:15PM +0530, Vinod Koul wrote:
> On 28-09-23, 08:07, Uwe Kleine-König wrote:
> > [Cc += Vinod Koul, dmaengine@...r.kernel.org]
> > 
> > Hello,
> > 
> > On Wed, Sep 27, 2023 at 06:19:39PM +0000, Chengfeng Ye wrote:
> > > As &sport->port.lock is acquired under irq context along the following
> > > call chain from imx_uart_rtsint(), other acquisition of the same lock
> > > inside process context or softirq context should disable irq avoid double
> > > lock.
> > > 
> > > <deadlock #1>
> > > 
> > > imx_uart_dma_rx_callback()
> > > --> spin_lock(&sport->port.lock)
> > > <interrupt>
> > >    --> imx_uart_rtsint()
> > >    --> spin_lock(&sport->port.lock)
> > > 
> > > This flaw was found by an experimental static analysis tool I am
> > > developing for irq-related deadlock.
> > 
> > Ah, I understood before that you really experienced that deadlock (or a
> > lockdep splat). I didn't test anything, but I think the
> > imx_uart_dma_rx_callback() is called indirectly by
> > sdma_update_channel_loop() which is called in irq context. I don't know
> > if this is the case for all dma drivers?!
> > 
> > @Vinod: Maybe you can chime in here: Is a dma callback always called in
> > irq context?
> 
> Not in callback but a tasklet context. The DMA irq handler is supposed
> to use a tasklet for invoking the callback

So drivers/dma/imx-sdma.c is bogous as it calls

	-> sdma_int_handler()
	  -> sdma_update_channel_loop()
	    -> dmaengine_desc_get_callback_invoke()

resulting in imx_uart_dma_rx_callback() (and others) being called in irq
context, right?

In that case:

Acked-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>

(for the imx-UART patch that stops assuming imx_uart_dma_rx_callback()
is called with irqs off).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)