Re: [PATCH] iommu/arm-smmu: pass smmu->dev to report_iommu_fault

[Robin Murphy <robin.murphy@arm.com>]

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))