Re: [PATCH v3 5/5] iommu/uapi: Support both kernel and user unbind guest PASID
From: Lu Baolu
Date: Wed Jun 24 2020 - 03:55:48 EST
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@xxxxxxxxx>
Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
---
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