| From 4d9b37f139f1e1f95c6895816917e480dd0151c9 Mon Sep 17 00:00:00 2001 |
| From: Robin Murphy <robin.murphy@arm.com> |
| Date: Tue, 18 Feb 2020 18:12:41 +0000 |
| Subject: [PATCH] iommu/qcom: Fix bogus detach logic |
| |
| 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@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> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c |
| index 34d0b9783b3e..2424d97fc49e 100644 |
| --- a/drivers/iommu/qcom_iommu.c |
| +++ b/drivers/iommu/qcom_iommu.c |
| @@ -322,21 +322,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); |
| } |
| @@ -382,7 +380,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); |
| @@ -395,8 +393,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, |
| -- |
| 2.7.4 |
| |