linux-kernel - Re: [PATCH v2 13/22] iommufd: Add mmap interface

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: <aBFIsYg+ITU8RvTT@Asurada-Nvidia>
Date: Tue, 29 Apr 2025 14:46:25 -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 13/22] iommufd: Add mmap interface

On Tue, Apr 29, 2025 at 09:35:07PM +0000, Pranjal Shrivastava wrote:
> On Tue, Apr 29, 2025 at 02:05:12PM -0700, Nicolin Chen wrote:
> > On Tue, Apr 29, 2025 at 08:55:47PM +0000, Pranjal Shrivastava wrote:
> > > On Tue, Apr 29, 2025 at 01:39:09PM -0700, Nicolin Chen wrote:
> > > > On Tue, Apr 29, 2025 at 08:34:56PM +0000, Pranjal Shrivastava wrote:
> > > > > On Tue, Apr 29, 2025 at 08:24:33PM +0000, Pranjal Shrivastava wrote:
> > > > > > On Fri, Apr 25, 2025 at 10:58:08PM -0700, Nicolin Chen wrote:
> > > > > > > +	struct iommufd_mmap *immap;
> > > > > > > +	int rc;
> > > > > > > +
> > > > > > > +	if (WARN_ON_ONCE(!immap_id))
> > > > > > > +		return -EINVAL;
> > > > > > > +	if (base & ~PAGE_MASK)
> > > > > > > +		return -EINVAL;
> > > > > > > +	if (!size || size & ~PAGE_MASK)
> > > > > > > +		return -EINVAL;
> > > > > > > +
> > > > > > > +	immap = kzalloc(sizeof(*immap), GFP_KERNEL);
> > > > > > > +	if (!immap)
> > > > > > > +		return -ENOMEM;
> > > > > > > +	immap->pfn_start = base >> PAGE_SHIFT;
> > > > > > > +	immap->pfn_end = immap->pfn_start + (size >> PAGE_SHIFT) - 1;
> > > > > > > +
> > > > > > > +	rc = mtree_alloc_range(&ictx->mt_mmap, immap_id, immap, sizeof(immap),
> > > > > > 
> > > > > > I believe this should be sizeof(*immap) ?
> > > > > 
> > > > > Ugh, Sorry, shouldn't this be size >> PAGE_SHIFT (num_indices to alloc) ?
> > > > 
> > > > mtree_load() returns a "struct iommufd_map *" pointer.
> > > 
> > > I'm not talking about mtree_load. I meant mtree_alloc_range takes in a
> > > "size" parameter, which is being passed as sizeof(imap) in this patch.
> > > IIUC, the mtree_alloc_range, via mas_empty_area, gets a range that is
> > > sufficient for the given "size". 
> > > 
> > > Now in this case, "size" would be the no. of pfns which are mmap-able.
> > > By passing sizeof(immap), we're simply reserving sizeof(ptr) i.e. 8 pfns
> > > for a 64-bit machine. Whereas we really, just want to reserve a range
> > > for size >> PAGE_SHIFT pfns.
> > 
> > But we are not storing pfns but the immap pointer..
> 
> Ohh... so we are storing the raw pointer in the mtree.. I got confused
> with the `LONG_MAX >> PAGE_SHIFT`.. Sorry about the confusion!

Yes. We want the pointer at mtree_load(). The pfn range is for
validation after mtree_load(). And we are likely to stuff more
bits into the immap structure for other verifications.

Thanks
Nicolin