| 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
| ||
|
Message-ID: <20250623161831.12109402.alex.williamson@redhat.com>
Date: Mon, 23 Jun 2025 16:18:31 -0600
From: Alex Williamson <alex.williamson@...hat.com>
To: Alex Mastro <amastro@...com>
Cc: <peterx@...hat.com>, <kbusch@...nel.org>, <kvm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, Jason Gunthorpe <jgg@...pe.ca>
Subject: Re: [PATCH] vfio/pci: print vfio-device name to fdinfo
On Mon, 23 Jun 2025 14:02:38 -0700
Alex Mastro <amastro@...com> wrote:
> Print the PCI device name to a vfio device's fdinfo. This enables tools
> to query which device is associated with a given vfio device fd. It's
> inspired by eventfd's printing of "eventfd-id" (fs/eventfd.c), which
> lsof uses to format the NAME column (e.g. "[eventfd:7278]").
>
> This results in output like below:
>
> $ cat /proc/"$process_using_vfio"/fdinfo/"$vfio_device_fd" | grep vfio
> vfio-device-name: 0000:c6:00.0
>
> Signed-off-by: Alex Mastro <amastro@...com>
> ---
> Hello, this is my first patch submission to vfio, and linux. We would
> like our tools to be able to query the PCI device name for a given
> vfio-device fd by inspecting a process's open file descriptors. It is
> inspired by eventfd's id printing, which is nicely formatted by lsof in
> the NAME column.
>
> I am not sure to what extent this should be generalized, so I opted
> to put as little policy as possible into vfio_main.c, and have each
> vfio_device_fops implement what it means to show_fdinfo. The only
> implementer is vfio_pci_ops in this change.
>
> 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. 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 noticed that other vfio_device_fops are guarded by checks on
> vfio_device_file.access_granted. From what I can tell, that shouldn't
> be required here, since a vfio pci device is guaranteed to be
> able to print its name (due to existence of vfio_device.pdev) after
> vfio_device_ops.init() construction.
Yep, we shouldn't need any extra locking.
> This change rooted on the for-linus branch of linux-vfio [1].
>
> [1] https://github.com/awilliam/linux-vfio
> ---
> drivers/vfio/pci/vfio_pci.c | 14 ++++++++++++++
> drivers/vfio/vfio_main.c | 15 +++++++++++++++
> include/linux/vfio.h | 2 ++
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 5ba39f7623bb..b682766127ab 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -21,6 +21,7 @@
> #include <linux/mutex.h>
> #include <linux/notifier.h>
> #include <linux/pm_runtime.h>
> +#include <linux/seq_file.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/uaccess.h>
> @@ -125,6 +126,16 @@ static int vfio_pci_open_device(struct vfio_device *core_vdev)
> return 0;
> }
>
> +#ifdef CONFIG_PROC_FS
> +static void vfio_pci_core_show_fdinfo(struct vfio_device *core_vdev, struct seq_file *m)
> +{
> + struct vfio_pci_core_device *vdev =
> + container_of(core_vdev, struct vfio_pci_core_device, vdev);
> +
> + seq_printf(m, "vfio-device-name: %s\n", pci_name(vdev->pdev));
> +}
> +#endif
> +
> static const struct vfio_device_ops vfio_pci_ops = {
> .name = "vfio-pci",
> .init = vfio_pci_core_init_dev,
> @@ -138,6 +149,9 @@ static const struct vfio_device_ops vfio_pci_ops = {
> .mmap = vfio_pci_core_mmap,
> .request = vfio_pci_core_request,
> .match = vfio_pci_core_match,
> +#ifdef CONFIG_PROC_FS
> + .show_fdinfo = vfio_pci_core_show_fdinfo,
> +#endif
> .bind_iommufd = vfio_iommufd_physical_bind,
> .unbind_iommufd = vfio_iommufd_physical_unbind,
> .attach_ioas = vfio_iommufd_physical_attach_ioas,
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 1fd261efc582..e02504247da8 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -28,6 +28,7 @@
> #include <linux/pseudo_fs.h>
> #include <linux/rwsem.h>
> #include <linux/sched.h>
> +#include <linux/seq_file.h>
Only where we're doing the seq_print do we need to include this.
I think you're missing an update to Documentation/filesystems/proc.rst
as well.
> #include <linux/slab.h>
> #include <linux/stat.h>
> #include <linux/string.h>
> @@ -1354,6 +1355,17 @@ static int vfio_device_fops_mmap(struct file *filep, struct vm_area_struct *vma)
> return device->ops->mmap(device, vma);
> }
>
> +#ifdef CONFIG_PROC_FS
> +static void vfio_device_show_fdinfo(struct seq_file *m, struct file *filep)
> +{
> + struct vfio_device_file *df = filep->private_data;
> + struct vfio_device *device = df->device;
> +
> + if (device->ops->show_fdinfo)
> + device->ops->show_fdinfo(device, m);
> +}
> +#endif
> +
> const struct file_operations vfio_device_fops = {
> .owner = THIS_MODULE,
> .open = vfio_device_fops_cdev_open,
> @@ -1363,6 +1375,9 @@ const struct file_operations vfio_device_fops = {
> .unlocked_ioctl = vfio_device_fops_unl_ioctl,
> .compat_ioctl = compat_ptr_ioctl,
> .mmap = vfio_device_fops_mmap,
> +#ifdef CONFIG_PROC_FS
> + .show_fdinfo = vfio_device_show_fdinfo,
> +#endif
> };
>
> static struct vfio_device *vfio_device_from_file(struct file *file)
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 707b00772ce1..54076045a44f 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -16,6 +16,7 @@
> #include <linux/cdev.h>
> #include <uapi/linux/vfio.h>
> #include <linux/iova_bitmap.h>
> +#include <linux/seq_file.h>
>
> struct kvm;
> struct iommufd_ctx;
> @@ -135,6 +136,7 @@ struct vfio_device_ops {
> void (*dma_unmap)(struct vfio_device *vdev, u64 iova, u64 length);
> int (*device_feature)(struct vfio_device *device, u32 flags,
> void __user *arg, size_t argsz);
> + void (*show_fdinfo)(struct vfio_device *device, struct seq_file *m);
Were we to keep this, the comment would need to be updated to describe
it. Thanks,
Alex
> };
>
> #if IS_ENABLED(CONFIG_IOMMUFD)
>
> ---
> base-commit: c1d9dac0db168198b6f63f460665256dedad9b6e
> change-id: 20250623-vfio-fdinfo-767e75a1496a
>
> Best regards,
Powered by blists - more mailing lists