linux-kernel - Re: [PATCH 02/11] iommu/vt-d: Optimize iotlb_sync

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250716141218.GA2166806@nvidia.com>
Date: Wed, 16 Jul 2025 11:12:18 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Lu Baolu <baolu.lu@...ux.intel.com>
Cc: Joerg Roedel <joro@...tes.org>, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/11] iommu/vt-d: Optimize iotlb_sync_map for
 non-caching/non-RWBF modes

On Mon, Jul 14, 2025 at 12:50:19PM +0800, Lu Baolu wrote:
> @@ -1833,6 +1845,8 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,
>  	if (ret)
>  		goto out_block_translation;
>  
> +	domain->iotlb_sync_map |= domain_need_iotlb_sync_map(domain, iommu);

This has no locking and is in the wrong order anyhow :(

Any change to how invalidation works has to be done before attaching
the HW so that the required invalidations are already happening before
the HW can walk the page table.

And you need to serialize somehow with concurrent map/unmap as iommufd
doesn't prevent userspace from racing attach with map/unmap.

The cache_tag_assign_domain() looks similarly wrong too, it needs to
start invalidating the cache tag of the new domain, then change the
context then stop invalidating the cache tag of the old
domain. Otherwise there are invalidation races.

Finally, if the HW needs RWBF then this also needs to do the buffer
flush in this thread before installing the context to prevent a race.

Overall this dynamic behavior may just be a bad idea, and perhaps you
can live with domains having the domain->iotlb_sync_map as a static
property set once during paging domain allocation.

If the iommu requires iotlb_sync_map but the domain does not have it
then the attach is rejected. This reduces domain sharing
possibilities, but maybe that is just fine??

Jason