linux-kernel - Re: [PATCH] dma-debug: fix physical address calculation for struct dma_debug

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

Message-ID: <20241128035011.GA13047@lst.de>
Date: Thu, 28 Nov 2024 04:50:11 +0100
From: Christoph Hellwig <hch@....de>
To: Fedor Pchelkin <pchelkin@...ras.ru>
Cc: Christoph Hellwig <hch@....de>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Robin Murphy <robin.murphy@....com>, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH] dma-debug: fix physical address calculation for struct
 dma_debug_entry

On Wed, Nov 27, 2024 at 09:59:26PM +0300, Fedor Pchelkin wrote:
> Offset into the page should also be considered while calculating a physical
> address for struct dma_debug_entry. page_to_phys() just shifts the value
> PAGE_SHIFT bits to the left so offset part is zero-filled.
> 
> An example (wrong) debug assertion failure with CONFIG_DMA_API_DEBUG
> enabled which is observed during systemd boot process after recent
> dma-debug changes:

Thanks!

Is it ok for you if I fold in the following cleanup to have a helper
instead of the duplicate very dense expression?

diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 27ade2bab531..e43c6de2bce4 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -1377,6 +1377,18 @@ void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
 	}
 }
 
+static phys_addr_t virt_to_paddr(void *virt)
+{
+	struct page *page;
+
+	if (is_vmalloc_addr(virt))
+		page = vmalloc_to_page(virt);
+	else
+		page = virt_to_page(virt);
+
+	return page_to_phys(page) + offset_in_page(virt);
+}
+
 void debug_dma_alloc_coherent(struct device *dev, size_t size,
 			      dma_addr_t dma_addr, void *virt,
 			      unsigned long attrs)
@@ -1399,9 +1411,7 @@ void debug_dma_alloc_coherent(struct device *dev, size_t size,
 
 	entry->type      = dma_debug_coherent;
 	entry->dev       = dev;
-	entry->paddr	 = page_to_phys((is_vmalloc_addr(virt) ?
-				vmalloc_to_page(virt) : virt_to_page(virt))) +
-				offset_in_page(virt);
+	entry->paddr	 = virt_to_paddr(virt);
 	entry->size      = size;
 	entry->dev_addr  = dma_addr;
 	entry->direction = DMA_BIDIRECTIONAL;
@@ -1424,9 +1434,7 @@ void debug_dma_free_coherent(struct device *dev, size_t size,
 	if (!is_vmalloc_addr(virt) && !virt_addr_valid(virt))
 		return;
 
-	ref.paddr = page_to_phys((is_vmalloc_addr(virt) ?
-			vmalloc_to_page(virt) : virt_to_page(virt))) +
-			offset_in_page(virt);
+	ref.paddr = virt_to_paddr(virt);
 
 	if (unlikely(dma_debug_disabled()))
 		return;