linux-kernel - Re: [PATCH v3 4/6] iommu: Move IOMMU pagesize check to attach

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]

Date:   Thu, 21 Oct 2021 09:10:49 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Lu Baolu <baolu.lu@...ux.intel.com>
Cc:     Sven Peter <sven@...npeter.dev>, iommu@...ts.linux-foundation.org,
        Robin Murphy <robin.murphy@....com>,
        Arnd Bergmann <arnd@...nel.org>,
        Hector Martin <marcan@...can.st>, linux-kernel@...r.kernel.org,
        Alexander Graf <graf@...zon.com>,
        Mohamed Mediouni <mohamed.mediouni@...amail.com>,
        Will Deacon <will@...nel.org>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>
Subject: Re: [PATCH v3 4/6] iommu: Move IOMMU pagesize check to attach_device

On Thu, 21 Oct 2021 03:22:30 +0100,
Lu Baolu <baolu.lu@...ux.intel.com> wrote:
> 
> On 10/20/21 10:22 PM, Marc Zyngier wrote:
> > On Wed, 20 Oct 2021 06:21:44 +0100,
> > Lu Baolu <baolu.lu@...ux.intel.com> wrote:
> >> 
> >> On 2021/10/20 0:37, Sven Peter via iommu wrote:
> >>> +	/*
> >>> +	 * Check that CPU pages can be represented by the IOVA granularity.
> >>> +	 * This has to be done after ops->attach_dev since many IOMMU drivers
> >>> +	 * only limit domain->pgsize_bitmap after having attached the first
> >>> +	 * device.
> >>> +	 */
> >>> +	ret = iommu_check_page_size(domain);
> >>> +	if (ret) {
> >>> +		__iommu_detach_device(domain, dev);
> >>> +		return ret;
> >>> +	}
> >> 
> >> It looks odd. __iommu_attach_device() attaches an I/O page table for a
> >> device. How does it relate to CPU pages? Why is it a failure case if CPU
> >> page size is not covered?
> > 
> > If you allocate a CPU PAGE_SIZE'd region, and point it at a device
> > that now can DMA to more than what you have allocated because the
> > IOMMU's own page size is larger, the device has now access to data it
> > shouldn't see. In my book, that's a pretty bad thing.
> 
> But even you enforce the CPU page size check here, this problem still
> exists unless all DMA buffers are PAGE_SIZE aligned and sized, right?

Let me take a CPU analogy: you have a page that contains some user
data *and* a kernel secret. How do you map this page into userspace
without leaking the kernel secret?

PAGE_SIZE allocations are the unit of isolation, and this applies to
both CPU and IOMMU. If you have allocated a DMA buffer that is less
than a page, you then have to resort to bounce buffering, or accept
that your data isn't safe.

	M.

-- 
Without deviation from the norm, progress is not possible.