iommu/qcom: Fix bogus detach logic

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@onstation.org>
Tested-by: Brian Masney <masneyb@onstation.org>
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu")
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Stephan Gerhold <stephan@gerhold.net>
Cc: stable@vger.kernel.org # v4.14+
Signed-off-by: Joerg Roedel <jroedel@suse.de>

diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 39759db4f0038c0ec7dd49928b75e6d89e5cae86..4328da0b0a9fdfa35b1d7f951b72e3599d195a31 100644 (file)
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -344,21 +344,19 @@ static void qcom_iommu_domain_free(struct iommu_domain *domain)
 {
        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);
 }
@@ -404,7 +402,7 @@ static void qcom_iommu_detach_dev(struct iommu_domain *domain, struct device *de
        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);
@@ -417,8 +415,6 @@ static void qcom_iommu_detach_dev(struct iommu_domain *domain, struct device *de
                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,