linux-kernel - RE: [PATCH v2 11/13] samples/vfio-mdev-pci: call vfio_add_group

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <A2975661238FB949B60364EF0F2C25743A0B55ED@SHSMSX104.ccr.corp.intel.com>
Date:   Mon, 30 Sep 2019 12:40:38 +0000
From:   "Liu, Yi L" <yi.l.liu@...el.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     "kwankhede@...dia.com" <kwankhede@...dia.com>,
        "Tian, Kevin" <kevin.tian@...el.com>,
        "baolu.lu@...ux.intel.com" <baolu.lu@...ux.intel.com>,
        "Sun, Yi Y" <yi.y.sun@...el.com>,
        "joro@...tes.org" <joro@...tes.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "Zhao, Yan Y" <yan.y.zhao@...el.com>,
        "He, Shaopeng" <shaopeng.he@...el.com>,
        "Xia, Chenbo" <chenbo.xia@...el.com>,
        "Tian, Jun J" <jun.j.tian@...el.com>
Subject: RE: [PATCH v2 11/13] samples/vfio-mdev-pci: call
 vfio_add_group_dev()

Hi Alex,

> From: Alex Williamson [mailto:alex.williamson@...hat.com]
> Sent: Thursday, September 26, 2019 10:37 AM
> To: Liu, Yi L <yi.l.liu@...el.com>
> Subject: Re: [PATCH v2 11/13] samples/vfio-mdev-pci: call vfio_add_group_dev()
> 
> On Thu,  5 Sep 2019 15:59:28 +0800
> Liu Yi L <yi.l.liu@...el.com> wrote:
> 
> > This patch adds vfio_add_group_dev() calling in probe() to make
> > vfio-mdev-pci work well with non-singleton iommu group. User could
> > bind devices from a non-singleton iommu group to either vfio-pci
> > driver or this sample driver. Existing passthru policy works well for
> > this non-singleton group.
> >
> > This is actually a policy choice. A device driver can make this call
> > if it wants to be vfio viable. And it needs to provide dummy
> > vfio_device_ops which is required by vfio framework. To prevent user
> > from opening the device from the iommu backed group fd, the open
> > callback of the dummy vfio_device_ops should return -ENODEV to fail
> > the VFIO_GET_DEVICE_FD request from userspace.
> >
> > Cc: Kevin Tian <kevin.tian@...el.com>
> > Cc: Lu Baolu <baolu.lu@...ux.intel.com>
> > Signed-off-by: Liu Yi L <yi.l.liu@...el.com>
> > ---
> >  drivers/vfio/pci/vfio_mdev_pci.c | 91
> > ++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 82 insertions(+), 9 deletions(-)
> >

[...]

> > +static int vfio_pci_dummy_open(void *device_data) {
> > +	struct vfio_mdev_pci_device *vmdev =
> > +		(struct vfio_mdev_pci_device *) device_data;
> > +	pr_warn("Device %s is not viable for vfio-pci passthru, please follow"
> > +		" vfio-mdev passthru path as it has been wrapped as mdev!!!\n",
> > +					dev_name(&vmdev->vdev.pdev->dev));
> > +	return -ENODEV;
> > +}
> > +
> > +static void vfio_pci_dummy_release(void *device_data) { }
> 
> Theoretically .release will never be called.  If we're paranoid, we could keep it with a
> pr_warn.

yes, it is.

> > +
> > +long vfio_pci_dummy_ioctl(void *device_data,
> > +		   unsigned int cmd, unsigned long arg) {
> > +	return 0;
> > +}
> > +
> > +ssize_t vfio_pci_dummy_read(void *device_data, char __user *buf,
> > +			     size_t count, loff_t *ppos)
> > +{
> > +	return 0;
> > +}
> > +
> > +ssize_t vfio_pci_dummy_write(void *device_data, const char __user *buf,
> > +			      size_t count, loff_t *ppos)
> > +{
> > +	return 0;
> > +}
> > +
> > +int vfio_pci_dummy_mmap(void *device_data, struct vm_area_struct
> > +*vma) {
> > +	return 0;
> > +}
> > +
> > +void vfio_pci_dummy_request(void *device_data, unsigned int count) {
> > +}
> 
> AFAICT, none of .ioctl, .read, .write, .mmap, or .request need to be provided,
> only .open and only .release for paranoia.

sure. let me fix it.

> > +
> > +static const struct vfio_device_ops vfio_pci_dummy_ops = {
> > +	.name		= "vfio-pci",
> 
> This is impersonating vfio-pci, shouldn't we use something like "vfio-mdev-pci-
> dummy".  Thanks,

Yep. will modify it.
 
> Alex

Thanks,
Yi Liu