| 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
| ||
|
Date: Tue, 16 Dec 2014 02:19:38 +0000
From: "fugang.duan@...escale.com" <fugang.duan@...escale.com>
To: Russell King - ARM Linux <linux@....linux.org.uk>
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
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().
But before mapping, if tx has buffer alignment limitation (tx_align is not zero), there need to do memcpy for buffer alignment.
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.
Do you think the above solution is right ? or maybe there have better method to fix it ?
Regards,
Andy
--
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