On Tue, Apr 26, 2022 at 08:20:52PM +0800, [email protected] wrote:
> 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.
Ah thanks, I sent the wrong version.
> We still need the fix move mm_pasid_drop from __mmput to __mmdrop, right?
Yes, that's Fenghua's patch
Thanks,
Jean
>
> 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
