dma-mapping: DMA_ATTR_NO_KERNEL_MAPPING related fixes

Hi All,

arch_dma_prep_coherent() is called by the generic DMA code on pages which may be located in highmem: dma_direct_alloc() forces the remap path for a highmem page returned from CMA and calls this hook on it before creating the uncached mapping. However, the hook has never been documented and some implementations assume that the page has a kernel linear mapping. This issue affects MIPS, xtensa and PowerPC. It gets fixed by patches #2-#4.

xtensa also only invalidates the cache, which discards the zeroes written by the dma_direct_alloc(). Patch #1 switches it to a writeback+invalidate.

Patch #5 documents that arch_dma_prep_coherent() must write back dirty lines and must accept highmem pages. With that in place, patch #6 drops the PageHighMem() check in dma_direct_alloc_no_mapping(), which so far skipped cache maintenance for highmem CMA pages allocated with DMA_ATTR_NO_KERNEL_MAPPING.

Patch #7 makes dma_direct_alloc_no_mapping() always return zeroed memory, like all other dma_alloc_* paths. Passing __GFP_ZERO is not enough there, because CMA ignores.

The analysis and the patchset was prepared with the help of an LLM. The code compiles fine, but it has not been runtime tested on any of the affected architectures yet. Testing on real hardware would be appreciated. I've reviewed the resulting changes.

I plan to merge this via my dma-mapping tree, so I would like to get acks from xtensa, MIPS and PowerPC arch maintainers.

Best regards
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Changelog:
v2:
- fixed minor issues in comments and patch descriptions pointed by sashiko
- dropped RFC annotation

v1: https://lore.kernel.org/all/[email protected]/

Patch summary:

Marek Szyprowski (7):
xtensa: write back dirty lines in arch_dma_prep_coherent()
xtensa: handle highmem pages in arch_dma_prep_coherent()
MIPS: mm: handle highmem pages in arch_dma_prep_coherent() powerpc/mm: handle highmem pages in arch_dma_prep_coherent()
dma-mapping: document that arch_dma_prep_coherent() must handle
highmem
dma-direct: do not skip cache maintenance for highmem pages in
no-mapping allocations
dma-direct: zero memory allocated with DMA_ATTR_NO_KERNEL_MAPPING

 arch/mips/mm/dma-noncoherent.c    | 10 +++++-----
 arch/powerpc/mm/dma-noncoherent.c |  4 +---
 arch/xtensa/kernel/pci-dma.c      | 12 +++++++++++-
 include/linux/dma-map-ops.h       | 11 +++++++++++
 kernel/dma/direct.c               |  9 +++++++--
 5 files changed, 35 insertions(+), 11 deletions(-)

2.43.0

arch_dma_prep_coherent() is expected to remove any dirty cache lines covering the buffer, so that they are neither lost nor evicted later on top of data written through the uncached alias or by the device. xtensa implements it with __invalidate_dcache_range(), which uses the DHI instruction and discards dirty lines without writing them back.

This loses data written through the cached kernel mapping right before the call. dma_direct_alloc() zeroes the buffer with memset() on the cached linear mapping and only then calls arch_dma_prep_coherent() and switches to the KSEG bypass alias via arch_dma_set_uncached(). On cores with a write-back data cache the zeroes still sitting in the cache are dropped, and dma_alloc_coherent() returns a buffer with stale content.

Use __flush_invalidate_dcache_range() (DHWBI) instead, which writes the dirty lines back before invalidating them. On cores with a write-through data cache it maps to __invalidate_dcache_range(), so nothing changes there.

---
 arch/xtensa/kernel/pci-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 94955caa4488..e1c00ec37eed 100644
