Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>,
	Rob Clark <robin.clark@oss.qualcomm.com>,
	Will Deacon <will@kernel.org>,
	"Joerg Roedel (AMD)" <joro@8bytes.org>
Cc: iommu@lists.linux.dev, linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/8] iommu/qcom: Add NULL ctx check in TLB invalidation paths
Date: Tue, 23 Jun 2026 18:08:08 +0100	[thread overview]
Message-ID: <3463e5cf-05d3-4e4b-ac22-699e5f95589b@arm.com> (raw)
In-Reply-To: <20260623122034.1166295-7-mukesh.ojha@oss.qualcomm.com>

On 23/06/2026 1:20 pm, Mukesh Ojha wrote:
> to_ctx() returns qcom_iommu->ctxs[asid], which can be NULL if the
> corresponding context bank failed to probe or was already removed.
> qcom_iommu_tlb_sync(), qcom_iommu_tlb_inv_context(), and
> qcom_iommu_tlb_inv_range_nosync() all dereference the returned pointer
> directly, risking a NULL pointer dereference.

But if there's no context bank, then how has a domain been allocated in 
order to permit io-pgtable operations that would eventually call into 
qcom_flush_ops at all? Can you please clarify whether you've actually 
observed a real-world issue here, and if so how?

Thanks,
Robin.

> Add WARN_ON(!ctx) guards with continue so TLB operations skip
> broken context banks without crashing.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>   drivers/iommu/arm/arm-smmu/qcom_iommu.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 40fb0408dc07..51b60b296bb8 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> @@ -121,6 +121,9 @@ static void qcom_iommu_tlb_sync(void *cookie)
>   		struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]);
>   		unsigned int val, ret;
>   
> +		if (WARN_ON(!ctx))
> +			continue;
> +
>   		iommu_writel(ctx, ARM_SMMU_CB_TLBSYNC, 0);
>   
>   		ret = readl_poll_timeout(ctx->base + ARM_SMMU_CB_TLBSTATUS, val,
> @@ -138,6 +141,10 @@ static void qcom_iommu_tlb_inv_context(void *cookie)
>   
>   	for (i = 0; i < fwspec->num_ids; i++) {
>   		struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]);
> +
> +		if (WARN_ON(!ctx))
> +			continue;
> +
>   		iommu_writel(ctx, ARM_SMMU_CB_S1_TLBIASID, ctx->asid);
>   	}
>   
> @@ -157,6 +164,9 @@ static void qcom_iommu_tlb_inv_range_nosync(unsigned long iova, size_t size,
>   		struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]);
>   		size_t s = size;
>   
> +		if (WARN_ON(!ctx))
> +			continue;
> +
>   		iova = (iova >> 12) << 12;
>   		iova |= ctx->asid;
>   		do {



  parent reply	other threads:[~2026-06-23 17:59 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-23 12:20 [PATCH 0/8] iommu/qcom: Misc Fixes Mukesh Ojha
2026-06-23 12:20 ` [PATCH 1/8] iommu/qcom: Fix inverted fault report check in qcom_iommu_fault() Mukesh Ojha
2026-06-23 16:00   ` Konrad Dybcio
2026-06-23 12:20 ` [PATCH 2/8] iommu/qcom: Fix missing pm_runtime_disable() in qcom_iommu_device_remove() Mukesh Ojha
2026-06-23 16:00   ` Konrad Dybcio
2026-06-23 12:20 ` [PATCH 3/8] iommu/qcom: Check pm_runtime_resume_and_get() return in probe Mukesh Ojha
2026-06-23 16:02   ` Konrad Dybcio
2026-06-23 12:20 ` [PATCH 4/8] iommu/qcom: Fix pgtbl_ops leak in qcom_iommu_init_domain() error path Mukesh Ojha
2026-06-23 16:09   ` Konrad Dybcio
2026-06-23 12:20 ` [PATCH 5/8] iommu/qcom: Publish pgtbl_ops before releasing init_mutex Mukesh Ojha
2026-06-23 16:15   ` Konrad Dybcio
2026-06-23 12:20 ` [PATCH 6/8] iommu/qcom: Add NULL ctx check in TLB invalidation paths Mukesh Ojha
2026-06-23 16:47   ` Konrad Dybcio
2026-06-23 17:08   ` Robin Murphy [this message]
2026-06-24 19:18     ` Mukesh Ojha
2026-06-23 12:20 ` [PATCH 7/8] iommu/qcom: Enable clocks before hardware access in qcom_iommu_ctx_probe() Mukesh Ojha
2026-06-23 16:36   ` Konrad Dybcio
2026-06-23 12:20 ` [PATCH 8/8] iommu/qcom: Document why sec_ptbl allocated flag needs no locking Mukesh Ojha
2026-06-23 16:41   ` Konrad Dybcio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3463e5cf-05d3-4e4b-ac22-699e5f95589b@arm.com \
    --to=robin.murphy@arm.com \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mukesh.ojha@oss.qualcomm.com \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox