Re: [PATCH 08/11] iommu/tegra-smmu: Defer display client translation to driver handover

[Mikko Perttunen <mperttunen@nvidia.com>]

On Tuesday, September 15, 2026 11:32 AM sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] iommu/tegra-smmu: type confusion in tegra_smmu_enable_translation() on ARM SMMU
> --
> 
> commit 35f909bd1b8b88cf1f8f29602efafacdd3b66778
> Author: Mikko Perttunen <mperttunen@nvidia.com>
> 
> iommu/tegra-smmu: Defer display client translation to driver handover
> 
> Bootloaders may leave display controllers scanning out a boot splash
> while the kernel boots. Enabling translation for them without special
> care during that time would cause translation faults. This patch
> defers enabling translation until the display controller is reset.
> 
> > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > index 0b3fcc7011b35..2ae4da07bbc99 100644
> > --- a/drivers/gpu/drm/tegra/dc.c
> > +++ b/drivers/gpu/drm/tegra/dc.c
> [ ... ]
> > @@ -2751,6 +2752,13 @@ static int tegra_dc_init(struct host1x_client *client)
> >  		return err;
> >  	}
> >  
> > +	/*
> > +	 * Enable translation now that the final domain is attached.
> > +	 * The hardware was reset at probe and has not been programmed
> > +	 * since.
> > +	 */
> > +	tegra_smmu_enable_translation(client->dev);
> 
> [Severity: Critical]
> Does this code call the Tegra SMMU API unconditionally on all Tegra display
> controllers? Newer SoC generations (Tegra186 and later) use the ARM SMMU
> instead of the Tegra SMMU, which means this could pass a device managed by
> the ARM SMMU driver into a Tegra SMMU specific function.

Thanks, will fix.

Mikko

> 
> [ ... ]
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> > index 67e7a7b925f0c..6636a044905f6 100644
> > --- a/drivers/iommu/tegra-smmu.c
> > +++ b/drivers/iommu/tegra-smmu.c
> [ ... ]
> > @@ -417,6 +423,49 @@ static void tegra_smmu_disable(struct tegra_smmu *smmu, unsigned int swgroup,
> >  	}
> >  }
> >  
> > +/**
> > + * tegra_smmu_enable_translation - Release a client's swgroups to translation
> > + * @dev: memory client device, already quiesced by its driver
> > + *
> > + * Enables translation for swgroups which were left in bypass at attach time
> > + * because the client could still have been running from a pre-kernel
> > + * configuration. Must be called after @dev has been attached to the domain it
> > + * will use. Safe to call for any device; clients which were not deferred are
> > + * unaffected.
> > + */
> > +void tegra_smmu_enable_translation(struct device *dev)
> > +{
> > +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > +	struct tegra_smmu *smmu = dev_iommu_priv_get(dev);
> 
> [Severity: Critical]
> Is it safe to blindly retrieve the IOMMU private data via
> dev_iommu_priv_get() and cast it to a struct tegra_smmu pointer without
> validating that the device is actually managed by the Tegra SMMU driver?
> 
> On ARM SMMU systems, this private data actually points to a completely
> different structure (like struct arm_smmu_master_cfg or
> struct arm_smmu_master) that is significantly smaller.
> 
> > +	struct iommu_domain *domain;
> > +	unsigned int i;
> > +
> > +	if (!fwspec || !smmu)
> > +		return;
> > +
> > +	domain = iommu_get_domain_for_dev(dev);
> > +	if (!domain || !(domain->type & __IOMMU_DOMAIN_PAGING))
> > +		return;
> > +
> > +	mutex_lock(&smmu->lock);
> 
> [Severity: Critical]
> Because lock is at a large offset in struct tegra_smmu, could dereferencing
> this type-confused pointer to acquire the mutex access memory out of bounds
> of the ARM SMMU configuration structure? This looks like it would lead to
> heap corruption and a kernel oops during the display controller probe path.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260915-b4-arm32-iommu-dma-cookie-v1-0-ba4f9d33a2b8@nvidia.com?part=8