@@ -82,7 +82,7 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
 
 void arch_dma_prep_coherent(struct page *page, size_t size)
 {
-	__invalidate_dcache_range((unsigned long)page_address(page), size);
+	__flush_invalidate_dcache_range((unsigned long)page_address(page), size);
 }
 
 /*

2.43.0

arch_dma_prep_coherent() performs the cache maintenance on the address returned by page_address(). xtensa with CONFIG_HIGHMEM uses HASHED_PAGE_VIRTUAL, so page_address() returns NULL for a highmem page that is not kmapped, and the cache operation then works on the virtual range starting at 0 instead of the buffer, leaving any dirty cache lines of the buffer in place.

Highmem pages do reach this function: dma_alloc_contiguous() can return a highmem CMA page, which dma_direct_alloc() handles by forcing the remap path and calling arch_dma_prep_coherent() on that page before creating the uncached mapping.

Fix this by reusing do_cache_op(), which the arch_sync_dma_for_*() hooks already use and which maps highmem pages one page at a time with kmap_atomic(). For lowmem pages it does a single cache operation over the whole range, so the behaviour is unchanged there. A small wrapper is needed because __flush_invalidate_dcache_range() is a macro on cores with a write-through data cache.

---
 arch/xtensa/kernel/pci-dma.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

index e1c00ec37eed..41467a1ba068 100644
@@ -80,9 +80,19 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
 	}
 }
 
+/*
+ * __flush_invalidate_dcache_range() may be a macro on cores with a
+ * write-through data cache, so it cannot be passed to do_cache_op().
+ */
+static void flush_invalidate_dcache_range(unsigned long start,
+					  unsigned long size)
+{
+	__flush_invalidate_dcache_range(start, size);
+}
+
 void arch_dma_prep_coherent(struct page *page, size_t size)
 {
-	__flush_invalidate_dcache_range((unsigned long)page_address(page), size);
+	do_cache_op(page_to_phys(page), size, flush_invalidate_dcache_range);
 }
 
 /*

2.43.0

arch_dma_prep_coherent() performs the cache maintenance on the address returned by page_address(). On 32-bit MIPS with CONFIG_HIGHMEM the kernel uses HASHED_PAGE_VIRTUAL, so page_address() returns NULL for a highmem page that is not kmapped, and dma_cache_wback_inv() then operates on the virtual range starting at 0 instead of the buffer, leaving any dirty cache lines of the buffer in place.

Highmem pages do reach this function: dma_alloc_contiguous() can return a highmem CMA page, which dma_direct_alloc() handles by forcing the remap path and calling arch_dma_prep_coherent() on that page before creating the uncached mapping. Since commit f5ff79fddf0e ("dma-mapping: remove CONFIG_DMA_REMAP") this path is available on all MMU architectures, including MIPS, which previously rejected highmem CMA pages.

Fix this by reusing dma_sync_phys(), which already handles highmem pages by mapping them one page at a time with kmap_atomic(). For lowmem pages it does a single dma_cache_wback_inv() over the whole range, so the behaviour is unchanged there.

---
 arch/mips/mm/dma-noncoherent.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

index 30ef3e247eb7..d8e5bc85bb3b 100644
@@ -44,11 +44,6 @@ static inline bool cpu_needs_post_dma_flush(void)
 	}
 }
 
-void arch_dma_prep_coherent(struct page *page, size_t size)
-{
-	dma_cache_wback_inv((unsigned long)page_address(page), size);
-}
-
 void *arch_dma_set_uncached(void *addr, size_t size)
 {
 	return (void *)(__pa(addr) + UNCAC_BASE);

@@ -127,6 +122,11 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
 	dma_sync_phys(paddr, size, dir, true);
 }
 
+void arch_dma_prep_coherent(struct page *page, size_t size)
+{
+	dma_sync_phys(page_to_phys(page), size, DMA_BIDIRECTIONAL, true);
+}
+
 #ifdef CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU
 void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
 		enum dma_data_direction dir)

2.43.0

arch_dma_prep_coherent() performs the cache maintenance on the address returned by page_address(). On 32-bit powerpc with CONFIG_HIGHMEM that is NULL for a highmem page which is not kmapped, so the flush would operate on the wrong virtual range and leave the dirty lines of the buffer in place.

The generic DMA code may pass highmem pages to this hook:
dma_direct_alloc() handles a highmem page returned by dma_alloc_contiguous() by forcing the remap path, which calls arch_dma_prep_coherent() on that page before creating the uncached mapping. This cannot happen on powerpc today only because it does not select HAVE_DMA_CONTIGUOUS, so dma-direct never gets a highmem page there, but nothing in the hook's contract rules it out.

Use __dma_sync_page() with DMA_BIDIRECTIONAL, which the arch_sync_dma_for_*() hooks already use and which kmaps highmem pages one at a time. Without CONFIG_HIGHMEM it does the same flush_dcache_range() on the linear mapping as before.

---
 arch/powerpc/mm/dma-noncoherent.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

index 30260b5d146d..e646256365d5 100644
@@ -118,7 +118,5 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
 
 void arch_dma_prep_coherent(struct page *page, size_t size)
 {
-	unsigned long kaddr = (unsigned long)page_address(page);
-
-	flush_dcache_range(kaddr, kaddr + size);
+	__dma_sync_page(page_to_phys(page), size, DMA_BIDIRECTIONAL);
 }

2.43.0

arch_dma_prep_coherent() has no documentation, and several implementations assumed that the page passed to it always has a kernel linear mapping and used page_address() on it. That is not the case:
dma_direct_alloc() and atomic_pool_expand() can get a highmem page from CMA and pass it to arch_dma_prep_coherent() before remapping it, and dma-iommu allocates the pages for non-contiguous buffers with __GFP_HIGHMEM.

Add a kernel-doc comment describing what the hook has to do and stating explicitly that it must accept highmem pages without a permanent kernel mapping.

---
 include/linux/dma-map-ops.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

index afaf2c12189b..ee4ac12bf2ad 100644
@@ -371,6 +371,17 @@ static inline void arch_sync_dma_for_cpu_all(void)
 }
 #endif /* CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL */
 
