linux-kernel - Re: [PATCH] vfio/pci: print vfio-device name to fdinfo

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250624102303.75146159.alex.williamson@redhat.com>
Date: Tue, 24 Jun 2025 10:23:03 -0600
From: Alex Williamson <alex.williamson@...hat.com>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Alex Mastro <amastro@...com>, peterx@...hat.com, kbusch@...nel.org,
 kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vfio/pci: print vfio-device name to fdinfo

On Mon, 23 Jun 2025 21:56:05 -0300
Jason Gunthorpe <jgg@...pe.ca> wrote:

> On Mon, Jun 23, 2025 at 04:18:31PM -0600, Alex Williamson wrote:
> > > Alternatively, if we wanted to normalize show_fdinfo formatting, this
> > > could instead hoist the print formatting up into vfio_main.c, and call
> > > an optional vfio_device_ops->instance_name() to get the name. I opted
> > > not to do this here due to unfamiliarity with other vfio drivers, but am
> > > open to changing it.  
> > 
> > TBH, I don't think we need a callback, just use dev_name() in
> > vfio_main.  
> 
> IMHO this should really be the name of /dev/vfio/XX file and not
> something made up like event fd uses.
> 
> The file was opened via /dev/vfio/XX, that is what lsof should report..
> 
> For the legacy route this effectively gives you the iommu group.

We don't need fdinfo for this, right?  The group is clearly visible in
/proc/PID/fd:

# ls -l | grep vfio
lrwx------. 1 qemu qemu 64 Jun 24 09:27 32 -> /dev/vfio/16
lrwx------. 1 qemu qemu 64 Jun 24 09:27 33 -> /dev/vfio/vfio
lrwx------. 1 qemu qemu 64 Jun 24 09:27 34 -> anon_inode:kvm-vfio
lrwx------. 1 qemu qemu 64 Jun 24 09:27 35 -> anon_inode:[vfio-device]
lrwx------. 1 qemu qemu 64 Jun 24 09:27 38 -> /dev/vfio/2
lrwx------. 1 qemu qemu 64 Jun 24 09:27 39 -> anon_inode:[vfio-device]
lrwx------. 1 qemu qemu 64 Jun 24 09:27 44 -> anon_inode:[vfio-device]
lrwx------. 1 qemu qemu 64 Jun 24 09:27 49 -> /dev/vfio/12
lrwx------. 1 qemu qemu 64 Jun 24 09:27 50 -> anon_inode:[vfio-device]
lrwx------. 1 qemu qemu 64 Jun 24 09:27 55 -> /dev/vfio/4
lrwx------. 1 qemu qemu 64 Jun 24 09:27 56 -> anon_inode:[vfio-device]

An iommufd/vfio-cdev VM even more clearly shows the devices:

# ls -l | grep vfio
lrwx------. 1 root root 64 Jun 24 10:06 23 -> /dev/vfio/devices/vfio7
lrwx------. 1 root root 64 Jun 24 10:06 24 -> anon_inode:kvm-vfio
lrwx------. 1 root root 64 Jun 24 10:06 30 -> /dev/vfio/devices/vfio8

I think we're specifically trying to gain visibility to the
anon_inode:[vfio-device] in the legacy case.

The @name passed to anon_inode_getfile_fmode() is described as the name
of the "class", which is why I think we used the static
"[vfio-device]", but I see KVM breaks the mold, adding the vcpu_id:

	snprintf(name, sizeof(name), "kvm-vcpu-stats:%d", vcpu->vcpu_id);

We could do something similar, but maybe fdinfo is the better option,
and if it is then dev_name() seems like the useful thing to add there
(though we could add more than one thing).

> For the new route this will give you the struct device.
> 
> The userspace can deduce more information, like the actual PCI BDF, by
> mapping the name through sysfs.

I don't really know what the rules are here, whether we're able to
report information for convenience or we should strive for the absolute
most concise reference.  cdev and group information is available
without fdinfo.

> I would have guessed this is already happening automatically as part
> of the cdev mechanism? Maybe we broken it when we changed the inode to
> use unmap mapping range?
> 
> > The group interface always requires the name, in some cases
> > it can require further information, but we seem to have forgotten that
> > in the cdev interface anyway :-\  
> 
> ?

I don't recall if or how we accounted for the concept of vf_tokens in
the cdev model and I don't see evidence that we did.  For instance
vfio_pci_validate_vf_token() is only called from vfio_pci_core_match(),
which is called as match through the vfio_device_ops, but only from
vfio_group_ioctl_get_device_fd().  So using cdev, it appears we don't
have the same opt-in requirement when using a VF where the PF is
managed by a vfio-pci userspace driver.  Thanks,

Alex