On Tuesday, September 15, 2026 11:32 AM [email protected] 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 <[email protected]>
>
> 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/[email protected]?part=8
