[PATCH 5.5 001/150] iommu/qcom: Fix bogus detach logic

From: Greg Kroah-Hartman
Date: Thu Feb 27 2020 - 09:13:03 EST


From: Robin Murphy <robin.murphy@xxxxxxx>

commit faf305c51aeabd1ea2d7131e798ef5f55f4a7750 upstream.

Currently, the implementation of qcom_iommu_domain_free() is guaranteed
to do one of two things: WARN() and leak everything, or dereference NULL
and crash. That alone is terrible, but in fact the whole idea of trying
to track the liveness of a domain via the qcom_domain->iommu pointer as
a sanity check is full of fundamentally flawed assumptions. Make things
robust and actually functional by not trying to be quite so clever.

Reported-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
Tested-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
Reported-by: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx>
Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu")
Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
Tested-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx # v4.14+
Signed-off-by: Joerg Roedel <jroedel@xxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
drivers/iommu/qcom_iommu.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)

--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -345,21 +345,19 @@ static void qcom_iommu_domain_free(struc
{
struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);

- if (WARN_ON(qcom_domain->iommu)) /* forgot to detach? */
- return;
-
iommu_put_dma_cookie(domain);

- /* NOTE: unmap can be called after client device is powered off,
- * for example, with GPUs or anything involving dma-buf. So we
- * cannot rely on the device_link. Make sure the IOMMU is on to
- * avoid unclocked accesses in the TLB inv path:
- */
- pm_runtime_get_sync(qcom_domain->iommu->dev);
-
- free_io_pgtable_ops(qcom_domain->pgtbl_ops);
-
- pm_runtime_put_sync(qcom_domain->iommu->dev);
+ if (qcom_domain->iommu) {
+ /*
+ * NOTE: unmap can be called after client device is powered
+ * off, for example, with GPUs or anything involving dma-buf.
+ * So we cannot rely on the device_link. Make sure the IOMMU
+ * is on to avoid unclocked accesses in the TLB inv path:
+ */
+ pm_runtime_get_sync(qcom_domain->iommu->dev);
+ free_io_pgtable_ops(qcom_domain->pgtbl_ops);
+ pm_runtime_put_sync(qcom_domain->iommu->dev);
+ }

kfree(qcom_domain);
}
@@ -405,7 +403,7 @@ static void qcom_iommu_detach_dev(struct
struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
unsigned i;

- if (!qcom_domain->iommu)
+ if (WARN_ON(!qcom_domain->iommu))
return;

pm_runtime_get_sync(qcom_iommu->dev);
@@ -418,8 +416,6 @@ static void qcom_iommu_detach_dev(struct
ctx->domain = NULL;
}
pm_runtime_put_sync(qcom_iommu->dev);
-
- qcom_domain->iommu = NULL;
}

static int qcom_iommu_map(struct iommu_domain *domain, unsigned long iova,