On 16/04/2021 04:30, chenxiang (M) wrote:
you need to make this: if (iova) free_iova_mem(iova);
as free_iova_mem(iova) dereferences iova:
if (iova->pfn_lo != IOVA_ANCHOR) kmem_cache_free(iova_cache, iova)
So if iova were NULL, we crash.
Or you can make free_iova_mem() NULL safe.
Right, it has the issue. What about changing it as below?
@@ -472,10 +472,13 @@ free_iova(struct iova_domain *iovad, unsigned long pfn)
spin_lock_irqsave(&iovad->iova_rbtree_lock, flags); iova = private_find_iova(iovad, pfn); - if (iova) - private_free_iova(iovad, iova); + if (!iova) {
- spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
+ return; + } + remove_iova(iovad, iova); spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
+ free_iova_mem(iova); } EXPORT_SYMBOL_GPL(free_iova);
I don't feel too strongly about how it's done.
Please note that kmem_cache_free() is not NULL safe. So the NULL check could be added in free_iova_mem(), but we prob don't want silent free_iova_mem(NULL) calls, so I would stick with changing free_iova().
Thanks, John