RE: Plan for /dev/ioasid RFC v2

From: "Tian, Kevin" <kevin.tian@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Jason Wang <jasowang@redhat.com>,
	Kirti Wankhede <kwankhede@nvidia.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	"Jiang, Dave" <dave.jiang@intel.com>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	Jonathan Corbet <corbet@lwn.net>,
	"parav@mellanox.com" <parav@mellanox.com>,
	"Enrico Weigelt, metux IT consult" <lkml@metux.net>,
	David Gibson <david@gibson.dropbear.id.au>,
	Robin Murphy <robin.murphy@arm.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Shenming Lu <lushenming@huawei.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: RE: Plan for /dev/ioasid RFC v2
Date: Wed, 16 Jun 2021 06:53:16 +0000	[thread overview]
Message-ID: <MWHPR11MB18864A728DC40F109EE9FF508C0F9@MWHPR11MB1886.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210615105601.4d7b8906.alex.williamson@redhat.com>

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Wednesday, June 16, 2021 12:56 AM
> 
> On Tue, 15 Jun 2021 01:21:35 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > Sent: Monday, June 14, 2021 9:38 PM
> > >
> > > On Mon, Jun 14, 2021 at 03:09:31AM +0000, Tian, Kevin wrote:
> > >
> > > > If a device can be always blocked from accessing memory in the IOMMU
> > > > before it's bound to a driver or more specifically before the driver
> > > > moves it to a new security context, then there is no need for VFIO
> > > > to track whether IOASIDfd has taken over ownership of the DMA
> > > > context for all devices within a group.
> > >
> > > I've been assuming we'd do something like this, where when a device is
> > > first turned into a VFIO it tells the IOMMU layer that this device
> > > should be DMA blocked unless an IOASID is attached to
> > > it. Disconnecting an IOASID returns it to blocked.
> >
> > Or just make sure a device is in block-DMA when it's unbound from a
> > driver or a security context. Then no need to explicitly tell IOMMU layer
> > to do so when it's bound to a new driver.
> >
> > Currently the default domain type applies even when a device is not
> > bound. This implies that if iommu=passthrough a device is always
> > allowed to access arbitrary system memory with or without a driver.
> > I feel the current domain type (identity, dma, unmanged) should apply
> > only when a driver is loaded...
> 
> Note that vfio does not currently require all devices in the group to
> be bound to drivers.  Other devices within the group, those bound to
> vfio drivers, can be used in this configuration.  This is not
> necessarily recommended though as a non-vfio, non-stub driver binding
> to one of those devices can trigger a BUG_ON.

This is a good learning that I didn't realize before. 😊

As explained in previous mail, we need reuse the group_viable mechanism
to trigger BUG_ON.

> 
> > > > If this works I didn't see the need for vfio to keep the sequence.
> > > > VFIO still keeps group fd to claim ownership of all devices in a
> > > > group.
> > >
> > > As Alex says you still have to deal with the problem that device A in
> > > a group can gain control of device B in the same group.
> >
> > There is no isolation in the group then how could vfio prevent device
> > A from gaining control of device B? for example when both are attached
> > to the same GPA address space with device MMIO bar included, devA
> > can do p2p to devB. It's all user's policy how to deal with devices within
> > the group.
> 
> The latter is user policy, yes, but it's a system security issue that
> the user cannot use device A to control device B if the user doesn't
> have access to both devices, ie. doesn't own the group.  vfio would
> prevent this by not allowing access to device A while device B is
> insecure and would require that all devices within the group remain in
> a secure, user owned state for the extent of access to device A.
> 
> > > This means device A and B can not be used from to two different
> > > security contexts.
> >
> > It depends on how the security context is defined. From iommu layer
> > p.o.v, an IOASID is a security context which isolates a device from
> > the rest of the system (but not the sibling in the same group). As you
> > suggested earlier, it's completely sane if an user wants to attach
> > devices in a group to different IOASIDs. Here I just talk about this fact.
> 
> This is sane, yes, but that doesn't give us license to allow the user
> to access device A regardless of the state of device B.
> 
> > >
> > > If the /dev/iommu FD is the security context then the tracking is
> > > needed there.
> > >
> >
> > As I replied to Alex, my point is that VFIO doesn't need to know the
> > attaching status of each device in a group before it can allow user to
> > access a device. As long as a device in a group either in block DMA
> > or switch to a new address space created via /dev/iommu FD, there's
> > no problem to allow user accessing it. User cannot do harm to the
> > world outside of the group. User knows there is no isolation within
> > the group. that is it.
> 
> This is self contradictory, "vfio doesn't need to know the attachment
> status"... "[a]s long as a device in a group either in block DMA or
> switch to a new address space".  So vfio does need to know the latter.
> How does it know that?  Thanks,
> 

My point was that, if a device can only be in two states: block-DMA or 
attached to a new address space, which both are secure, then vfio 
doesn't need to track which state a device is actually in.

Thanks
Kevin