linux-kernel - Re: [PATCH] iommu: __iommu_attach_group: check for non-NULL blocking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250929135316.GL2617119@nvidia.com>
Date: Mon, 29 Sep 2025 10:53:16 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Hans Verkuil <hverkuil+cisco@...nel.org>
Cc: iommu@...ts.linux.dev, Linux Kernel <linux-kernel@...r.kernel.org>,
	Linux Media Mailing List <linux-media@...r.kernel.org>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	Robin Murphy <robin.murphy@....com>
Subject: Re: [PATCH] iommu: __iommu_attach_group: check for non-NULL
 blocking_domain

On Mon, Sep 29, 2025 at 03:30:22PM +0200, Hans Verkuil wrote:
> On 29/09/2025 15:02, Jason Gunthorpe wrote:
> > On Mon, Sep 29, 2025 at 02:18:50PM +0200, Hans Verkuil wrote:
> >> On 29/09/2025 14:07, Jason Gunthorpe wrote:
> >>> On Mon, Sep 29, 2025 at 10:23:47AM +0200, Hans Verkuil wrote:
> >>>
> >>>> Since I am unfamiliar with the iommu core code, I am uncertain whether I am
> >>>> just papering over a bug elsewhere, or whether this is really the correct solution.
> >>>
> >>> It is papering over something, group->domain is not supposed to be
> >>> NULL at this point.. That probably means the iommu driver has not been
> >>
> >> It's group->blocking_domain that's NULL, not group->domain.
> > 
> > Er, I thought you were hitting a false positive on this:
> > 
> >   group->domain != group->blocking_domain
> > 
> > ie NULL != NULL
> > 
> > But I suppose the whole expression is checking for group->domain
> > already.
> > 
> > All your patch does is entirely disable the safetly logic :\
> > 
> > What is isp_attach_iommu() trying to accomplish? It does
> > arm_iommu_detach_device() and then arm_iommu_attach_device() ?
> > 
> > Why?
> > 
> > Is this trying to force a non-identity translation for ISP?
> 
> I have absolutely no idea. The commit where this was added is this:

Maybe try deleting everything in the CONFIG_ARM_DMA_USE_IOMMU branches
and just succeed in isp_attach_iommu() ?

It is almost the same code flow as in arm_setup_iommu_dma_ops(),
except it ignores the DT.

arm_setup_iommu_dma_ops() should be called by ISP via arch_setup_dma_ops():

	if (device_iommu_mapped(dev))
		arm_setup_iommu_dma_ops(dev);

?

I'm thinking this is all dead code now. The original version was
creating iommu_groups, so most likely 11 years ago
device_iommu_mapped() == false during driver probe and it had to open
code the whole sequence.

But today the groups are clearly there.. So the iommu should be
setup..

The main direct difference was IPS hard wiring the 1G:

	mapping = arm_iommu_create_mapping(isp->dev, SZ_1G, SZ_2G);

In the generic code this should come from the DT:

	if (dev->dma_range_map) {
		dma_base = dma_range_map_min(dev->dma_range_map);
		size = dma_range_map_max(dev->dma_range_map) - dma_base;
	}
	mapping = arm_iommu_create_mapping(dev, dma_base, size);

So potentially a DT change is also needed?

Can you check around these sequences to see what is happening?

Jason