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

[Robin Murphy <robin.murphy@arm.com>]

On 2026-11-04 7:37 am, Marek Szyprowski wrote:
> 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.

Realistically, the granularity of any physical memory protection system 
is going to be based on at least 4KB pages, so I don't see much excuse 
for any restricted pool not being aligned to at least the current 2KB 
segment size - if anything I reckon we could just consider it a DT error 
and fail the init rather than worry about rounding either way.

Note that freeing parts of a restricted pool for any other use would not 
be a thing anyway, as they are by definition not a "reusable" region.

Thanks,
Robin.
>> 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