linux-kernel - Re: [PATCH 2/2] serial: imx: dma_unmap

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 May 2018 20:46:12 +0200
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Sebastian Reichel <sebastian.reichel@...labora.co.uk>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, linux-serial@...r.kernel.org,
        Fabio Estevam <fabio.estevam@....com>,
        Shawn Guo <shawnguo@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] serial: imx: dma_unmap_sg buffers on shutdown

Hello Sebastian,

On Tue, May 08, 2018 at 03:40:47PM +0200, Sebastian Reichel wrote:
> On Tue, May 08, 2018 at 08:43:51AM +0200, Uwe Kleine-König wrote:
> > On Mon, May 07, 2018 at 11:36:10PM +0200, Sebastian Reichel wrote:
> > > This properly unmaps DMA SG on device shutdown.
> > > 
> > > Reported-by: Nandor Han <nandor.han@...com>
> > > Suggested-by: Nandor Han <nandor.han@...com>
> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.co.uk>
> > > ---
> > >  drivers/tty/serial/imx.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > > index 3ca767b1162a..6c53e74244ec 100644
> > > --- a/drivers/tty/serial/imx.c
> > > +++ b/drivers/tty/serial/imx.c
> > > @@ -1425,10 +1425,18 @@ static void imx_uart_shutdown(struct uart_port *port)
> > >  	u32 ucr1, ucr2;
> > >  
> > >  	if (sport->dma_is_enabled) {
> > > -		sport->dma_is_rxing = 0;
> > > -		sport->dma_is_txing = 0;
> > >  		dmaengine_terminate_sync(sport->dma_chan_tx);
> > > +		if (sport->dma_is_txing) {
> > > +			dma_unmap_sg(sport->port.dev, &sport->tx_sgl[0],
> > > +				     sport->dma_tx_nents, DMA_TO_DEVICE);
> > > +			sport->dma_is_txing = 0;
> > > +		}
> > 
> > did you find this because the kernel crashed or consumed more and more
> > memory, or is this "only" a finding of reading the source code? If the
> > former it would be great to point out in the commit log, if the latter,
> > I wonder if this is a real problem that warrants a stable backport.
> 
> A bit of both. One of Collabora's customers had a (scarce) kernel crash
> in imx-serial and modified multiple things in the driver. The crash is
> gone, but it's not clear which change fixed it. I could not
> reproduce the crash so far and I'm currently rebasing and splitting
> their changes into upstreamable portions with proper patch
> descriptions. From reading the source this looked like a real issue.

In which context (kernel version, operating mode (e.g. rs485)) did these
happen? What does "crash" mean? The kernel did just hang or produced an
oops? If the latter, can you show it/them?

Best regards
Uwe

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