Hi, Jean

On 2022/4/26 下午6:04, Jean-Philippe Brucker wrote:
We currently call arm64_mm_context_put() without holding a reference to
the mm, which can result in use-after-free. Call mmgrab()/mmdrop() to
ensure the mm only gets freed after we unpinned the ASID.

Fixes: 32784a9562fb ("iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()")
Signed-off-by: Jean-Philippe Brucker <[email protected]>
---
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++++++++++--
  1 file changed, 10 insertions(+), 2 deletions(-)
Missing +#include <linux/sched/mm.h> for compile.

We still need the fix move mm_pasid_drop from __mmput to __mmdrop, right?

1. Test OK with the mm_pasid_drop patch.

2. Test fail if revert the mm_pasid_drop patch,
uacce_fops_release
Unable to handle kernel paging request at virtual address ffff00083cc6ffc0

By the way, we use mmgrab in bind and mmput in unbind before,
then the fops_release is not be called if exit without close(fd).

This patch does not have this issue, still not understand why.

Thanks

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 582114f94da0..c93477a2d7f1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -98,9 +98,14 @@ static struct arm_smmu_ctx_desc 
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
        struct arm_smmu_ctx_desc *cd;
        struct arm_smmu_ctx_desc *ret = NULL;
+ /* Don't free the mm until we release the ASID */
+       mmgrab(mm);
+
        asid = arm64_mm_context_get(mm);
-       if (!asid)
-               return ERR_PTR(-ESRCH);
+       if (!asid) {
+               err = -ESRCH;
+               goto out_drop_mm;
+       }
cd = kzalloc(sizeof(*cd), GFP_KERNEL);
        if (!cd) {
@@ -167,6 +172,8 @@ static struct arm_smmu_ctx_desc 
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
        kfree(cd);
  out_put_context:
        arm64_mm_context_put(mm);
+out_drop_mm:
+       mmdrop(mm);
        return err < 0 ? ERR_PTR(err) : ret;
  }
@@ -175,6 +182,7 @@ static void arm_smmu_free_shared_cd(struct arm_smmu_ctx_desc *cd)
        if (arm_smmu_free_asid(cd)) {
                /* Unpin ASID */
                arm64_mm_context_put(cd->mm);
+               mmdrop(cd->mm);
                kfree(cd);
        }
  }

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to