RE: [PATCH] swiotlb: use the adjusted address for the highmem page lookup

[Michael Kelley]

From: Donggeun Yoo <donggeunyoo.kernel@xxxxxxxxx> Sent: Saturday, September 5, 2026 1:42 AM
> 
> swiotlb_bounce() reads the page frame number from the slot's recorded
> orig_addr, then advances orig_addr by tlb_offset to reach the address
> the caller asked about. The highmem branch mixes the two: the offset
> within the page comes from the adjusted address, the page from the value
> before it.
> 
> Once the adjustment crosses a page boundary the pair no longer describes
> one location, and the whole copy lands one page below the intended one
> for a positive tlb_offset, one above for a negative one. DMA_FROM_DEVICE
> writes the device data over the wrong page and leaves the intended one
> stale, DMA_TO_DEVICE feeds the device from a page the mapping may not
> cover. Partial syncs through dma_sync_single_range_for_*() are what make
> tlb_offset non-zero.

Yes -- this all makes sense. Current code is clearly using the wrong starting
pfn in the highmem branch, and your fix looks good.

> 
> The branch test is picked the same way, so a slot recorded in lowmem can
> be adjusted into highmem and the lowmem path then hands a highmem
> address to phys_to_virt().

I don't understand this paragraph, but that may be because I'm not that
familiar with highmem. Are all the slots making up a particular swiotlb
mapping either highmem or lowmem? If a mixture is possible, then a
partial sync could start somewhere in a lowmem page and cross over
into a highmem page, which would break.

Michael

> 
> Take both from orig_addr once it is final and keep pfn in the branch
> that uses it. PhysHighMem() asks the question straight from the address,
> as dma-debug already does.
> 
> Fixes: 5f89468e2f06 ("swiotlb: manipulate orig_addr when tlb_addr has offset")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@xxxxxxxxx>
> ---
> Reproduced under QEMU (qemu-system-arm -M virt, 2G) with a 32-bit ARM
> kernel built from multi_v7_defconfig plus CONFIG_ARM_LPAE=y and
> CONFIG_HIGHMEM=y, which brings in CONFIG_SWIOTLB, booted with
> swiotlb=force. A test module maps two highmem pages at page offset 3840,
> writes a pattern into the bounce buffer 500 bytes in and calls
> dma_sync_single_range_for_cpu() over that range, so that 3840 + 500
> crosses into the second page:
> 
>   before: swbug: pages pfn=700d4 highmem=1
>           swbug: orig phys=700d4f00 (page off 3840)
>           swbug: RESULT page0_off=244 page1_off=-1
>   after:  swbug: RESULT page0_off=-1 page1_off=244
> 
> The pattern lands one page below its intended location without the
> change and in the right place with it.
> 
>  kernel/dma/swiotlb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index ded7016a46a7..aa2f1c4588b9 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -1019,7 +1019,6 @@ static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size
>  	int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
>  	phys_addr_t orig_addr = mem->slots[index].orig_addr;
>  	size_t alloc_size = mem->slots[index].alloc_size;
> -	unsigned long pfn = PFN_DOWN(orig_addr);
>  	unsigned char *vaddr = mem->vaddr + tlb_addr - mem->start;
>  	int tlb_offset;
> 
> @@ -1052,7 +1051,8 @@ static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size
>  		size = alloc_size;
>  	}
> 
> -	if (PageHighMem(pfn_to_page(pfn))) {
> +	if (PhysHighMem(orig_addr)) {
> +		unsigned long pfn = PFN_DOWN(orig_addr);
>  		unsigned int offset = orig_addr & ~PAGE_MASK;
>  		struct page *page;
>  		unsigned int sz = 0;
> --
> 2.53.0
>