Using restricted DMA pools (CONFIG_DMA_RESTRICTED_POOL=y) in conjunction
with dynamic SWIOTLB (CONFIG_SWIOTLB_DYNAMIC=y) leads to the following
crash when initialising the restricted pools at boot-time:
| Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
| Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
| pc : rmem_swiotlb_device_init+0xfc/0x1ec
| lr : rmem_swiotlb_device_init+0xf0/0x1ec
| Call trace:
| rmem_swiotlb_device_init+0xfc/0x1ec
| of_reserved_mem_device_init_by_idx+0x18c/0x238
| of_dma_configure_id+0x31c/0x33c
| platform_dma_configure+0x34/0x80
faddr2line reveals that the crash is in the list validation code:
include/linux/list.h:83
include/linux/rculist.h:79
include/linux/rculist.h:106
kernel/dma/swiotlb.c:306
kernel/dma/swiotlb.c:1695
because add_mem_pool() is trying to list_add_rcu() to a NULL
'mem->pools'.
Fix the crash by initialising the 'mem->pools' list_head in
rmem_swiotlb_device_init() before calling add_mem_pool().
Cc: Christoph Hellwig <[email protected]>
Cc: Marek Szyprowski <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Petr Tesařík <[email protected]>
Cc: Michael Kelley <[email protected]>
Reported-by: Nikita Ioffe <[email protected]>
Tested-by: Nikita Ioffe <[email protected]>
Fixes: 1aaa736815eb ("swiotlb: allocate a new memory pool when existing pools are full")
Signed-off-by: Will Deacon <[email protected]>
---
kernel/dma/swiotlb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 86fe172b5958..87dd3301dde3 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -1773,6 +1773,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
mem->for_alloc = true;
#ifdef CONFIG_SWIOTLB_DYNAMIC
spin_lock_init(&mem->lock);
+ INIT_LIST_HEAD_RCU(&mem->pools);
#endif
add_mem_pool(mem, pool);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
Thanks,
applied to the dma-mapping for-linus branch.
I plan to send it to Linus this weekend unless someone find a grave bug
in this pretty obvious one liner.
On Thu, 2 May 2024 14:56:01 +0200
Christoph Hellwig <[email protected]> wrote:
> Thanks,
>
> applied to the dma-mapping for-linus branch.
>
> I plan to send it to Linus this weekend unless someone find a grave bug
> in this pretty obvious one liner.
Thank you, and big thanks to Will for the fix!
Yes, the fix is obviously correct. During development, the pool list was
never dereferenced when mem->can_grow was false, but I forgot to add
the initialization when I optimized away the check for can_grow.
BTW this mem->can_grow flag is also why mem->dyn_alloc can be left
uninitialized, but now I wonder if it should be initialized even though
it's unused, just to make the code more robust in case of future
changes.
Petr T