| Message ID | 20221014222541.3912195-1-fenghua.yu@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0ec8ce07394442d722806fe61b901a5b2b17249d |
| Headers | show |
| Series | [v2] dmaengine: idxd: Do not enable user type Work Queue without Shared Virtual Addressing | expand |
On Fri, Oct 14, 2022 at 03:25:41PM -0700, Fenghua Yu wrote: > When the idxd_user_drv driver is bound to a Work Queue (WQ) device > without IOMMU or with IOMMU Passthrough without Shared Virtual > Addressing (SVA), the application gains direct access to physical > memory via the device by programming physical address to a submitted > descriptor. This allows direct userspace read and write access to > arbitrary physical memory. This is inconsistent with the security > goals of a good kernel API. > > Unlike vfio_pci driver, the IDXD char device driver does not provide any > ways to pin user pages and translate the address from user VA to IOVA or > PA without IOMMU SVA. Therefore the application has no way to instruct the > device to perform DMA function. This makes the char device not usable for > normal application usage. > > Since user type WQ without SVA cannot be used for normal application usage > and presents the security issue, bind idxd_user_drv driver and enable user > type WQ only when SVA is enabled (i.e. user PASID is enabled). > > Fixes: 448c3de8ac83 ("dmaengine: idxd: create user driver for wq 'device'") > Cc: stable@vger.kernel.org > Suggested-by: Arjan Van De Ven <arjan.van.de.ven@intel.com> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > Reviewed-by: Dave Jiang <dave.jiang@intel.com> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com> > --- > v2: > - Update changlog per Dave Hansen's comments > > drivers/dma/idxd/cdev.c | 18 ++++++++++++++++++ > include/uapi/linux/idxd.h | 1 + > 2 files changed, 19 insertions(+) > > diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c > index c2808fd081d6..a9b96b18772f 100644 > --- a/drivers/dma/idxd/cdev.c > +++ b/drivers/dma/idxd/cdev.c > @@ -312,6 +312,24 @@ static int idxd_user_drv_probe(struct idxd_dev *idxd_dev) > if (idxd->state != IDXD_DEV_ENABLED) > return -ENXIO; > > + /* > + * User type WQ is enabled only when SVA is enabled for two reasons: > + * - If no IOMMU or IOMMU Passthrough without SVA, userspace > + * can directly access physical address through the WQ. > + * - The IDXD cdev driver does not provide any ways to pin > + * user pages and translate the address from user VA to IOVA or > + * PA without IOMMU SVA. Therefore the application has no way > + * to instruct the device to perform DMA function. This makes > + * the cdev not usable for normal application usage. > + */ > + if (!device_user_pasid_enabled(idxd)) { > + idxd->cmd_status = IDXD_SCMD_WQ_USER_NO_IOMMU; > + dev_dbg(&idxd->pdev->dev, > + "User type WQ cannot be enabled without SVA.\n"); > + > + return -EOPNOTSUPP; > + } > + > mutex_lock(&wq->wq_lock); > wq->type = IDXD_WQT_USER; > rc = drv_enable_wq(wq); > diff --git a/include/uapi/linux/idxd.h b/include/uapi/linux/idxd.h > index 095299c75828..2b9e7feba3f3 100644 > --- a/include/uapi/linux/idxd.h > +++ b/include/uapi/linux/idxd.h > @@ -29,6 +29,7 @@ enum idxd_scmd_stat { > IDXD_SCMD_WQ_NO_SIZE = 0x800e0000, > IDXD_SCMD_WQ_NO_PRIV = 0x800f0000, > IDXD_SCMD_WQ_IRQ_ERR = 0x80100000, > + IDXD_SCMD_WQ_USER_NO_IOMMU = 0x80110000, > }; > > #define IDXD_SCMD_SOFTERR_MASK 0x80000000 > -- > 2.32.0 >
> From: Fenghua Yu <fenghua.yu@intel.com> > Sent: Saturday, October 15, 2022 6:26 AM > > + /* > + * User type WQ is enabled only when SVA is enabled for two > reasons: > + * - If no IOMMU or IOMMU Passthrough without SVA, userspace This statement is kind of misleading. Even if IOMMU is in DMA mode user type WQ still doesn't work w/o SVA. > + * can directly access physical address through the WQ. > + * - The IDXD cdev driver does not provide any ways to pin > + * user pages and translate the address from user VA to IOVA or > + * PA without IOMMU SVA. Therefore the application has no way > + * to instruct the device to perform DMA function. This makes > + * the cdev not usable for normal application usage. > + */ It could be simply stated as "SVA is the only secure/reliable way for the device to access user space memory" > + if (!device_user_pasid_enabled(idxd)) { > + idxd->cmd_status = IDXD_SCMD_WQ_USER_NO_IOMMU; be specific i.e. IDXD_SCMD_WQ_USER_NO_SVA > + dev_dbg(&idxd->pdev->dev, > + "User type WQ cannot be enabled without SVA.\n"); > + > + return -EOPNOTSUPP; > + } > + > with above change the check on pasid_enabled should be removed from idxd_cdev_open() and idxd_cdev_release(). They should always work with SVA enabled.
On 14-10-22, 15:25, Fenghua Yu wrote: > When the idxd_user_drv driver is bound to a Work Queue (WQ) device > without IOMMU or with IOMMU Passthrough without Shared Virtual > Addressing (SVA), the application gains direct access to physical > memory via the device by programming physical address to a submitted > descriptor. This allows direct userspace read and write access to > arbitrary physical memory. This is inconsistent with the security > goals of a good kernel API. > > Unlike vfio_pci driver, the IDXD char device driver does not provide any > ways to pin user pages and translate the address from user VA to IOVA or > PA without IOMMU SVA. Therefore the application has no way to instruct the > device to perform DMA function. This makes the char device not usable for > normal application usage. > > Since user type WQ without SVA cannot be used for normal application usage > and presents the security issue, bind idxd_user_drv driver and enable user > type WQ only when SVA is enabled (i.e. user PASID is enabled). Applied, thanks
diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c index c2808fd081d6..a9b96b18772f 100644 --- a/drivers/dma/idxd/cdev.c +++ b/drivers/dma/idxd/cdev.c @@ -312,6 +312,24 @@ static int idxd_user_drv_probe(struct idxd_dev *idxd_dev) if (idxd->state != IDXD_DEV_ENABLED) return -ENXIO; + /* + * User type WQ is enabled only when SVA is enabled for two reasons: + * - If no IOMMU or IOMMU Passthrough without SVA, userspace + * can directly access physical address through the WQ. + * - The IDXD cdev driver does not provide any ways to pin + * user pages and translate the address from user VA to IOVA or + * PA without IOMMU SVA. Therefore the application has no way + * to instruct the device to perform DMA function. This makes + * the cdev not usable for normal application usage. + */ + if (!device_user_pasid_enabled(idxd)) { + idxd->cmd_status = IDXD_SCMD_WQ_USER_NO_IOMMU; + dev_dbg(&idxd->pdev->dev, + "User type WQ cannot be enabled without SVA.\n"); + + return -EOPNOTSUPP; + } + mutex_lock(&wq->wq_lock); wq->type = IDXD_WQT_USER; rc = drv_enable_wq(wq); diff --git a/include/uapi/linux/idxd.h b/include/uapi/linux/idxd.h index 095299c75828..2b9e7feba3f3 100644 --- a/include/uapi/linux/idxd.h +++ b/include/uapi/linux/idxd.h @@ -29,6 +29,7 @@ enum idxd_scmd_stat { IDXD_SCMD_WQ_NO_SIZE = 0x800e0000, IDXD_SCMD_WQ_NO_PRIV = 0x800f0000, IDXD_SCMD_WQ_IRQ_ERR = 0x80100000, + IDXD_SCMD_WQ_USER_NO_IOMMU = 0x80110000, }; #define IDXD_SCMD_SOFTERR_MASK 0x80000000
When the idxd_user_drv driver is bound to a Work Queue (WQ) device without IOMMU or with IOMMU Passthrough without Shared Virtual Addressing (SVA), the application gains direct access to physical memory via the device by programming physical address to a submitted descriptor. This allows direct userspace read and write access to arbitrary physical memory. This is inconsistent with the security goals of a good kernel API. Unlike vfio_pci driver, the IDXD char device driver does not provide any ways to pin user pages and translate the address from user VA to IOVA or PA without IOMMU SVA. Therefore the application has no way to instruct the device to perform DMA function. This makes the char device not usable for normal application usage. Since user type WQ without SVA cannot be used for normal application usage and presents the security issue, bind idxd_user_drv driver and enable user type WQ only when SVA is enabled (i.e. user PASID is enabled). Fixes: 448c3de8ac83 ("dmaengine: idxd: create user driver for wq 'device'") Cc: stable@vger.kernel.org Suggested-by: Arjan Van De Ven <arjan.van.de.ven@intel.com> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> --- v2: - Update changlog per Dave Hansen's comments drivers/dma/idxd/cdev.c | 18 ++++++++++++++++++ include/uapi/linux/idxd.h | 1 + 2 files changed, 19 insertions(+)