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

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

[Giorgi Kobakhia]

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.

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

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

[Michael Kelley]

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
> 

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

[Marek Szyprowski]

On 29.10.2026 18:39, Michael Kelley wrote:
> 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.
Frankly speaking it would much easier to round the pool size down to match
the multiple of IO_TLB_SEGSIZE, waste some memory and issue a warning.
It is not that trivial to allocate "slightly more memory" if the size of
the restricted pool is statically specified in device tree.

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