Messages in this thread | | | Subject | Re: [PATCH v3 5/5] iommu/uapi: Support both kernel and user unbind guest PASID | From | Lu Baolu <> | Date | Wed, 24 Jun 2020 15:55:41 +0800 |
| |
Hi Jacob,
On 2020/6/24 1:03, Jacob Pan wrote: > Guest SVA unbind data can come from either kernel and user space, if a
either kernel or user space
> user pointer is passed in, IOMMU driver must copy from data from user.
copy data from user
> If the unbind data is assembled in kernel, data can be trusted and > directly used. This patch creates a wrapper for unbind gpasid such that > user pointer can be parsed and sanitized before calling into the kernel > unbind function. Common user data copy code also consolidated. > > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > --- > drivers/iommu/iommu.c | 70 ++++++++++++++++++++++++++++++++++++++------------- > include/linux/iommu.h | 13 ++++++++-- > 2 files changed, 64 insertions(+), 19 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 4a025c429b41..595527e4c6b7 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -2010,19 +2010,15 @@ int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev, > } > EXPORT_SYMBOL_GPL(iommu_cache_invalidate); > > -int iommu_sva_bind_gpasid(struct iommu_domain *domain, struct device *dev, > - void __user *udata) > -{ > > - struct iommu_gpasid_bind_data data; > +static int iommu_sva_prepare_bind_data(void __user *udata, bool bind, > + struct iommu_gpasid_bind_data *data) > +{ > unsigned long minsz, maxsz; > > - if (unlikely(!domain->ops->sva_bind_gpasid)) > - return -ENODEV; > - > /* Current kernel data size is the max to be copied from user */ > maxsz = sizeof(struct iommu_gpasid_bind_data); > - memset((void *)&data, 0, maxsz); > + memset((void *)data, 0, maxsz); > > /* > * No new spaces can be added before the variable sized union, the > @@ -2031,11 +2027,11 @@ int iommu_sva_bind_gpasid(struct iommu_domain *domain, struct device *dev, > minsz = offsetof(struct iommu_gpasid_bind_data, vendor); > > /* Copy minsz from user to get flags and argsz */ > - if (copy_from_user(&data, udata, minsz)) > + if (copy_from_user(data, udata, minsz)) > return -EFAULT; > > /* Fields before variable size union is mandatory */ > - if (data.argsz < minsz) > + if (data->argsz < minsz) > return -EINVAL; > /* > * User might be using a newer UAPI header, we shall let IOMMU vendor > @@ -2043,26 +2039,66 @@ int iommu_sva_bind_gpasid(struct iommu_domain *domain, struct device *dev, > * can be vendor specific, larger argsz could be the result of extension > * for one vendor but it should not affect another vendor. > */ > - if (data.argsz > maxsz) > - data.argsz = maxsz; > + if (data->argsz > maxsz) > + data->argsz = maxsz; > + > + /* > + * For unbind, we don't need any extra data, host PASID is included in > + * the minsz and that is all we need. > + */ > + if (!bind) > + return 0; > > /* Copy the remaining user data _after_ minsz */ > - if (copy_from_user((void *)&data + minsz, udata + minsz, > - data.argsz - minsz)) > + if (copy_from_user((void *)data + minsz, udata + minsz, > + data->argsz - minsz)) > return -EFAULT; > > + return 0; > +} > + > +int iommu_sva_bind_gpasid(struct iommu_domain *domain, struct device *dev, > + void __user *udata) > +{ > + > + struct iommu_gpasid_bind_data data; > + int ret; > + > + if (unlikely(!domain->ops->sva_bind_gpasid)) > + return -ENODEV; > + > + ret = iommu_sva_prepare_bind_data(udata, true, &data); > + if (ret) > + return ret; > > return domain->ops->sva_bind_gpasid(domain, dev, &data); > } > EXPORT_SYMBOL_GPL(iommu_sva_bind_gpasid); > > -int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, > - ioasid_t pasid) > +int __iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, > + struct iommu_gpasid_bind_data *data) > { > if (unlikely(!domain->ops->sva_unbind_gpasid)) > return -ENODEV; > > - return domain->ops->sva_unbind_gpasid(dev, pasid); > + return domain->ops->sva_unbind_gpasid(dev, data->hpasid); How about passing @data to the vendor iommu driver as well?
> +} > +EXPORT_SYMBOL_GPL(__iommu_sva_unbind_gpasid); > + > +int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, > + void __user *udata)
Keep it aligned.
> +{ > + struct iommu_gpasid_bind_data data; > + int ret; > + > + if (unlikely(!domain->ops->sva_bind_gpasid)) > + return -ENODEV; > + > + ret = iommu_sva_prepare_bind_data(udata, false, &data); > + if (ret) > + return ret; > + > + return __iommu_sva_unbind_gpasid(domain, dev, &data); > } > EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid); > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index a688fea42ae5..2567c33dc4e8 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -437,7 +437,9 @@ extern int iommu_cache_invalidate(struct iommu_domain *domain, > extern int iommu_sva_bind_gpasid(struct iommu_domain *domain, > struct device *dev, void __user *udata); > extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain, > - struct device *dev, ioasid_t pasid); > + struct device *dev, void __user *udata); > +extern int __iommu_sva_unbind_gpasid(struct iommu_domain *domain, > + struct device *dev, struct iommu_gpasid_bind_data *data); > extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev); > extern struct iommu_domain *iommu_get_dma_domain(struct device *dev); > extern int iommu_map(struct iommu_domain *domain, unsigned long iova, > @@ -1069,7 +1071,14 @@ static inline int iommu_sva_bind_gpasid(struct iommu_domain *domain, > } > > static inline int iommu_sva_unbind_gpasid(struct iommu_domain *domain, > - struct device *dev, int pasid) > + struct device *dev, void __user *udata) > +{ > + return -ENODEV; > +} > + > +static inline int __iommu_sva_unbind_gpasid(struct iommu_domain *domain, > + struct device *dev, > + struct iommu_gpasid_bind_data *data) > { > return -ENODEV; > } >
Best regards, baolu
| |