linux-kernel - DMA API usage in au1000

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>] [day] [month] [year] [list]

Date:   Tue, 25 Jun 2019 14:34:33 +0200
From:   Christoph Hellwig <hch@....de>
To:     BjoernRiemerriemer@...us.fraunhofer.de,
        Matt Porter <mporter@...nel.crashing.org>,
        Herbert Valerio Riedel <hvr@....org>
Cc:     linux-mips@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: DMA API usage in au1000_eth.c

Hi all,

you are the persons that have their names listed in the driver,
hope you all remember what you did 15 years ago :)

The au1000_eth driver uses the DMA API somewhat oddly.  For one
it uses the DMA_ATTR_NON_CONSISTENT flag to allocate memory that
is not DMA coherent, accompanied by a comment say:

	/* Allocate the data buffers
	 * Snooping works fine with eth on all au1xxx
	 */

which suggests that it actually is DMA coherent.  As far as I can
tell many amd mips platforms are DMA coherent, in which case
DMA_ATTR_NON_CONSISTENT is no-op and everything is fine here.  But
it seems some are not, in which case DMA_ATTR_NON_CONSISTENT will
give us not coherent memory, in which case something would be
broken?  Removing DMA_ATTR_NON_CONSISTENT would be no-op on
coherent platforms, but return an address in the cache segement
on those that are non-coherent.  Do you know if the hardware
event cares?

The next issue is that it calls virt_to_phys on the return value
from dma_alloc_attrs.  Why would the hardware care about the physical
address and not the DMA address (in general those are the same in
mips)?

Last but not least it stores the kernel address return from
dma_alloc_attrs in a u32 instead of a pointer, which is a little
odd and not type safe, but not otherwise dramatic.

I can prepare a patch to fix these up, but I'd like to confirm my
theory about coherent of the platform and device first.