| Message ID | 20200723214705.5399-3-giovanni.cabiddu@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | vfio/pci: add denylist and disable qat | expand |
On Thu, 23 Jul 2020 22:47:02 +0100 Giovanni Cabiddu <giovanni.cabiddu@intel.com> wrote: > Add denylist of devices that by default are not probed by vfio-pci. > Devices in this list may be susceptible to untrusted application, even > if the IOMMU is enabled. To be accessed via vfio-pci, the user has to > explicitly disable the denylist. > > The denylist can be disabled via the module parameter disable_denylist. > > Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > --- > drivers/vfio/pci/vfio_pci.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > index 7c0779018b1b..673f53c4798e 100644 > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -60,6 +60,10 @@ module_param(enable_sriov, bool, 0644); > MODULE_PARM_DESC(enable_sriov, "Enable support for SR-IOV configuration. Enabling SR-IOV on a PF typically requires support of the userspace PF driver, enabling VFs without such support may result in non-functional VFs or PF."); > #endif > > +static bool disable_denylist; > +module_param(disable_denylist, bool, 0444); > +MODULE_PARM_DESC(disable_denylist, "Disable use of device denylist. Disabling the denylist prevents binding to devices with known errata that may lead to exploitable stability or security issues when accessed by untrusted users."); s/prevents/allows/ ie. the denylist prevents binding, therefore disabling the denylist allows binding I can fix this on commit without a new version if you agree. I also see that patch 1/5 didn't change since v2, so I'll transfer Bjorn's ack. If that sounds good I'll queue the first 3 patches in my next branch for v5.9. Thanks, Alex > + > static inline bool vfio_vga_disabled(void) > { > #ifdef CONFIG_VFIO_PCI_VGA > @@ -69,6 +73,29 @@ static inline bool vfio_vga_disabled(void) > #endif > } > > +static bool vfio_pci_dev_in_denylist(struct pci_dev *pdev) > +{ > + return false; > +} > + > +static bool vfio_pci_is_denylisted(struct pci_dev *pdev) > +{ > + if (!vfio_pci_dev_in_denylist(pdev)) > + return false; > + > + if (disable_denylist) { > + pci_warn(pdev, > + "device denylist disabled - allowing device %04x:%04x.\n", > + pdev->vendor, pdev->device); > + return false; > + } > + > + pci_warn(pdev, "%04x:%04x exists in vfio-pci device denylist, driver probing disallowed.\n", > + pdev->vendor, pdev->device); > + > + return true; > +} > + > /* > * Our VGA arbiter participation is limited since we don't know anything > * about the device itself. However, if the device is the only VGA device > @@ -1847,6 +1874,9 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > struct iommu_group *group; > int ret; > > + if (vfio_pci_is_denylisted(pdev)) > + return -EINVAL; > + > if (pdev->hdr_type != PCI_HEADER_TYPE_NORMAL) > return -EINVAL; > > @@ -2336,6 +2366,9 @@ static int __init vfio_pci_init(void) > > vfio_pci_fill_ids(); > > + if (disable_denylist) > + pr_warn("device denylist disabled.\n"); > + > return 0; > > out_driver:
On Thu, Jul 23, 2020 at 04:41:26PM -0600, Alex Williamson wrote: > On Thu, 23 Jul 2020 22:47:02 +0100 > Giovanni Cabiddu <giovanni.cabiddu@intel.com> wrote: > > > Add denylist of devices that by default are not probed by vfio-pci. > > Devices in this list may be susceptible to untrusted application, even > > if the IOMMU is enabled. To be accessed via vfio-pci, the user has to > > explicitly disable the denylist. > > > > The denylist can be disabled via the module parameter disable_denylist. > > > > Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > > --- > > drivers/vfio/pci/vfio_pci.c | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > > index 7c0779018b1b..673f53c4798e 100644 > > --- a/drivers/vfio/pci/vfio_pci.c > > +++ b/drivers/vfio/pci/vfio_pci.c > > @@ -60,6 +60,10 @@ module_param(enable_sriov, bool, 0644); > > MODULE_PARM_DESC(enable_sriov, "Enable support for SR-IOV configuration. Enabling SR-IOV on a PF typically requires support of the userspace PF driver, enabling VFs without such support may result in non-functional VFs or PF."); > > #endif > > > > +static bool disable_denylist; > > +module_param(disable_denylist, bool, 0444); > > +MODULE_PARM_DESC(disable_denylist, "Disable use of device denylist. Disabling the denylist prevents binding to devices with known errata that may lead to exploitable stability or security issues when accessed by untrusted users."); > > s/prevents/allows/ > > ie. the denylist prevents binding, therefore disabling the denylist > allows binding > > I can fix this on commit without a new version if you agree. I also > see that patch 1/5 didn't change since v2, so I'll transfer Bjorn's > ack. If that sounds good I'll queue the first 3 patches in my next > branch for v5.9. Thanks, My bad, apologies! I'm ok also to re-spin adding Bjorn's ack and the fix above. Regards,
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 7c0779018b1b..673f53c4798e 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -60,6 +60,10 @@ module_param(enable_sriov, bool, 0644); MODULE_PARM_DESC(enable_sriov, "Enable support for SR-IOV configuration. Enabling SR-IOV on a PF typically requires support of the userspace PF driver, enabling VFs without such support may result in non-functional VFs or PF."); #endif +static bool disable_denylist; +module_param(disable_denylist, bool, 0444); +MODULE_PARM_DESC(disable_denylist, "Disable use of device denylist. Disabling the denylist prevents binding to devices with known errata that may lead to exploitable stability or security issues when accessed by untrusted users."); + static inline bool vfio_vga_disabled(void) { #ifdef CONFIG_VFIO_PCI_VGA @@ -69,6 +73,29 @@ static inline bool vfio_vga_disabled(void) #endif } +static bool vfio_pci_dev_in_denylist(struct pci_dev *pdev) +{ + return false; +} + +static bool vfio_pci_is_denylisted(struct pci_dev *pdev) +{ + if (!vfio_pci_dev_in_denylist(pdev)) + return false; + + if (disable_denylist) { + pci_warn(pdev, + "device denylist disabled - allowing device %04x:%04x.\n", + pdev->vendor, pdev->device); + return false; + } + + pci_warn(pdev, "%04x:%04x exists in vfio-pci device denylist, driver probing disallowed.\n", + pdev->vendor, pdev->device); + + return true; +} + /* * Our VGA arbiter participation is limited since we don't know anything * about the device itself. However, if the device is the only VGA device @@ -1847,6 +1874,9 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) struct iommu_group *group; int ret; + if (vfio_pci_is_denylisted(pdev)) + return -EINVAL; + if (pdev->hdr_type != PCI_HEADER_TYPE_NORMAL) return -EINVAL; @@ -2336,6 +2366,9 @@ static int __init vfio_pci_init(void) vfio_pci_fill_ids(); + if (disable_denylist) + pr_warn("device denylist disabled.\n"); + return 0; out_driver:
Add denylist of devices that by default are not probed by vfio-pci. Devices in this list may be susceptible to untrusted application, even if the IOMMU is enabled. To be accessed via vfio-pci, the user has to explicitly disable the denylist. The denylist can be disabled via the module parameter disable_denylist. Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> --- drivers/vfio/pci/vfio_pci.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)