From: Jason Gunthorpe <jgg@nvidia.com> To: Robin Murphy <robin.murphy@arm.com> Cc: Christoph Hellwig <hch@infradead.org>, Kevin Tian <kevin.tian@intel.com>, Chaitanya Kulkarni <kch@nvidia.com>, Ashok Raj <ashok.raj@intel.com>, kvm@vger.kernel.org, rafael@kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Cornelia Huck <cohuck@redhat.com>, Will Deacon <will@kernel.org>, linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Alex Williamson <alex.williamson@redhat.com>, Jacob jun Pan <jacob.jun.pan@intel.com>, linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>, Diana Craciun <diana.craciun@oss.nxp.com> Subject: Re: [PATCH 03/11] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming Date: Mon, 15 Nov 2021 12:17:56 -0400 [thread overview] Message-ID: <20211115161756.GP2105516@nvidia.com> (raw) In-Reply-To: <495c65e4-bd97-5f29-d39b-43671acfec78@arm.com> On Mon, Nov 15, 2021 at 03:14:49PM +0000, Robin Murphy wrote: > > If userspace has control of device A and can cause A to issue DMA to > > arbitary DMA addresses then there are certain PCI topologies where A > > can now issue peer to peer DMA and manipulate the MMMIO registers in > > device B. > > > > A kernel driver on device B is thus subjected to concurrent > > manipulation of the device registers from userspace. > > > > So, a 'safe' kernel driver is one that can tolerate this, and an > > 'unsafe' driver is one where userspace can break kernel integrity. > > You mean in the case where the kernel driver is trying to use device B in a > purely PIO mode, such that userspace might potentially be able to interfere > with data being transferred in and out of the kernel? s/PIO/MMIO, but yes basically. And not just data trasnfer but userspace can interfere with the device state as well. > Perhaps it's not so clear to put that under a notion of "DMA > ownership", since device B's DMA is irrelevant and it's really much > more equivalent to /dev/mem access or mmaping BARs to userspace > while a driver is bound. It is DMA ownership because device A's DMA is what is relevant here. device A's DMA compromises device B. So device A asserts it has USER ownership for DMA. Any device in a group with USER ownership is incompatible with a kernel driver. > > The second issue is DMA - because there is only one iommu_domain > > underlying many devices if we give that iommu_domain to userspace it > > means the kernel DMA API on other devices no longer works. > > Actually, the DMA API itself via iommu-dma will "work" just fine in the > sense that it will still successfully perform all its operations in the > unattached default domain, it's just that if the driver then programs the > device to access the returned DMA address, the device is likely to get a > nasty surprise. A DMA API that returns an dma_ddr_t that does not result in data transfer to the specified buffers is not working, in my book - it breaks the API contract. > > So no kernel driver doing DMA can work at all, under any PCI topology, > > if userspace owns the IO page table. > > This isn't really about userspace at all - it's true of any case where a > kernel driver wants to attach a grouped device to its own unmanaged > domain. This is true for the dma api issue in isolation. I think if we have a user someday it would make sense to add another API DMA_OWNER_DRIVER_DOMAIN that captures how the dma API doesn't work but DMA MMIO attacks are not possible. > The fact that the VFIO kernel driver uses its unmanaged domains to map user > pages upon user requests is merely a VFIO detail, and VFIO happens to be the > only common case where unmanaged domains and non-singleton groups intersect. > I'd say that, logically, if you want to put policy on mutual driver/usage > compatibility anywhere it should be in iommu_attach_group(). It would make sense for iommu_attach_group() to require that the DMA_OWNERSHIP is USER or DRIVER_DOMAIN. That has a nice symmetry with iommu_attach_device() already requiring that the group has a single device. For a driver to use these APIs it must ensure security, one way or another. That is a good idea, but requires understanding what tegra is doing. Maybe tegra is that DMA_OWNER_DRIVER_DOMAIN user? I wouldn't want to see iommu_attach_group() change the DMA_OWNERSHIP, I think ownership is cleaner as a dedicated API. Adding a file * and probably the enum to iommu_attach_group() feels weird. We need the dedicated API for the dma_configure op, and keeping ownership split from the current domain makes more sense with the design in the iommfd RFC. Thanks, Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org> To: Robin Murphy <robin.murphy@arm.com> Cc: Alex Williamson <alex.williamson@redhat.com>, Kevin Tian <kevin.tian@intel.com>, Chaitanya Kulkarni <kch@nvidia.com>, Ashok Raj <ashok.raj@intel.com>, kvm@vger.kernel.org, rafael@kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Cornelia Huck <cohuck@redhat.com>, linux-kernel@vger.kernel.org, Christoph Hellwig <hch@infradead.org>, iommu@lists.linux-foundation.org, Jacob jun Pan <jacob.jun.pan@intel.com>, linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>, Will Deacon <will@kernel.org>, Diana Craciun <diana.craciun@oss.nxp.com> Subject: Re: [PATCH 03/11] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming Date: Mon, 15 Nov 2021 12:17:56 -0400 [thread overview] Message-ID: <20211115161756.GP2105516@nvidia.com> (raw) In-Reply-To: <495c65e4-bd97-5f29-d39b-43671acfec78@arm.com> On Mon, Nov 15, 2021 at 03:14:49PM +0000, Robin Murphy wrote: > > If userspace has control of device A and can cause A to issue DMA to > > arbitary DMA addresses then there are certain PCI topologies where A > > can now issue peer to peer DMA and manipulate the MMMIO registers in > > device B. > > > > A kernel driver on device B is thus subjected to concurrent > > manipulation of the device registers from userspace. > > > > So, a 'safe' kernel driver is one that can tolerate this, and an > > 'unsafe' driver is one where userspace can break kernel integrity. > > You mean in the case where the kernel driver is trying to use device B in a > purely PIO mode, such that userspace might potentially be able to interfere > with data being transferred in and out of the kernel? s/PIO/MMIO, but yes basically. And not just data trasnfer but userspace can interfere with the device state as well. > Perhaps it's not so clear to put that under a notion of "DMA > ownership", since device B's DMA is irrelevant and it's really much > more equivalent to /dev/mem access or mmaping BARs to userspace > while a driver is bound. It is DMA ownership because device A's DMA is what is relevant here. device A's DMA compromises device B. So device A asserts it has USER ownership for DMA. Any device in a group with USER ownership is incompatible with a kernel driver. > > The second issue is DMA - because there is only one iommu_domain > > underlying many devices if we give that iommu_domain to userspace it > > means the kernel DMA API on other devices no longer works. > > Actually, the DMA API itself via iommu-dma will "work" just fine in the > sense that it will still successfully perform all its operations in the > unattached default domain, it's just that if the driver then programs the > device to access the returned DMA address, the device is likely to get a > nasty surprise. A DMA API that returns an dma_ddr_t that does not result in data transfer to the specified buffers is not working, in my book - it breaks the API contract. > > So no kernel driver doing DMA can work at all, under any PCI topology, > > if userspace owns the IO page table. > > This isn't really about userspace at all - it's true of any case where a > kernel driver wants to attach a grouped device to its own unmanaged > domain. This is true for the dma api issue in isolation. I think if we have a user someday it would make sense to add another API DMA_OWNER_DRIVER_DOMAIN that captures how the dma API doesn't work but DMA MMIO attacks are not possible. > The fact that the VFIO kernel driver uses its unmanaged domains to map user > pages upon user requests is merely a VFIO detail, and VFIO happens to be the > only common case where unmanaged domains and non-singleton groups intersect. > I'd say that, logically, if you want to put policy on mutual driver/usage > compatibility anywhere it should be in iommu_attach_group(). It would make sense for iommu_attach_group() to require that the DMA_OWNERSHIP is USER or DRIVER_DOMAIN. That has a nice symmetry with iommu_attach_device() already requiring that the group has a single device. For a driver to use these APIs it must ensure security, one way or another. That is a good idea, but requires understanding what tegra is doing. Maybe tegra is that DMA_OWNER_DRIVER_DOMAIN user? I wouldn't want to see iommu_attach_group() change the DMA_OWNERSHIP, I think ownership is cleaner as a dedicated API. Adding a file * and probably the enum to iommu_attach_group() feels weird. We need the dedicated API for the dma_configure op, and keeping ownership split from the current domain makes more sense with the design in the iommfd RFC. Thanks, Jason _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2021-11-15 16:19 UTC|newest] Thread overview: 120+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-15 2:05 [PATCH 00/11] Fix BUG_ON in vfio_iommu_group_notifier() Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 01/11] iommu: Add device dma ownership set/release interfaces Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 13:14 ` Christoph Hellwig 2021-11-15 13:14 ` Christoph Hellwig 2021-11-16 1:57 ` Lu Baolu 2021-11-16 1:57 ` Lu Baolu 2021-11-16 13:46 ` Jason Gunthorpe 2021-11-16 13:46 ` Jason Gunthorpe via iommu 2021-11-17 5:22 ` Lu Baolu 2021-11-17 5:22 ` Lu Baolu 2021-11-17 13:35 ` Jason Gunthorpe 2021-11-17 13:35 ` Jason Gunthorpe via iommu 2021-11-18 1:12 ` Lu Baolu 2021-11-18 1:12 ` Lu Baolu 2021-11-18 14:10 ` Jason Gunthorpe 2021-11-18 14:10 ` Jason Gunthorpe via iommu 2021-11-18 2:39 ` Tian, Kevin 2021-11-18 2:39 ` Tian, Kevin 2021-11-18 13:33 ` Jason Gunthorpe 2021-11-18 13:33 ` Jason Gunthorpe via iommu 2021-11-19 5:44 ` Tian, Kevin 2021-11-19 5:44 ` Tian, Kevin 2021-11-19 11:14 ` Lu Baolu 2021-11-19 11:14 ` Lu Baolu 2021-11-19 15:06 ` Jörg Rödel 2021-11-19 15:06 ` Jörg Rödel 2021-11-19 15:43 ` Jason Gunthorpe 2021-11-19 15:43 ` Jason Gunthorpe via iommu 2021-11-20 11:16 ` Lu Baolu 2021-11-20 11:16 ` Lu Baolu 2021-11-19 12:56 ` Jason Gunthorpe 2021-11-19 12:56 ` Jason Gunthorpe via iommu 2021-11-15 20:38 ` Bjorn Helgaas 2021-11-15 20:38 ` Bjorn Helgaas 2021-11-16 1:52 ` Lu Baolu 2021-11-16 1:52 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 02/11] driver core: Set DMA ownership during driver bind/unbind Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 6:59 ` Greg Kroah-Hartman 2021-11-15 6:59 ` Greg Kroah-Hartman 2021-11-15 13:20 ` Christoph Hellwig 2021-11-15 13:20 ` Christoph Hellwig 2021-11-15 13:38 ` Jason Gunthorpe via iommu 2021-11-15 13:38 ` Jason Gunthorpe 2021-11-15 13:19 ` Christoph Hellwig 2021-11-15 13:19 ` Christoph Hellwig 2021-11-15 13:24 ` Jason Gunthorpe 2021-11-15 13:24 ` Jason Gunthorpe via iommu 2021-11-15 15:37 ` Robin Murphy 2021-11-15 15:37 ` Robin Murphy 2021-11-15 15:56 ` Jason Gunthorpe 2021-11-15 15:56 ` Jason Gunthorpe via iommu 2021-11-15 18:15 ` Christoph Hellwig 2021-11-15 18:15 ` Christoph Hellwig 2021-11-15 18:35 ` Robin Murphy 2021-11-15 18:35 ` Robin Murphy 2021-11-15 19:39 ` Jason Gunthorpe via iommu 2021-11-15 19:39 ` Jason Gunthorpe 2021-11-15 2:05 ` [PATCH 03/11] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 13:21 ` Christoph Hellwig 2021-11-15 13:21 ` Christoph Hellwig 2021-11-15 13:31 ` Jason Gunthorpe via iommu 2021-11-15 13:31 ` Jason Gunthorpe 2021-11-15 15:14 ` Robin Murphy 2021-11-15 15:14 ` Robin Murphy 2021-11-15 16:17 ` Jason Gunthorpe [this message] 2021-11-15 16:17 ` Jason Gunthorpe via iommu 2021-11-15 17:54 ` Robin Murphy 2021-11-15 17:54 ` Robin Murphy 2021-11-15 18:19 ` Christoph Hellwig 2021-11-15 18:19 ` Christoph Hellwig 2021-11-15 18:44 ` Robin Murphy 2021-11-15 18:44 ` Robin Murphy 2021-11-15 19:22 ` Jason Gunthorpe via iommu 2021-11-15 19:22 ` Jason Gunthorpe 2021-11-15 20:58 ` Robin Murphy 2021-11-15 20:58 ` Robin Murphy 2021-11-15 21:19 ` Jason Gunthorpe via iommu 2021-11-15 21:19 ` Jason Gunthorpe 2021-11-15 20:48 ` Bjorn Helgaas 2021-11-15 20:48 ` Bjorn Helgaas 2021-11-15 22:17 ` Bjorn Helgaas 2021-11-15 22:17 ` Bjorn Helgaas 2021-11-16 6:05 ` Lu Baolu 2021-11-16 6:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 04/11] PCI: portdrv: " Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 20:44 ` Bjorn Helgaas 2021-11-15 20:44 ` Bjorn Helgaas 2021-11-16 7:24 ` Lu Baolu 2021-11-16 7:24 ` Lu Baolu 2021-11-16 20:22 ` Bjorn Helgaas 2021-11-16 20:22 ` Bjorn Helgaas 2021-11-16 20:48 ` Jason Gunthorpe 2021-11-16 20:48 ` Jason Gunthorpe via iommu 2021-11-15 2:05 ` [PATCH 05/11] iommu: Add security context management for assigned devices Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 13:22 ` Christoph Hellwig 2021-11-15 13:22 ` Christoph Hellwig 2021-11-16 7:25 ` Lu Baolu 2021-11-16 7:25 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 06/11] iommu: Expose group variants of dma ownership interfaces Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 13:27 ` Christoph Hellwig 2021-11-15 13:27 ` Christoph Hellwig 2021-11-16 9:42 ` Lu Baolu 2021-11-16 9:42 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 07/11] vfio: Use DMA_OWNER_USER to declaim passthrough devices Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 08/11] vfio: Remove use of vfio_group_viable() Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 09/11] vfio: Delete the unbound_list Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 10/11] vfio: Remove iommu group notifier Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 11/11] iommu: Remove iommu group changes notifier Lu Baolu 2021-11-15 2:05 ` Lu Baolu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20211115161756.GP2105516@nvidia.com \ --to=jgg@nvidia.com \ --cc=alex.williamson@redhat.com \ --cc=ashok.raj@intel.com \ --cc=bhelgaas@google.com \ --cc=cohuck@redhat.com \ --cc=diana.craciun@oss.nxp.com \ --cc=gregkh@linuxfoundation.org \ --cc=hch@infradead.org \ --cc=iommu@lists.linux-foundation.org \ --cc=jacob.jun.pan@intel.com \ --cc=kch@nvidia.com \ --cc=kevin.tian@intel.com \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=rafael@kernel.org \ --cc=robin.murphy@arm.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.