1. Patchew
  2. linux
  3. View series

[PATCH v2] iommu/iova: Add NULL check in iova_magazine_free()

Lynn Liu posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/iommu/iova.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH v2] iommu/iova: Add NULL check in iova_magazine_free()
Posted by Lynn Liu 1 month, 1 week ago 
When iova_domain_init_rcaches() fails to allocate an iova_magazine
during the initialization of per-cpu rcaches, it jumps to out_err and
calls free_iova_rcaches() for cleanup.

In free_iova_rcaches(), the code iterates through all possible CPUs to
free both cpu_rcache->loaded and cpu_rcache->prev. However, if the
original allocation failed mid-way through the CPU loop, the pointers
for the remaining CPUs remain NULL.

Since kmem_cache_free() does not explicitly handle NULL pointers like
kfree() does, passing these NULL pointers leads to a kernel paging
request fault.

The crash can be observed when fault injection triggers a failure in
iova_magazine_alloc():

pc : _compound_head include/linux/page-flags.h:251 [inline]
pc : virt_to_folio include/linux/mm.h:1395 [inline]
pc : virt_to_slab mm/slab.h:215 [inline]
pc : kmem_cache_free+0x90/0x5c4 mm/slub.c:4769
...
Call trace:
 trace_kmem_cache_free include/trace/events/kmem.h:114 [inline]
 kmem_cache_free+0x90/0x5c4 mm/slub.c:4768
 iova_magazine_free drivers/iommu/iova.c:623 [inline]
 free_iova_rcaches+0x114/0x344 drivers/iommu/iova.c:894
 iova_domain_init_rcaches drivers/iommu/iova.c:762 [inline]
 iova_domain_init_rcaches+0x3d0/0x528 drivers/iommu/iova.c:717

Add a NULL check in iova_magazine_free() to safely handle partially
initialized rcaches in error paths.

Fixes: 84e6f56be9c6 ("iommu/iova: use named kmem_cache for iova magazines")
Signed-off-by: Lynn Liu <liulynn@google.com>
---
v1: https://lore.kernel.org/linux-iommu/20260214080919.390846-1-liulynn@google.com/T/#u
Changelog in v1 -> v2:
 - Update author name to full name.
 - Add Fixes tag.
 - Include the Call trace from the kernel crash.

 drivers/iommu/iova.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 18f839721813..e026be5e068b 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -611,7 +611,8 @@ static struct iova_magazine *iova_magazine_alloc(gfp_t flags)
 
 static void iova_magazine_free(struct iova_magazine *mag)
 {
-	kmem_cache_free(iova_magazine_cache, mag);
+	if (mag)
+		kmem_cache_free(iova_magazine_cache, mag);
 }
 
 static void
-- 
2.53.0.473.g4a7958ca14-goog
Re: [PATCH v2] iommu/iova: Add NULL check in iova_magazine_free()
Posted by Robin Murphy 1 month, 1 week ago 
On 2026-03-04 8:06 am, Lynn Liu wrote:
> When iova_domain_init_rcaches() fails to allocate an iova_magazine
> during the initialization of per-cpu rcaches, it jumps to out_err and
> calls free_iova_rcaches() for cleanup.
> 
> In free_iova_rcaches(), the code iterates through all possible CPUs to
> free both cpu_rcache->loaded and cpu_rcache->prev. However, if the
> original allocation failed mid-way through the CPU loop, the pointers
> for the remaining CPUs remain NULL.
> 
> Since kmem_cache_free() does not explicitly handle NULL pointers like
> kfree() does, passing these NULL pointers leads to a kernel paging
> request fault.
> 
> The crash can be observed when fault injection triggers a failure in
> iova_magazine_alloc():
> 
> pc : _compound_head include/linux/page-flags.h:251 [inline]
> pc : virt_to_folio include/linux/mm.h:1395 [inline]
> pc : virt_to_slab mm/slab.h:215 [inline]
> pc : kmem_cache_free+0x90/0x5c4 mm/slub.c:4769
> ...
> Call trace:
>   trace_kmem_cache_free include/trace/events/kmem.h:114 [inline]
>   kmem_cache_free+0x90/0x5c4 mm/slub.c:4768
>   iova_magazine_free drivers/iommu/iova.c:623 [inline]
>   free_iova_rcaches+0x114/0x344 drivers/iommu/iova.c:894
>   iova_domain_init_rcaches drivers/iommu/iova.c:762 [inline]
>   iova_domain_init_rcaches+0x3d0/0x528 drivers/iommu/iova.c:717
> 
> Add a NULL check in iova_magazine_free() to safely handle partially
> initialized rcaches in error paths.

Wouldn't it be even better to just break the for_each_possible_cpu() 
loop in free_iova_rcaches() on the first NULL, much like the outer 
rcache->cpu_rcaches condition? Since the cleanup is already trying to be 
symmetric to the allocation, I'd rather fix that than add a blanket 
check that's redundant for every other caller.

