All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Jason Gunthorpe <jgg@nvidia.com>, Christoph Hellwig <hch@infradead.org>
Cc: 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 15:14:49 +0000	[thread overview]
Message-ID: <495c65e4-bd97-5f29-d39b-43671acfec78@arm.com> (raw)
In-Reply-To: <20211115133107.GB2379906@nvidia.com>

On 2021-11-15 13:31, Jason Gunthorpe via iommu wrote:
> On Mon, Nov 15, 2021 at 05:21:26AM -0800, Christoph Hellwig wrote:
>> On Mon, Nov 15, 2021 at 10:05:44AM +0800, Lu Baolu wrote:
>>> pci_stub allows the admin to block driver binding on a device and make
>>> it permanently shared with userspace. Since pci_stub does not do DMA,
>>> it is safe.
>>
>> If an IOMMU is setup and dma-iommu or friends are not used nothing is
>> unsafe anyway, it just is that IOMMU won't work..
>>
>>> However the admin must understand that using pci_stub allows
>>> userspace to attack whatever device it was bound to.
>>
>> I don't understand this sentence at all.
> 
> 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? 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.

> 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.

> 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. 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().

Robin.

WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: Jason Gunthorpe <jgg@nvidia.com>, Christoph Hellwig <hch@infradead.org>
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, 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 15:14:49 +0000	[thread overview]
Message-ID: <495c65e4-bd97-5f29-d39b-43671acfec78@arm.com> (raw)
In-Reply-To: <20211115133107.GB2379906@nvidia.com>

On 2021-11-15 13:31, Jason Gunthorpe via iommu wrote:
> On Mon, Nov 15, 2021 at 05:21:26AM -0800, Christoph Hellwig wrote:
>> On Mon, Nov 15, 2021 at 10:05:44AM +0800, Lu Baolu wrote:
>>> pci_stub allows the admin to block driver binding on a device and make
>>> it permanently shared with userspace. Since pci_stub does not do DMA,
>>> it is safe.
>>
>> If an IOMMU is setup and dma-iommu or friends are not used nothing is
>> unsafe anyway, it just is that IOMMU won't work..
>>
>>> However the admin must understand that using pci_stub allows
>>> userspace to attack whatever device it was bound to.
>>
>> I don't understand this sentence at all.
> 
> 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? 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.

> 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.

> 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. 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().

Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2021-11-15 15:15 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 [this message]
2021-11-15 15:14         ` Robin Murphy
2021-11-15 16:17         ` Jason Gunthorpe
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=495c65e4-bd97-5f29-d39b-43671acfec78@arm.com \
    --to=robin.murphy@arm.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=jgg@nvidia.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=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: link
Be 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.