linux-kernel - Re: [PATCH v7 10/28] iommufd/access: Bypass access->ops->unmap for internal use

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: <aGdT0uNtfIFLvnBO@Asurada-Nvidia>
Date: Thu, 3 Jul 2025 21:08:50 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: "Tian, Kevin" <kevin.tian@...el.com>
CC: "jgg@...dia.com" <jgg@...dia.com>, "corbet@....net" <corbet@....net>,
	"will@...nel.org" <will@...nel.org>, "bagasdotme@...il.com"
	<bagasdotme@...il.com>, "robin.murphy@....com" <robin.murphy@....com>,
	"joro@...tes.org" <joro@...tes.org>, "thierry.reding@...il.com"
	<thierry.reding@...il.com>, "vdumpa@...dia.com" <vdumpa@...dia.com>,
	"jonathanh@...dia.com" <jonathanh@...dia.com>, "shuah@...nel.org"
	<shuah@...nel.org>, "jsnitsel@...hat.com" <jsnitsel@...hat.com>,
	"nathan@...nel.org" <nathan@...nel.org>, "peterz@...radead.org"
	<peterz@...radead.org>, "Liu, Yi L" <yi.l.liu@...el.com>,
	"mshavit@...gle.com" <mshavit@...gle.com>, "praan@...gle.com"
	<praan@...gle.com>, "zhangzekun11@...wei.com" <zhangzekun11@...wei.com>,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-tegra@...r.kernel.org"
	<linux-tegra@...r.kernel.org>, "linux-kselftest@...r.kernel.org"
	<linux-kselftest@...r.kernel.org>, "patches@...ts.linux.dev"
	<patches@...ts.linux.dev>, "mochs@...dia.com" <mochs@...dia.com>,
	"alok.a.tiwari@...cle.com" <alok.a.tiwari@...cle.com>, "vasant.hegde@....com"
	<vasant.hegde@....com>, "dwmw2@...radead.org" <dwmw2@...radead.org>,
	"baolu.lu@...ux.intel.com" <baolu.lu@...ux.intel.com>
Subject: Re: [PATCH v7 10/28] iommufd/access: Bypass access->ops->unmap for
 internal use

On Thu, Jul 03, 2025 at 04:57:34AM +0000, Tian, Kevin wrote:
> I meant something like below:
> 
> iopt_unmap_iova_range()
> {
> 	bool internal_access = false;
> 
> 	down_read(&iopt->domains_rwsem);
> 	down_write(&iopt->iova_rwsem);
> 	/* Bypass any unmap if there is an internal access */
> 	xa_for_each(&iopt->access_list, index, access) {
> 		if (iommufd_access_is_internal(access)) {
> 			internal_access = true;
> 			break;
> 		}
> 	}
> 
> 	while ((area = iopt_area_iter_first(iopt, start, last))) {
> 		if (area->num_access) {
> 			if (internal_access) {
> 				rc = -EBUSY;
> 				goto out_unlock_iova;
> 			}
> 			up_write(&iopt->iova_rwsem);
> 			up_read(&iopt->domains_rwsem);
> 			iommufd_access_notify_unmap(iopt, area_first, length);	
> 		}
> 	}
> }
> 
> it checks the access_list in the common path, but the cost should be
> negligible when there is no access attached to this iopt. The upside
> is that now unmap is denied explicitly in the area loop instead of 
> still trying to unmap and then handling errors.

Hmm, I realized that either way might be incorrect, as it iterates
the entire iopt for any internal access regardless its iova ranges.

What we really want is to reject an unmap against the same range as
once pinged by an internal access, i.e. other range of unmap should
be still allowed.

So, doing it at this level isn't enough. I think we should still go
down to struct iopt_area as my v5 did:
https://lore.kernel.org/all/3ddc8c678406772a8358a265912bb1c064f4c796.1747537752.git.nicolinc@nvidia.com/
We'd only need to rename to num_locked as you suggested, i.e.

@@ -719,6 +719,12 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start,
 			goto out_unlock_iova;
 		}
 
+		/* The area is locked by an object that has not been destroyed */
+		if (area->num_locked) {
+			rc = -EBUSY;
+			goto out_unlock_iova;
+		}
+
 		if (area_first < start || area_last > last) {
 			rc = -ENOENT;
 			goto out_unlock_iova;

Thanks
Nicolin