| 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
| ||
|
Message-ID: <20141216105707.GO11285@n2100.arm.linux.org.uk>
Date: Tue, 16 Dec 2014 10:57:08 +0000
From: Russell King - ARM Linux <linux@....linux.org.uk>
To: "fugang.duan@...escale.com" <fugang.duan@...escale.com>
Cc: David Miller <davem@...emloft.net>,
"Fabio.Estevam@...escale.com" <Fabio.Estevam@...escale.com>,
"ezequiel.garcia@...e-electrons.com"
<ezequiel.garcia@...e-electrons.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Bug: mv643xxx fails with highmem
On Tue, Dec 16, 2014 at 02:19:38AM +0000, fugang.duan@...escale.com wrote:
> From: Russell King - ARM Linux <linux@....linux.org.uk> Sent: Tuesday, December 16, 2014 2:05 AM
> > To: Duan Fugang-B38611
> > Cc: David Miller; Estevam Fabio-R49496; ezequiel.garcia@...e-
> > electrons.com; netdev@...r.kernel.org
> > Subject: Re: Bug: mv643xxx fails with highmem
> >
> > On Fri, Dec 12, 2014 at 05:34:01AM +0000, fugang.duan@...escale.com wrote:
> > > I will submit one patch to fix the issue.
> >
> > There's more bugs in the FEC driver... here's the relevant bits:
> >
> > static void
> > fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) {
> > bdp = txq->dirty_tx;
> >
> > bdp = fec_enet_get_nextdesc(bdp, fep, queue_id);
> >
> > while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
> > /* current queue is empty */
> > if (bdp == txq->cur_tx)
> > break;
> >
> > skb = txq->tx_skbuff[index];
> > txq->tx_skbuff[index] = NULL;
> > if (!IS_TSO_HEADER(txq, bdp->cbd_bufaddr))
> > dma_unmap_single(&fep->pdev->dev, bdp-
> > >cbd_bufaddr,
> > bdp->cbd_datlen, DMA_TO_DEVICE);
> > bdp->cbd_bufaddr = 0;
> > if (!skb) {
> > bdp = fec_enet_get_nextdesc(bdp, fep, queue_id);
> > continue;
> > }
> > ...
> > txq->dirty_tx = bdp;
> > bdp = fec_enet_get_nextdesc(bdp, fep, queue_id);
> > }
> >
> > Consider the following code path:
> > - we enter this function
> > - get the dirty_tx pointer
> > - move to the next descriptor (which we'll call descriptor A)
> > - next descriptor indicates that TX_READY = 0
> > - bdp != txq->cur_tx
> > - we unmap if needed
> > - we set bdp->cmdbufaddr = 0
> > - assume skb is NULL, so we move to the next descriptor (we'll call this
> > B)
> > - next descriptor _may_ have TX_READY = 1
> > - we break out of the loop, and return
> >
> > Some time later, we re-enter:
> > - get the dirty_tx pointer
> > - move to the next descriptor (which is descriptor A above)
> > - next descriptor indicates that TX_READY = 0
> > - bdp != txq->cur_tx
> > - we call dma_unmap_single(..., bdp->cbd_bufaddr, which we previously
> > zeroed
> > - the DMA API debugging complains that FEC is unmapping memory which it
> > doesn't own
> >
> > Unfortunately, this does appear to happen - from a paste from Jon
> > Nettleton from iMX6Q:
> >
> > 32. [ 45.033001] unmapping this address 0x0 size 66 33. [ 45.037470]
> > ------------[ cut here ]------------ 34. [ 45.042127] WARNING: CPU: 0
> > PID: 102 at lib/dma-debug.c:1080 check_unmap+0x784/0x9f4()
> > 35. [ 45.050066] fec 2188000.ethernet: DMA-API: device driver tries to
> > free DMA memory it has not a]
> >
> > (where the printk at line 32 is something that was added to debug this.)
> >
> > The sad thing is that the remainder of my FEC patches did go a long way
> > to clean up these kinds of issues in the driver (and there's /many/ of
> > them), but unfortunately other conflicting changes got merged before I
> > could finish rebasing them, I decided to move on to other things and
> > discard the remainder of my patch set. Marek showed some interest in
> > taking the patch set over, but I've not heard anything more - and I'm not
> > about to resurect my efforts only to get into the same situation where
> > I'm carrying 50 odd patches which I can't merge back into mainline
> > without spending weeks endlessly rebasing them.
> >
> Russell, many thanks for your effort and thanks for your pointing out the bug.
> I will think one method to fix it.
>
> And I have one question for highmem dma mapping issue as below:
> fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, struct net_device *ndev)
> {
> ...
> bufaddr = page_address(this_frag->page.p) + this_frag->page_offset;
>
> index = fec_enet_get_bd_index(txq->tx_bd_base, bdp, fep);
> if (((unsigned long) bufaddr) & fep->tx_align ||
> fep->quirks & FEC_QUIRK_SWAP_FRAME) {
> memcpy(txq->tx_bounce[index], bufaddr, frag_len);
> bufaddr = txq->tx_bounce[index];
>
> if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
> swap_buffer(bufaddr, frag_len);
> }
>
> addr = dma_map_single(&fep->pdev->dev, bufaddr, frag_len,
> DMA_TO_DEVICE);
> if (dma_mapping_error(&fep->pdev->dev, addr)) {
> dev_kfree_skb_any(skb);
> if (net_ratelimit())
> netdev_err(ndev, "Tx DMA memory map failed\n");
> goto dma_mapping_error;
> }
> ...
> }
>
> If the frag page is located at high memory, use dma_map_single() is not
> right, must use skb_frag_dma_map() or dma_map_page().
Correct.
> But before mapping, if tx has buffer alignment limitation (tx_align is
> not zero), there need to do memcpy for buffer alignment.
Right, and that can be detected by simply checking this_frag->page_offset
as we know that a page address is always aligned to a page.
> So, there we need to check whether the page is in highmem, if so, we
> need to call kmap_atomic() or kmap_high_get() to get cpu address,
> And then do memcpy or swap buffer operation.
Yes - you'd need to do something like this:
if (this_frag->page_offset & fep->tx_align ||
fep->quirks & FEC_QUIRK_SWAP_FRAME) {
bufaddr = kmap_atomic(this_frag->page.p) + this_frag->page_offset;
memcpy(txq->tx_bounce[index], bufaddr, frag_len);
kunmap_atomic(bufaddr);
bufaddr = txq->tx_bounce[index];
if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(bufaddr, frag_len);
addr = dma_map_single(&fep->pdev->dev, bufaddr, frag_len,
DMA_TO_DEVICE);
} else {
addr = skb_frag_dma_map(&fep->pdev->dev, this_frag, 0,
frag_len, DMA_TO_DEVICE);
}
if (dma_mapping_error(&fep->pdev->dev, addr)) {
dev_kfree_skb_any(skb);
if (net_ratelimit())
netdev_err(ndev, "Tx DMA memory map failed\n");
goto dma_mapping_error;
}
You'll also need to record whether you should use dma_unmap_page() or
dma_unmap_single().
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists