在 2021/4/16 16:53, John Garry 写道:
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().
so I would stick with changing free_iova()
----- Do you mean free_iova_mem()?
But i think there is a check (judge iova is NULL) before free_iova_mem()
for following scenarios, and it is not necessary to change in funciton
free_iova_mem():
1) iova_magazine_free_pfns()
2) alloc_iova()
3) free_iova()
4) __free_iova(): Those functions which call __free_iova() have a check
Only for function put_iova_doamin(), it doesn't check iova, but i think
iova in rbtree should not be NULL.
Thanks,
John
.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
