| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1592931837-58223-5-git-send-email-jacob.jun.pan@linux.intel.com>
Date: Tue, 23 Jun 2020 10:03:56 -0700
From: Jacob Pan <jacob.jun.pan@...ux.intel.com>
To: iommu@...ts.linux-foundation.org,
LKML <linux-kernel@...r.kernel.org>,
Joerg Roedel <joro@...tes.org>,
Alex Williamson <alex.williamson@...hat.com>
Cc: "Lu Baolu" <baolu.lu@...ux.intel.com>,
David Woodhouse <dwmw2@...radead.org>,
Yi Liu <yi.l.liu@...el.com>,
"Tian, Kevin" <kevin.tian@...el.com>,
Raj Ashok <ashok.raj@...el.com>,
"Christoph Hellwig" <hch@...radead.org>,
Jean-Philippe Brucker <jean-philippe@...aro.com>,
Eric Auger <eric.auger@...hat.com>,
Jonathan Corbet <corbet@....net>,
Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: [PATCH v3 4/5] iommu/uapi: Handle data and argsz filled by users
IOMMU UAPI data has a user filled argsz field which indicates the data
length comes with the API call. User data is not trusted, argsz must be
validated based on the current kernel data size, mandatory data size,
and feature flags.
User data may also be extended, results in possible argsz increase.
Backward compatibility is ensured based on size and flags checking.
Details are documented in Documentation/userspace-api/iommu.rst
This patch adds sanity checks in both IOMMU layer and vendor code, where
VT-d is the only user for now.
Signed-off-by: Liu Yi L <yi.l.liu@...el.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@...ux.intel.com>
---
drivers/iommu/intel/svm.c | 3 ++
drivers/iommu/iommu.c | 96 ++++++++++++++++++++++++++++++++++++++++++++---
include/linux/iommu.h | 7 ++--
3 files changed, 98 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 713b3a218483..237db56878c0 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -244,6 +244,9 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
return -EINVAL;
+ if (data->argsz != offsetofend(struct iommu_gpasid_bind_data, vendor.vtd))
+ return -EINVAL;
+
if (!dev_is_pci(dev))
return -ENOTSUPP;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d43120eb1dc5..4a025c429b41 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1951,22 +1951,108 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
EXPORT_SYMBOL_GPL(iommu_attach_device);
int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
- struct iommu_cache_invalidate_info *inv_info)
+ void __user *uinfo)
{
+ struct iommu_cache_invalidate_info inv_info;
+ unsigned long minsz, maxsz;
+
if (unlikely(!domain->ops->cache_invalidate))
return -ENODEV;
- return domain->ops->cache_invalidate(domain, dev, inv_info);
+ /* Current kernel data size is the max to be copied from user */
+ maxsz = sizeof(struct iommu_cache_invalidate_info);
+ memset((void *)&inv_info, 0, maxsz);
+
+ /*
+ * No new spaces can be added before the variable sized union, the
+ * minimum size is the offset to the union.
+ */
+ minsz = offsetof(struct iommu_cache_invalidate_info, granu);
+
+ /* Copy minsz from user to get flags and argsz */
+ if (copy_from_user(&inv_info, uinfo, minsz))
+ return -EFAULT;
+
+ /* Fields before variable size union is mandatory */
+ if (inv_info.argsz < minsz)
+ return -EINVAL;
+ /*
+ * User might be using a newer UAPI header, we shall let IOMMU vendor
+ * driver decide on what size it needs. Since the UAPI data extension
+ * can be vendor specific, larger argsz could be the result of extension
+ * for one vendor but it should not affect another vendor.
+ */
+ /*
+ * User might be using a newer UAPI header which has a larger data
+ * size, we shall support the existing flags within the current
+ * size.
+ */
+ if (inv_info.argsz > maxsz)
+ inv_info.argsz = maxsz;
+
+ /* Checking the exact argsz based on generic flags */
+ if (inv_info.granularity == IOMMU_INV_GRANU_ADDR &&
+ inv_info.argsz != offsetofend(struct iommu_cache_invalidate_info,
+ granu.addr_info))
+ return -EINVAL;
+
+ if (inv_info.granularity == IOMMU_INV_GRANU_PASID &&
+ inv_info.argsz != offsetofend(struct iommu_cache_invalidate_info,
+ granu.pasid_info))
+ return -EINVAL;
+
+ /* Copy the remaining user data _after_ minsz */
+ if (copy_from_user((void *)&inv_info + minsz, uinfo + minsz,
+ inv_info.argsz - minsz))
+ return -EFAULT;
+
+ return domain->ops->cache_invalidate(domain, dev, &inv_info);
}
EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
-int iommu_sva_bind_gpasid(struct iommu_domain *domain,
- struct device *dev, struct iommu_gpasid_bind_data *data)
+int iommu_sva_bind_gpasid(struct iommu_domain *domain, struct device *dev,
+ void __user *udata)
{
+
+ struct iommu_gpasid_bind_data data;
+ unsigned long minsz, maxsz;
+
if (unlikely(!domain->ops->sva_bind_gpasid))
return -ENODEV;
- return domain->ops->sva_bind_gpasid(domain, dev, data);
+ /* Current kernel data size is the max to be copied from user */
+ maxsz = sizeof(struct iommu_gpasid_bind_data);
+ memset((void *)&data, 0, maxsz);
+
+ /*
+ * No new spaces can be added before the variable sized union, the
+ * minimum size is the offset to the union.
+ */
+ minsz = offsetof(struct iommu_gpasid_bind_data, vendor);
+
+ /* Copy minsz from user to get flags and argsz */
+ if (copy_from_user(&data, udata, minsz))
+ return -EFAULT;
+
+ /* Fields before variable size union is mandatory */
+ if (data.argsz < minsz)
+ return -EINVAL;
+ /*
+ * User might be using a newer UAPI header, we shall let IOMMU vendor
+ * driver decide on what size it needs. Since the guest PASID bind data
+ * 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;
+
+ /* Copy the remaining user data _after_ minsz */
+ if (copy_from_user((void *)&data + minsz, udata + minsz,
+ data.argsz - minsz))
+ return -EFAULT;
+
+
+ return domain->ops->sva_bind_gpasid(domain, dev, &data);
}
EXPORT_SYMBOL_GPL(iommu_sva_bind_gpasid);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5f0b7859d2eb..a688fea42ae5 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -432,9 +432,10 @@ extern void iommu_detach_device(struct iommu_domain *domain,
struct device *dev);
extern int iommu_cache_invalidate(struct iommu_domain *domain,
struct device *dev,
- struct iommu_cache_invalidate_info *inv_info);
+ void __user *uinfo);
+
extern int iommu_sva_bind_gpasid(struct iommu_domain *domain,
- struct device *dev, struct iommu_gpasid_bind_data *data);
+ struct device *dev, void __user *udata);
extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain,
struct device *dev, ioasid_t pasid);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
@@ -1062,7 +1063,7 @@ iommu_cache_invalidate(struct iommu_domain *domain,
return -ENODEV;
}
static inline int iommu_sva_bind_gpasid(struct iommu_domain *domain,
- struct device *dev, struct iommu_gpasid_bind_data *data)
+ struct device *dev, void __user *udata)
{
return -ENODEV;
}
--
2.7.4
Powered by blists - more mailing lists