linux-kernel - Re: [PATCH v2 08/22] iommufd: Abstract iopt_pin_pages and iopt_unpin

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: <aBAQdq0jeoCdKdsC@Asurada-Nvidia>
Date: Mon, 28 Apr 2025 16:34:14 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: Pranjal Shrivastava <praan@...gle.com>
CC: <jgg@...dia.com>, <kevin.tian@...el.com>, <corbet@....net>,
	<will@...nel.org>, <bagasdotme@...il.com>, <robin.murphy@....com>,
	<joro@...tes.org>, <thierry.reding@...il.com>, <vdumpa@...dia.com>,
	<jonathanh@...dia.com>, <shuah@...nel.org>, <jsnitsel@...hat.com>,
	<nathan@...nel.org>, <peterz@...radead.org>, <yi.l.liu@...el.com>,
	<mshavit@...gle.com>, <zhangzekun11@...wei.com>, <iommu@...ts.linux.dev>,
	<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-tegra@...r.kernel.org>,
	<linux-kselftest@...r.kernel.org>, <patches@...ts.linux.dev>,
	<mochs@...dia.com>, <alok.a.tiwari@...cle.com>, <vasant.hegde@....com>
Subject: Re: [PATCH v2 08/22] iommufd: Abstract iopt_pin_pages and
 iopt_unpin_pages helpers

On Mon, Apr 28, 2025 at 03:12:33PM -0700, Nicolin Chen wrote:
> On Mon, Apr 28, 2025 at 08:14:19PM +0000, Pranjal Shrivastava wrote:
> > On Fri, Apr 25, 2025 at 10:58:03PM -0700, Nicolin Chen wrote:
> > > +	iopt_for_each_contig_area(&iter, area, iopt, iova, last_iova) {
> > > +		unsigned long last = min(last_iova, iopt_area_last_iova(area));
> > > +		unsigned long last_index = iopt_area_iova_to_index(area, last);
> > > +		unsigned long index =
> > > +			iopt_area_iova_to_index(area, iter.cur_iova);
> > > +
> > > +		if (area->prevent_access ||
> > 
> > Nit:
> > Shouldn't we return -EBUSY or something if (area->prevent_access == 1) ?
> > IIUC, this just means that an unmap attempt is in progress, hence avoid
> > accessing the area.
> 
> Maybe. But this is what it was. So we need a different patch to
> change that.

Rereading the code. The prevent_access is set by an unmap(), which
means there shouldn't be any pin() and rw() as the caller should
finish unmap() first.

In the newer use case of vCMDQ, it's similar. If VMM is unmapping
the stage-2 mapping, it shouldn't try to allocate a vCMDQ.

-EBUSY makes some sense, but -EINVAL could still stand.

So, I am leaving it as is, since this patch is just about moving
the functions for sharing.

Nicolin