[PATCH] swiotlb: Initialise restricted pool list_head when SWIOTLB_DYNAMIC=y

[PATCH] swiotlb: Initialise restricted pool list_head when SWIOTLB_DYNAMIC=y

[Will Deacon]

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 <hch@lst.de>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Petr Tesařík <petr@tesarici.cz>
Cc: Michael Kelley <mhkelley58@gmail.com>
Reported-by: Nikita Ioffe <ioffe@google.com>
Tested-by: Nikita Ioffe <ioffe@google.com>
Fixes: 1aaa736815eb ("swiotlb: allocate a new memory pool when existing pools are full")
Signed-off-by: Will Deacon <will@kernel.org>
---
 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

Re: [PATCH] swiotlb: Initialise restricted pool list_head when SWIOTLB_DYNAMIC=y

[Christoph Hellwig]

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.

Re: [PATCH] swiotlb: Initialise restricted pool list_head when SWIOTLB_DYNAMIC=y

[Petr Tesařík]

On Thu, 2 May 2024 14:56:01 +0200
Christoph Hellwig <hch@lst.de> 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