RE: [PATCH] dma: swiotlb: Round restricted DMA pool size down to IO_TLB_SEGSIZE

[Michael Kelley <mhklinux@outlook.com>]

From: Marek Szyprowski <m.szyprowski@samsung.com> Sent: Tuesday, November 3, 2026 11:37 PM
> 
> On 03.11.2026 22:18, Michael Kelley wrote:
> > From: Marek Szyprowski <m.szyprowski@samsung.com> Sent: Tuesday, November 3, 2026 7:50 AM
> >> All other paths creating a swiotlb pool ensure that nslabs is a multiple
> >> of IO_TLB_SEGSIZE, and the swiotlb code relies on that. Since commit
> >> 0b84e4f8b793 ("swiotlb: Add restricted DMA pool initialization") a
> >> restricted pool can have an arbitrary size taken from the device tree.
> >>
> >> If the pool size is not a multiple of IO_TLB_SEGSIZE, the last segment is
> >> partial and swiotlb_release_slots() reads mem->slots[index + nslabs],
> >> which is one past the end of the slots array when the freed buffer ends
> >> at the end of the pool:
> >>
> >>  BUG: KASAN: slab-out-of-bounds in swiotlb_release_slots
> >>  Read of size 2
> >>
> >> Round the pool size down to a multiple of IO_TLB_SEGSIZE, wasting a bit
> >> of memory, and warn about it.
> > I'm not familiar with the macro scenarios where a device has a
> > restricted pool specified in DT. But I wonder if it would be better to
> > round up instead of down. DT has specified that the device should
> > have a restricted DMA pool of a certain size, and rounding down gives
> > the device a smaller pool than was requested. Could DMA operations
> > then fail because of not having a large enough pool? Rounding down
> > also allows the pool to end up being size zero, which this patch
> > detects and results in a definite failure at device initialization time.
> >
> > What's the tradeoff in rounding up instead?The device restricted
> > pool at least as large as requested. Presumably, being larger than
> > requested doesn't cause any problems for the device or its driver.
> > The downside is using up to 256 KiB more system memory than
> > expected. In an embedded system with only a few MiB of system
> > memory, maybe that's a relatively big problem. I don't know the
> > scenarios well enough to judge whether using a bit more system
> > memory is better or worse than the device not having all the
> > restricted pool space it said it needed for DMA.
> 
> The main problem with enlarging the restricted pool is that the memory
> has been already allocated (reserved) by the generic of_reserved_mem
> code before the rmem_swiotlb_device_init() is called. There might be
> even another reserved region just after the one being processed by
> rmem_swiotlb_device_init(). The reserved region might be specified in
> the device tree exactly, by pointing its base address and size. In
> such case the rmem_swiotlb_device_init() really cannot do anything
> else than using the provided already allocated region partially.
> The only thing that might be improved is freeing the 'wasted' part
> of the region back to the memory allocator if You think that
> printing a warning is not enough.
> 

OK, got it. Thanks for the short education on how this works. :-)

And with that,
Reviewed-by: Michael Kelley <mhklinux@outlook.com>

> 
> 
> >
> > Your commit message also mentions "wasting a bit of memory".
> > What is wasted in the round down case? The rounding down is
> > done before the pool memory is allocated. I would have thought
> > the wasting would occur in the round up case, where the restricted
> > pool is given more memory than the device said it needed.
> >
> > Michael
> >
> >> Reported-by: Giorgi Kobakhia <gkobakhi@asu.edu>
> >> Closes: https://lore.kernel.org/all/20261026032007.1957949-1-gkobakhi@asu.edu/
> >> Fixes: 0b84e4f8b793 ("swiotlb: Add restricted DMA pool initialization")
> >> CC: stable@vger.kernel.org
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >>  kernel/dma/swiotlb.c | 15 ++++++++++++++-
> >>  1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> >> index 2c57b49ac7c0..a68f35192582 100644
> >> --- a/kernel/dma/swiotlb.c
> >> +++ b/kernel/dma/swiotlb.c
> >> @@ -2007,6 +2007,19 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> >>  	 */
> >>  	if (!mem) {
> >>  		struct io_tlb_pool *pool;
> >> +		unsigned long aligned_nslabs = ALIGN_DOWN(nslabs, IO_TLB_SEGSIZE);
> >> +
> >> +		if (aligned_nslabs != nslabs) {
> >> +			dev_warn(dev, "Restricted DMA pool size %zu is not a multiple of %lu, rounding down to %lu bytes\n",
> >> +				 (size_t)rmem->size,
> >> +				 (unsigned long)IO_TLB_SEGSIZE << IO_TLB_SHIFT,
> >> +				 aligned_nslabs << IO_TLB_SHIFT);
> >> +			nslabs = aligned_nslabs;
> >> +		}
> >> +		if (!nslabs) {
> >> +			dev_err(dev, "Restricted DMA pool is too small\n");
> >> +			return -EINVAL;
> >> +		}
> >>
> >>  		mem = kzalloc_obj(*mem);
> >>  		if (!mem)
> >> @@ -2034,7 +2047,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> >>
> >>  			mem->cc_shared = true;
> >>  			ret = set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
> >> -						   rmem->size >> PAGE_SHIFT);
> >> +						   (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
> >>  			if (ret) {
> >>  				dev_err(dev, "Failed to decrypt restricted DMA pool\n");
> >>  				kfree(pool->areas);
> >> --
> >> 2.43.0
> >
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>