Commit faf305c5 authored by Robin Murphy's avatar Robin Murphy Committed by Joerg Roedel

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: default avatarBrian Masney <masneyb@onstation.org>
Tested-by: default avatarBrian Masney <masneyb@onstation.org>
Reported-by: default avatarNaresh Kamboju <naresh.kamboju@linaro.org>
Fixes: 0ae349a0 ("iommu/qcom: Add qcom_iommu")
Signed-off-by: default avatarRobin Murphy <robin.murphy@arm.com>
Tested-by: default avatarStephan Gerhold <stephan@gerhold.net>
Cc: stable@vger.kernel.org # v4.14+
Signed-off-by: default avatarJoerg Roedel <jroedel@suse.de>
parent 3dfee47b
...@@ -344,21 +344,19 @@ static void qcom_iommu_domain_free(struct iommu_domain *domain) ...@@ -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); 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); iommu_put_dma_cookie(domain);
/* NOTE: unmap can be called after client device is powered off, if (qcom_domain->iommu) {
* for example, with GPUs or anything involving dma-buf. So we /*
* cannot rely on the device_link. Make sure the IOMMU is on to * NOTE: unmap can be called after client device is powered
* avoid unclocked accesses in the TLB inv path: * 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); pm_runtime_get_sync(qcom_domain->iommu->dev);
free_io_pgtable_ops(qcom_domain->pgtbl_ops); free_io_pgtable_ops(qcom_domain->pgtbl_ops);
pm_runtime_put_sync(qcom_domain->iommu->dev); pm_runtime_put_sync(qcom_domain->iommu->dev);
}
kfree(qcom_domain); kfree(qcom_domain);
} }
...@@ -404,7 +402,7 @@ static void qcom_iommu_detach_dev(struct iommu_domain *domain, struct device *de ...@@ -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); struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
unsigned i; unsigned i;
if (!qcom_domain->iommu) if (WARN_ON(!qcom_domain->iommu))
return; return;
pm_runtime_get_sync(qcom_iommu->dev); pm_runtime_get_sync(qcom_iommu->dev);
...@@ -417,8 +415,6 @@ static void qcom_iommu_detach_dev(struct iommu_domain *domain, struct device *de ...@@ -417,8 +415,6 @@ static void qcom_iommu_detach_dev(struct iommu_domain *domain, struct device *de
ctx->domain = NULL; ctx->domain = NULL;
} }
pm_runtime_put_sync(qcom_iommu->dev); pm_runtime_put_sync(qcom_iommu->dev);
qcom_domain->iommu = NULL;
} }
static int qcom_iommu_map(struct iommu_domain *domain, unsigned long iova, static int qcom_iommu_map(struct iommu_domain *domain, unsigned long iova,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment