netdev - RE: [PATCH] net:fec: fix WARNING caused by lack of calls to 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, 13 Nov 2013 02:10:04 +0000
From:	Fugang Duan <fugang.duan@...escale.com>
To:	Ben Hutchings <bhutchings@...arflare.com>
CC:	Frank Li <Frank.Li@...escale.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH] net:fec: fix WARNING caused by lack of calls to
 dma_mapping_error()

From: Ben Hutchings [mailto:bhutchings@...arflare.com]
Data: Wednesday, November 13, 2013 9:43 AM

>To: Duan Fugang-B38611
>Cc: Li Frank-B20596; davem@...emloft.net; netdev@...r.kernel.org
>Subject: Re: [PATCH] net:fec: fix WARNING caused by lack of calls to
>dma_mapping_error()
>
>On Wed, 2013-11-13 at 01:00 +0000, Ben Hutchings wrote:
>> On Tue, 2013-11-12 at 10:54 +0800, Fugang Duan wrote:
>> > Enable CONFIG_HAVE_DMA_API_DEBUG, the kernel dump warning:
>[...]
>> > @@ -1001,6 +1005,9 @@ fec_enet_rx(struct net_device *ndev, int
>> > budget)
>> >
>> >  		bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, data,
>> >  				FEC_ENET_TX_FRSIZE, DMA_FROM_DEVICE);
>> > +		/* here dma mapping shouldn't be error, just avoid kernel dump */
>> > +		if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr))
>> > +			netdev_err(ndev, "Rx DMA memory map failed\n");
>>
>> This is not handling the error.
>[...]
>
>It looks like the same RX DMA buffers are used repeatedly and the driver copies
>into an skb.  So fec_enet_rx() should not map or unmap at all; it should use
>dma_sync_single_for_cpu().  Then, indeed, no error handling is required.
>
Yes, there don't need to use dma_unmap_single()/dma_map_single() since the fec_enet_alloc_buffers() have mapped the virtual address.
I will change it. Thanks.

>But there's still a big problem with this function.  Instead of remembering the
>virtual address of each buffer, it passes the DMA address to __va().  Physical
>and DMA addresses are *not* the same thing!
>They may differ if the device is behind an IOMMU or bounce buffering was
>required, or just because there is a fixed offset between the device and host
>physical addresses.
>
Yes, I agree that physical and DMA addresses are not the same thing.
But for imx serial platforms, there have no IOMMU, no offset between device and host physical address(this happen on FPGA platforms in mostly), so there have no problem.

I agree it is not common for other platforms like IOMMU...
Since it is another problem, I will submit another patch to fix the problem.

Thanks, Ben.


>Ben.
>
>--
>Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's
>the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>