+/**
+ * arch_dma_prep_coherent - prepare memory for use as a coherent DMA buffer
+ * @page: first page of the physically contiguous buffer
+ * @size: size of the buffer in bytes
+ *
+ * Called before the buffer is accessed through an uncached alias or handed
+ * to a non-coherent device. The implementation must write back any dirty
+ * data cache lines covering the buffer, so no dirty line can be evicted
+ * later on top of data written through the uncached alias or by the device.
+ * Must accept highmem pages without a permanent kernel mapping.
+ */
 #ifdef CONFIG_ARCH_HAS_DMA_PREP_COHERENT
 void arch_dma_prep_coherent(struct page *page, size_t size);
 #else

2.43.0

dma_direct_alloc_no_mapping() calls arch_dma_prep_coherent() only for lowmem pages. The check was added when the DMA_ATTR_NO_KERNEL_MAPPING handling was moved to dma-direct, because some implementations of the hook used page_address() and could not handle highmem pages.

As a result, a buffer backed by a highmem CMA page is handed to the device without removing the dirty cache lines covering it, which may still be present from the previous user of the page. These lines can be evicted later and corrupt data written by a non-coherent device.

All implementations of arch_dma_prep_coherent() now handle highmem pages, and the hook is documented to do so; dma_direct_alloc() already calls it on highmem pages in its remap path. Drop the check so that highmem pages get the same cache maintenance as lowmem pages.

---
 kernel/dma/direct.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 25e9a86835fd..fea0dd36dd25 100644
@@ -207,8 +207,7 @@ static void *dma_direct_alloc_no_mapping(struct device *dev, size_t size,
 		return NULL;
 
 	/* remove any dirty cache lines on the kernel alias */
-	if (!PageHighMem(page))
-		arch_dma_prep_coherent(page, size);
+	arch_dma_prep_coherent(page, size);
 	/*
 	 * return the page pointer as the opaque cookie.
 	 * Never used for unencrypted allocation

2.43.0

dma_direct_alloc_no_mapping() explicitly clears __GFP_ZERO before allocating the pages, so buffers allocated with DMA_ATTR_NO_KERNEL_MAPPING are handed out with stale contents of the previously freed pages. Such buffers are typically passed to devices or mapped to userspace, which may expose old kernel data.

The mask comes from the regular allocation path, which always zeroes the buffer manually through its kernel mapping once it is set up. The DMA_ATTR_NO_KERNEL_MAPPING path returns early with only a page cookie, so it never reaches that memset(). This has been the case since the attribute was first supported in the generic remap code in commit e440e26a0251 ("dma-remap: support DMA_ATTR_NO_KERNEL_MAPPING") and was carried over when that handling was lifted into dma-direct.

All other dma_alloc_* paths always return zeroed memory, so do the same here. Passing __GFP_ZERO to the page allocator is not sufficient, as __dma_direct_alloc_pages() first tries dma_alloc_contiguous(), and CMA ignores __GFP_ZERO.

---
 kernel/dma/direct.c | 6 ++++++
 1 file changed, 6 insertions(+)

index fea0dd36dd25..4bfb3e76cd76 100644
@@ -6,6 +6,7 @@
  */
 #include <linux/memblock.h> /* for max_pfn */
 #include <linux/export.h>
+#include <linux/highmem.h>
 #include <linux/mm.h>
 #include <linux/dma-map-ops.h>
 #include <linux/scatterlist.h>

@@ -201,11 +202,16 @@ static void *dma_direct_alloc_no_mapping(struct device *dev, size_t size,
 		dma_addr_t *dma_handle, gfp_t gfp)
 {
 	struct page *page;
+	unsigned long i;
 
 	page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
 	if (!page)
 		return NULL;
 
+	/* we always manually zero the allocated memory */
+	for (i = 0; i < size >> PAGE_SHIFT; i++)
+		clear_highpage(page + i);
+
 	/* remove any dirty cache lines on the kernel alias */
 	arch_dma_prep_coherent(page, size);
 	/*

2.43.0