netdev - Re: [PATCH] 3c59x: fix missing dma_mapping

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:   Wed, 3 Jan 2018 05:42:04 -0500
From:   Neil Horman <nhorman@...driver.com>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, nhorman@...hat.com,
        klassert@...hematik.tu-chemnitz.de
Subject: Re: [PATCH] 3c59x: fix missing dma_mapping_error check

On Tue, Jan 02, 2018 at 09:48:27PM -0500, David Miller wrote:
> From: Neil Horman <nhorman@...driver.com>
> Date: Fri, 29 Dec 2017 11:40:10 -0500
> 
> > @@ -2067,6 +2072,9 @@ vortex_start_xmit(struct sk_buff *skb, struct net_device *dev)
> >  		int len = (skb->len + 3) & ~3;
> >  		vp->tx_skb_dma = pci_map_single(VORTEX_PCI(vp), skb->data, len,
> >  						PCI_DMA_TODEVICE);
> > +		if (dma_mapping_error(&VORTEX_PCI(vp)->dev, vp->tx_skb_dma))
> > +			return NETDEV_TX_OK;
> > +
> 
> This leaks the SKB, right?
> 
Crud, I think you're right, I'll respin this to address today.

> And for the RX cases, it allows the RX ring to deplete to empty which
> tends to hang most chips.  You need to make the DMA failure detection
> early and recycle the RX buffer back to the chip instead of passing
> it up to the stack.
> 
Strictly speaking, I think we're ok here, because the dirty_rx counter creates a
contiguous area to refill, and we will just pick up where we left off on the
next napi poll.

That said, it can still depelete if we get several consecutive
dma_mapping_errors in a sufficiently long period of time that the ring doesn't
ever refill, so yeah, recycling will be better.  That will take a bigger rewrite
of this code.  I'll post something asap.

Neil