Re: [PATCH v3 6.6] iommu: Handle race with default domain setup

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Apr 29, 2025 at 02:07:19PM +0100, Robin Murphy wrote:
> On 29/04/2025 2:00 pm, Greg KH wrote:
> > On Tue, Apr 29, 2025 at 11:47:40AM +0100, Robin Murphy wrote:
> > > [ Upstream commit b46064a18810bad3aea089a79993ca5ea7a3d2b2 ]
> > > 
> > > It turns out that deferred default domain creation leaves a subtle
> > > race window during iommu_device_register() wherein a client driver may
> > > asynchronously probe in parallel and get as far as performing DMA API
> > > operations with dma-direct, only to be switched to iommu-dma underfoot
> > > once the default domain attachment finally happens, with obviously
> > > disastrous consequences. Even the wonky of_iommu_configure() path is at
> > > risk, since iommu_fwspec_init() will no longer defer client probe as the
> > > instance ops are (necessarily) already registered, and the "replay"
> > > iommu_probe_device() call can see dev->iommu_group already set and so
> > > think there's nothing to do either.
> > > 
> > > Fortunately we already have the right tool in the right place in the
> > > form of iommu_device_use_default_domain(), which just needs to ensure
> > > that said default domain is actually ready to *be* used. Deferring the
> > > client probe shouldn't have too much impact, given that this only
> > > happens while the IOMMU driver is probing, and thus due to kick the
> > > deferred probe list again once it finishes.
> > > 
> > > [ Backport: The above is true for mainline, but here we still have
> > > arch_setup_dma_ops() to worry about, which is not replayed if the
> > > default domain happens to be allocated *between* that call and
> > > subsequently reaching iommu_device_use_default_domain(), so we need an
> > > additional earlier check to cover that case. Also we're now back before
> > > the nominal commit 98ac73f99bc4 so we need to tweak the logic to depend
> > > on IOMMU_DMA as well, to avoid falsely deferring on architectures not
> > > using default domains. This then serves us back as far as f188056352bc,
> > > where this specific form of the problem first arises. ]
> > > 
> > > Reported-by: Charan Teja Kalla <quic_charante@quicinc.com>
> > > Fixes: 98ac73f99bc4 ("iommu: Require a default_domain for all iommu drivers")
> > > Fixes: f188056352bc ("iommu: Avoid locking/unlocking for iommu_probe_device()")
> > > Link: https://lore.kernel.org/r/e88b94c9b575034a2c98a48b3d383654cbda7902.1740753261.git.robin.murphy@arm.com
> > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > > ---
> > > 
> > > Resending as a new version with a new Message-Id so as not to confuse
> > > the tools... 6.12.y should simply have a straight cherry-pick of the
> > > mainline commit - 98ac73f99bc4 was in 6.7 so I'm not sure why autosel
> > > hasn't picked that already?
> > 
> > autosel is "maybe we get it", NEVER rely on it for an actual backport to
> > happen.
> > 
> > If you want this in 6.12.y, and it applies cleanly, just ask!  But I
> > can't take this 6.6.y patch before that happens for obvious reasons.
> 
> Understood; I shall try harder to remember to include explicit stable tags
> in future.
> 
> Could you please pick b46064a18810 for 6.12? I checked and there are indeed
> no conflicts :)

Great, all now queued up.

greg k-h