Thanks,
Robin.

> Fixes: 84e6f56be9c6 ("iommu/iova: use named kmem_cache for iova magazines")
> Signed-off-by: Lynn Liu <liulynn@google.com>
> ---
> v1: https://lore.kernel.org/linux-iommu/20260214080919.390846-1-liulynn@google.com/T/#u
> Changelog in v1 -> v2:
>   - Update author name to full name.
>   - Add Fixes tag.
>   - Include the Call trace from the kernel crash.
> 
>   drivers/iommu/iova.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 18f839721813..e026be5e068b 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -611,7 +611,8 @@ static struct iova_magazine *iova_magazine_alloc(gfp_t flags)
>   
>   static void iova_magazine_free(struct iova_magazine *mag)
>   {
> -	kmem_cache_free(iova_magazine_cache, mag);
> +	if (mag)
> +		kmem_cache_free(iova_magazine_cache, mag);
>   }
>   
>   static void
[PATCH v3] iommu/iova: Add NULL check in iova_magazine_free()
Posted by Lynn Liu 1 month ago 
In iova_domain_init_rcaches(), when an iova_magazine allocation fails,
the code jumps to the error path and calls free_iova_rcaches().

In free_iova_rcaches(), the code iterates through all possible CPUs to
free both cpu_rcache->loaded and cpu_rcache->prev. However, if the
original allocation failed mid-way through the CPU loop, the pointers
for the remaining CPUs remain NULL. free_iova_rcaches() then blindly
attempts to free these NULL pointers, which causes a kernel paging
request fault in kmem_cache_free().

The crash can be observed when fault injection triggers a failure in
iova_magazine_alloc():

pc : _compound_head include/linux/page-flags.h:251 [inline]
pc : virt_to_folio include/linux/mm.h:1395 [inline]
pc : virt_to_slab mm/slab.h:215 [inline]
pc : kmem_cache_free+0x90/0x5c4 mm/slub.c:4769
...
Call trace:
 trace_kmem_cache_free include/trace/events/kmem.h:114 [inline]
 kmem_cache_free+0x90/0x5c4 mm/slub.c:4768
 iova_magazine_free drivers/iommu/iova.c:623 [inline]
 free_iova_rcaches+0x114/0x344 drivers/iommu/iova.c:894
 iova_domain_init_rcaches drivers/iommu/iova.c:762 [inline]
 iova_domain_init_rcaches+0x3d0/0x528 drivers/iommu/iova.c:717

These changes update iova_domain_init_rcaches() to check for allocation
failure immediately after each individual magazine allocation.
Correspondingly, update free_iova_rcaches() to break the cleanup loop
when it encounters a NULL pointer, ensuring only successfully allocated
resources are processed.

Fixes: 84e6f56be9c6 ("iommu/iova: use named kmem_cache for iova magazines")
Signed-off-by: Lynn Liu <liulynn@google.com>
---
v3:
 - Split magazine allocation checks in iova_domain_init_rcaches().
 - Add early breaks in free_iova_rcaches() to safely handle partial allocations as suggested.
 Link to v2: https://lore.kernel.org/linux-iommu/116b79a2-fadb-4796-8cb7-9c2dfde54999@arm.com/T/#t

v2:
 - Update author name to full name.
 - Add Fixes tag.
 - Include the Call trace from the kernel crash.
 Link to v1: https://lore.kernel.org/linux-iommu/20260214080919.390846-1-liulynn@google.com/T/#u

 drivers/iommu/iova.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 18f839721813..cf6cd9f23b97 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -740,8 +740,12 @@ int iova_domain_init_rcaches(struct iova_domain *iovad)
 
 			spin_lock_init(&cpu_rcache->lock);
 			cpu_rcache->loaded = iova_magazine_alloc(GFP_KERNEL);
+			if (!cpu_rcache->loaded) {
+				ret = -ENOMEM;
+				goto out_err;
+			}
 			cpu_rcache->prev = iova_magazine_alloc(GFP_KERNEL);
-			if (!cpu_rcache->loaded || !cpu_rcache->prev) {
+			if (!cpu_rcache->prev) {
 				ret = -ENOMEM;
 				goto out_err;
 			}
@@ -886,7 +890,11 @@ static void free_iova_rcaches(struct iova_domain *iovad)
 			break;
 		for_each_possible_cpu(cpu) {
 			cpu_rcache = per_cpu_ptr(rcache->cpu_rcaches, cpu);
+			if (!cpu_rcache->loaded)
+				break;
 			iova_magazine_free(cpu_rcache->loaded);
+			if (!cpu_rcache->prev)
+				break;
 			iova_magazine_free(cpu_rcache->prev);
 		}
 		free_percpu(rcache->cpu_rcaches);
-- 
2.53.0.473.g4a7958ca14-goog

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.