[v2,2/8] iommu: Validate that devices match domains
Commit Message
Before we can allow drivers to coexist, we need to make sure that one
driver's domain ops can't misinterpret another driver's dev_iommu_priv
data. To that end, add a token to the domain so we can remember how it
was allocated - for now this may as well be the device ops, since they
still correlate 1:1 with drivers. We can trust ourselves for internal
default domain attachment, so add the check where it covers both the
external attach interfaces.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v2: Tweaked commit message
drivers/iommu/iommu.c | 13 +++++++++----
include/linux/iommu.h | 1 +
2 files changed, 10 insertions(+), 4 deletions(-)
Comments
On 2023/1/27 2:26, Robin Murphy wrote:
> Before we can allow drivers to coexist, we need to make sure that one
> driver's domain ops can't misinterpret another driver's dev_iommu_priv
> data. To that end, add a token to the domain so we can remember how it
> was allocated - for now this may as well be the device ops, since they
> still correlate 1:1 with drivers. We can trust ourselves for internal
> default domain attachment, so add the check where it covers both the
> external attach interfaces.
>
> Signed-off-by: Robin Murphy<robin.murphy@arm.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Best regards,
baolu
On Thu, Jan 26, 2023 at 06:26:17PM +0000, Robin Murphy wrote:
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 3589d1b8f922..86fa52025e75 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -95,6 +95,7 @@ struct iommu_domain_geometry {
> struct iommu_domain {
> unsigned type;
> const struct iommu_domain_ops *ops;
> + const struct iommu_ops *owner; /* Whose domain_alloc we came from */
For some reason I thought we were trying to save bytes in this struct?
Putting a pointer back to iommu_ops in iommu_domain_ops would allow
that, then it is just domain->ops->owner
Regardless,
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
@@ -1941,20 +1941,22 @@ EXPORT_SYMBOL_GPL(iommu_set_fault_handler);
static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
unsigned type)
{
+ const struct iommu_ops *ops = bus ? bus->iommu_ops : NULL;
struct iommu_domain *domain;
- if (bus == NULL || bus->iommu_ops == NULL)
+ if (!ops)
return NULL;
- domain = bus->iommu_ops->domain_alloc(type);
+ domain = ops->domain_alloc(type);
if (!domain)
return NULL;
domain->type = type;
+ domain->owner = ops;
/* Assume all sizes by default; the driver may override this later */
- domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
+ domain->pgsize_bitmap = ops->pgsize_bitmap;
if (!domain->ops)
- domain->ops = bus->iommu_ops->default_domain_ops;
+ domain->ops = ops->default_domain_ops;
if (iommu_is_dma_domain(domain) && iommu_get_dma_cookie(domain)) {
iommu_domain_free(domain);
@@ -2120,6 +2122,9 @@ static int iommu_group_do_attach_device(struct device *dev, void *data)
{
struct iommu_domain *domain = data;
+ if (dev_iommu_ops(dev) != domain->owner)
+ return -EINVAL;
+
return __iommu_attach_device(domain, dev);
}
@@ -95,6 +95,7 @@ struct iommu_domain_geometry {
struct iommu_domain {
unsigned type;
const struct iommu_domain_ops *ops;
+ const struct iommu_ops *owner; /* Whose domain_alloc we came from */
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
struct iommu_domain_geometry geometry;
struct iommu_dma_cookie *iova_cookie;