RE: [PATCH] swiotlb: fix out-of-bounds read in swiotlb_release_slots()

[Michael Kelley <mhklinux@outlook.com>]

From: Giorgi Kobakhia <gkobakhi@asu.edu> Sent: Sunday, October 25, 2026 8:20 PM
> 
> When buffer is freed, swiotlb_release_slots() tries to merge it with the
> following free space by reading the list value of the next slot.
> 
>     if (index + nslots < ALIGN(index + 1, IO_TLB_SEGSIZE))
>         count = mem->slots[index + nslots].list;
>     else
>         count = 0;
> 
> If index is in the last segment of the pool and the pool size is not
> multiple of IO_TLB_SEGSIZE, index + nslots becomes equal to mem->nslabs
> leading to out of bound read.
> 
>  BUG: KASAN: slab-out-of-bounds in swiotlb_release_slots
>  Read of size 2
> 
>  Call trace:
>   swiotlb_release_slots (kernel/dma/swiotlb.c:1653)
>   __swiotlb_tbl_unmap_single (kernel/dma/swiotlb.c:1734)
>   dma_unmap_phys (./include/linux/swiotlb.h:262 kernel/dma/direct.h:102
>                     kernel/dma/mapping.c:218)
>   dma_unmap_page_attrs (kernel/dma/mapping.c:236)
> 
> The commit 0b84e4f8b793 ("swiotlb: Add restricted DMA pool
> initialization") makes it possible to initialize a pool with an
> arbitrary size. Before it nslabs was multiple of IO_TLB_SEGSIZE and no
> OOB occurred.
> 
> Add overflow check.

I can see that this is a legitimate problem. But I'm not sure this is the right
solution.

All other paths for creating a swiotlb pool ensure that nslabs is a multiple
of IO_TLB_SEGSIZE. The better solution may be for rmem_swiotlb_device_init()
to round-up nslabs to be a multiple of IO_TLB_SEGSIZE. My concern is
that there may be other places in the swiotlb code where there is an
implicit assumption that there are no partial segments at the end of
a pool. It would take some effort for me to re-immerse myself in the
swiotlb code to see if that's true, and I haven't done that. But my
guess is that there's a good chance of such an assumption.

What are the implications of potentially allocating slightly more memory
for a restricted pool than is specified? It wouldn't be much additional
memory since the segment size is 256 KiB.

Michael

> 
> Fixes: 0b84e4f8b793 ("swiotlb: Add restricted DMA pool initialization")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Tested-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Giorgi Kobakhia <gkobakhi@asu.edu>
> ---
>  kernel/dma/swiotlb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index ded7016a46a7..1f18003be343 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -1649,7 +1649,8 @@ static void swiotlb_release_slots(struct device *dev,
> phys_addr_t tlb_addr,
>  	BUG_ON(aindex >= mem->nareas);
> 
>  	spin_lock_irqsave(&area->lock, flags);
> -	if (index + nslots < ALIGN(index + 1, IO_TLB_SEGSIZE))
> +	if (index + nslots < ALIGN(index + 1, IO_TLB_SEGSIZE) &&
> +	    index + nslots < mem->nslabs)
>  		count = mem->slots[index + nslots].list;
>  	else
>  		count = 0;
> --
> 2.43.0
>