* [PATCH] iommu/arm-smmu: pass smmu->dev to report_iommu_fault
@ 2026-05-17 0:50 Shyam Saini
2026-05-18 17:00 ` Robin Murphy
0 siblings, 1 reply; 3+ messages in thread
From: Shyam Saini @ 2026-05-17 0:50 UTC (permalink / raw)
To: iommu
Cc: linux-arm-kernel, linux-arm-msm, robin.clark, will, robin.murphy,
joro, stable
report_iommu_fault() passes the dev argument to trace_io_page_fault(),
which dereferences it via dev_name() and dev_driver_string(). Passing
NULL causes a kernel crash when the io_page_fault tracepoint is
enabled.
In arm-smmu.c, 'commit f8f934c180f6 ("iommu/arm-smmu: Add support for driver IOMMU fault handlers")'
replaced a dev_err_ratelimited() call that correctly used smmu->dev with
report_iommu_fault() but passed NULL instead.
In arm-smmu-qcom-debug.c, 'commit d374555ef993 ("iommu/arm-smmu-qcom: Use a custom context fault handler for sdm845")'
introduced two report_iommu_fault() calls also with NULL.
Pass smmu->dev to all three call sites.
Fixes: f8f934c180f629bb ("iommu/arm-smmu: Add support for driver IOMMU fault handlers")
Fixes: d374555ef993433f ("iommu/arm-smmu-qcom: Use a custom context fault handler for sdm845")
Cc: stable@vger.kernel.org
Assisted-by: GitHub_Copilot:claude-opus-4.6
Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c | 4 ++--
drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
index 65e0ef6539fe7..8eb9f7831de07 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
@@ -399,7 +399,7 @@ irqreturn_t qcom_smmu_context_fault(int irq, void *dev)
return IRQ_NONE;
if (list_empty(&tbu_list)) {
- ret = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
+ ret = report_iommu_fault(&smmu_domain->domain, smmu->dev, cfi.iova,
cfi.fsynr & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
if (ret == -ENOSYS)
@@ -417,7 +417,7 @@ irqreturn_t qcom_smmu_context_fault(int irq, void *dev)
phys_soft = ops->iova_to_phys(ops, cfi.iova);
- tmp = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
+ tmp = report_iommu_fault(&smmu_domain->domain, smmu->dev, cfi.iova,
cfi.fsynr & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
if (!tmp || tmp == -EBUSY) {
ret = IRQ_HANDLED;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 0bd21d206eb3e..92d8fa2100adb 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -467,7 +467,7 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
if (!(cfi.fsr & ARM_SMMU_CB_FSR_FAULT))
return IRQ_NONE;
- ret = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
+ ret = report_iommu_fault(&smmu_domain->domain, smmu->dev, cfi.iova,
cfi.fsynr & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
if (ret == -ENOSYS && __ratelimit(&rs))
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] iommu/arm-smmu: pass smmu->dev to report_iommu_fault
2026-05-17 0:50 [PATCH] iommu/arm-smmu: pass smmu->dev to report_iommu_fault Shyam Saini
@ 2026-05-18 17:00 ` Robin Murphy
2026-06-02 10:25 ` Will Deacon
0 siblings, 1 reply; 3+ messages in thread
From: Robin Murphy @ 2026-05-18 17:00 UTC (permalink / raw)
To: Shyam Saini, iommu
Cc: linux-arm-kernel, linux-arm-msm, robin.clark, will, joro, stable
On 17/05/2026 1:50 am, Shyam Saini wrote:
> report_iommu_fault() passes the dev argument to trace_io_page_fault(),
> which dereferences it via dev_name() and dev_driver_string(). Passing
> NULL causes a kernel crash when the io_page_fault tracepoint is
> enabled.
>
> In arm-smmu.c, 'commit f8f934c180f6 ("iommu/arm-smmu: Add support for driver IOMMU fault handlers")'
> replaced a dev_err_ratelimited() call that correctly used smmu->dev with
I'm not sure it was really correct - it's pretty clear that "dev" is
intended to be the client device that _caused_ the fault, since why
would it make any sense to pass the IOMMU device to some other
driver/subsystem's fault handler? (Yes, other IOMMU drivers already do
that; I would consider them just as wrong too).
IMO it would seem more robust to just fix the tracepoint to handle a
NULL "dev" in the case that one can't (easily) be identified.
Thanks,
Robin.
> report_iommu_fault() but passed NULL instead.
> In arm-smmu-qcom-debug.c, 'commit d374555ef993 ("iommu/arm-smmu-qcom: Use a custom context fault handler for sdm845")'
> introduced two report_iommu_fault() calls also with NULL.
>
> Pass smmu->dev to all three call sites.
>
> Fixes: f8f934c180f629bb ("iommu/arm-smmu: Add support for driver IOMMU fault handlers")
> Fixes: d374555ef993433f ("iommu/arm-smmu-qcom: Use a custom context fault handler for sdm845")
> Cc: stable@vger.kernel.org
> Assisted-by: GitHub_Copilot:claude-opus-4.6
> Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c | 4 ++--
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
> index 65e0ef6539fe7..8eb9f7831de07 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
> @@ -399,7 +399,7 @@ irqreturn_t qcom_smmu_context_fault(int irq, void *dev)
> return IRQ_NONE;
>
> if (list_empty(&tbu_list)) {
> - ret = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
> + ret = report_iommu_fault(&smmu_domain->domain, smmu->dev, cfi.iova,
> cfi.fsynr & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
>
> if (ret == -ENOSYS)
> @@ -417,7 +417,7 @@ irqreturn_t qcom_smmu_context_fault(int irq, void *dev)
>
> phys_soft = ops->iova_to_phys(ops, cfi.iova);
>
> - tmp = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
> + tmp = report_iommu_fault(&smmu_domain->domain, smmu->dev, cfi.iova,
> cfi.fsynr & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
> if (!tmp || tmp == -EBUSY) {
> ret = IRQ_HANDLED;
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 0bd21d206eb3e..92d8fa2100adb 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -467,7 +467,7 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
> if (!(cfi.fsr & ARM_SMMU_CB_FSR_FAULT))
> return IRQ_NONE;
>
> - ret = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
> + ret = report_iommu_fault(&smmu_domain->domain, smmu->dev, cfi.iova,
> cfi.fsynr & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
>
> if (ret == -ENOSYS && __ratelimit(&rs))
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] iommu/arm-smmu: pass smmu->dev to report_iommu_fault
2026-05-18 17:00 ` Robin Murphy
@ 2026-06-02 10:25 ` Will Deacon
0 siblings, 0 replies; 3+ messages in thread
From: Will Deacon @ 2026-06-02 10:25 UTC (permalink / raw)
To: Robin Murphy
Cc: Shyam Saini, iommu, linux-arm-kernel, linux-arm-msm, robin.clark,
joro, stable
On Mon, May 18, 2026 at 06:00:14PM +0100, Robin Murphy wrote:
> On 17/05/2026 1:50 am, Shyam Saini wrote:
> > report_iommu_fault() passes the dev argument to trace_io_page_fault(),
> > which dereferences it via dev_name() and dev_driver_string(). Passing
> > NULL causes a kernel crash when the io_page_fault tracepoint is
> > enabled.
> >
> > In arm-smmu.c, 'commit f8f934c180f6 ("iommu/arm-smmu: Add support for driver IOMMU fault handlers")'
> > replaced a dev_err_ratelimited() call that correctly used smmu->dev with
>
> I'm not sure it was really correct - it's pretty clear that "dev" is
> intended to be the client device that _caused_ the fault, since why would it
> make any sense to pass the IOMMU device to some other driver/subsystem's
> fault handler? (Yes, other IOMMU drivers already do that; I would consider
> them just as wrong too).
>
> IMO it would seem more robust to just fix the tracepoint to handle a NULL
> "dev" in the case that one can't (easily) be identified.
Yeah, I agree. Passing the SMMU device just because it's the only thing
we have is just a bodge around the NULL dereference. We'd be better off
hacking include/trace/events/iommu.h to print placeholder "(NULL)"
entries when the device pointer is NULL.
Will
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-02 10:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 0:50 [PATCH] iommu/arm-smmu: pass smmu->dev to report_iommu_fault Shyam Saini
2026-05-18 17:00 ` Robin Murphy
2026-06-02 10:25 ` Will Deacon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox