Re: [PATCH v3 02/10] iommu/sva: Bind process address spaces to devices

From: Jacob Pan <jacob.jun.pan@linux.intel.com>
To: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Cc: Lu Baolu <baolu.lu@linux.intel.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"joro@8bytes.org" <joro@8bytes.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"jcrouse@codeaurora.org" <jcrouse@codeaurora.org>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"Jonathan.Cameron@huawei.com" <Jonathan.Cameron@huawei.com>,
	"christian.koenig@amd.com" <christian.koenig@amd.com>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"kevin.tian@intel.com" <kevin.tian@intel.com>,
	"yi.l.liu@intel.com" <yi.l.liu@intel.com>,
	Andrew Murray <Andrew.Murray@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	Robin Murphy <Robin.Murphy@arm.com>,
	"ashok.raj@intel.com" <ashok.raj@intel.com>,
	"xuzaibo@huawei.com" <xuzaibo@huawei.com>,
	"liguozhu@hisilicon.com" <liguozhu@hisilicon.com>,
	"okaya@codeaurora.org" <okaya@codeaurora.org>,
	"bharatku@xilinx.com" <bharatku@xilinx.com>,
	"ilias.apalodimas@linaro.org" <ilias.apalodimas@linaro.org>,
	"shunyong.yang@hxt-semitech.com" <shunyong.yang@hxt-semitech.com>,
	jacob.jun.pan@linux.intel.com
Subject: Re: [PATCH v3 02/10] iommu/sva: Bind process address spaces to devices
Date: Wed, 26 Sep 2018 11:01:03 -0700	[thread overview]
Message-ID: <20180926110103.45b57f75@jacob-builder> (raw)
In-Reply-To: <7cbd503a-c79e-3c40-7388-ce6c23f7f536@arm.com>

On Mon, 24 Sep 2018 13:07:47 +0100
Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:

> On 23/09/2018 04:05, Lu Baolu wrote:
> > Hi,
> > 
> > On 09/21/2018 01:00 AM, Jean-Philippe Brucker wrote:  
> >> Add bind() and unbind() operations to the IOMMU API. Bind()
> >> returns a PASID that drivers can program in hardware, to let their
> >> devices access an mm. This patch only adds skeletons for the
> >> device driver API, most of the implementation is still missing.  
> > 
> > Is it possible that a malicious process can unbind a pasid which is
> > used by another normal process?  
> 
> Yes, it's up to the device driver that calls unbind() to check that
> the caller is allowed to unbind this PASID. We can't do it ourselves
> since unbind() could also be called from a kernel thread for example
> from a cleanup function in some workqueue, outside the context of the
> process to unbind.
> 
I am wondering if we can avoid the complexity around permission
checking by simply _only_ allow bind/unbind() on current mm? what would
be the missing use cases if we bind current only?
It can also avoid other race such as unbind and mmu_notifier release
call.

> Jean
> 
> > 
> > It might happen in below sequence:
> > 
> > 
> > Process A                       Process B
> > =========                       =========
> > iommu_sva_init_device(dev)
> > iommu_sva_bind_device(dev)
> > ....
> > device access mm of A with
> > #PASID returned above
> > ....
> >                                 iommu_sva_unbind_device(dev, #PASID)
> > ....
> > [unrecoverable errors]
> > 
> > I didn't have a thorough consideration of this. Sorry if this has
> > been prevented.
> > 
> > Best regards,
> > Lu Baolu  

[Jacob Pan]