[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v3 05/19] Add validation ops vector
From: |
Alex Williamson |
Subject: |
Re: [RFC v3 05/19] Add validation ops vector |
Date: |
Fri, 19 Nov 2021 15:42:19 -0700 |
Add a prefix on Subject: please. Same for previous in series.
On Mon, 8 Nov 2021 16:46:33 -0800
John Johnson <john.g.johnson@oracle.com> wrote:
> Validates cases where the return values aren't fully trusted
> (prep work for vfio-user, where the return values from the
> remote process aren't trusted)
>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> ---
> include/hw/vfio/vfio-common.h | 21 ++++++++++++++
> hw/vfio/pci.c | 67
> +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 88 insertions(+)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 43fa948..c0dbbfb 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -125,6 +125,7 @@ typedef struct VFIOHostDMAWindow {
>
> typedef struct VFIODeviceOps VFIODeviceOps;
> typedef struct VFIODevIO VFIODevIO;
> +typedef struct VFIOValidOps VFIOValidOps;
>
> typedef struct VFIODevice {
> QLIST_ENTRY(VFIODevice) next;
> @@ -141,6 +142,7 @@ typedef struct VFIODevice {
> bool enable_migration;
> VFIODeviceOps *ops;
> VFIODevIO *io_ops;
> + VFIOValidOps *valid_ops;
> unsigned int num_irqs;
> unsigned int num_regions;
> unsigned int flags;
> @@ -214,6 +216,25 @@ struct VFIOContIO {
> extern VFIODevIO vfio_dev_io_ioctl;
> extern VFIOContIO vfio_cont_io_ioctl;
>
> +/*
> + * This ops vector allows for bus-specific verification
> + * routines in cases where the server may not be fully
> + * trusted.
> + */
> +struct VFIOValidOps {
> + int (*validate_get_info)(VFIODevice *vdev, struct vfio_device_info
> *info);
> + int (*validate_get_region_info)(VFIODevice *vdev,
> + struct vfio_region_info *info, int *fd);
> + int (*validate_get_irq_info)(VFIODevice *vdev, struct vfio_irq_info
> *info);
> +};
> +
> +#define VDEV_VALID_INFO(vdev, info) \
> + ((vdev)->valid_ops->validate_get_info((vdev), (info)))
> +#define VDEV_VALID_REGION_INFO(vdev, info, fd) \
> + ((vdev)->valid_ops->validate_get_region_info((vdev), (info), (fd)))
> +#define VDEV_VALID_IRQ_INFO(vdev, irq) \
> + ((vdev)->valid_ops->validate_get_irq_info((vdev), (irq)))
> +
> #endif /* CONFIG_LINUX */
>
> typedef struct VFIOGroup {
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 28f21f8..6e2ce35 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3371,3 +3371,70 @@ static void register_vfio_pci_dev_type(void)
> }
>
> type_init(register_vfio_pci_dev_type)
> +
> +
> +/*
> + * PCI validation ops - used when return values need
> + * validation before use
> + */
> +
> +static int vfio_pci_valid_info(VFIODevice *vbasedev,
> + struct vfio_device_info *info)
> +{
> + /* must be PCI */
> + if ((info->flags & VFIO_DEVICE_FLAGS_PCI) == 0) {
> + return -EINVAL;
> + }
> + /* only other valid flag is reset */
> + if (info->flags & ~(VFIO_DEVICE_FLAGS_PCI | VFIO_DEVICE_FLAGS_RESET)) {
> + return -EINVAL;
> + }
This means QEMU vfio-pci breaks on any extension of the flags field.
> + /* account for extra migration region */
> + if (info->num_regions > VFIO_PCI_NUM_REGIONS + 1) {
> + return -EINVAL;
> + }
This is also invalid, there can be device specific regions beyond
migration.
> + if (info->num_irqs > VFIO_PCI_NUM_IRQS) {
> + return -EINVAL;
> + }
And device specific IRQs.
> + return 0;
> +}
> +
> +static int vfio_pci_valid_region_info(VFIODevice *vbasedev,
> + struct vfio_region_info *info,
> + int *fd)
> +{
> + if (info->flags & ~(VFIO_REGION_INFO_FLAG_READ |
> + VFIO_REGION_INFO_FLAG_WRITE |
> + VFIO_REGION_INFO_FLAG_MMAP |
> + VFIO_REGION_INFO_FLAG_CAPS)) {
> + return -EINVAL;
> + }
Similarly, this allows zero future extensions. Notice for instance how
the CAPS flag was added later as a backwards compatible extension.
> + if (info->index > vbasedev->num_regions) {
> + return -EINVAL;
> + }
> + /* cap_offset in valid area */
> + if ((info->flags & VFIO_REGION_INFO_FLAG_CAPS) &&
> + (info->cap_offset < sizeof(*info) || info->cap_offset >
> info->argsz)) {
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int vfio_pci_valid_irq_info(VFIODevice *vbasedev,
> + struct vfio_irq_info *info)
> +{
> + if (info->flags & ~(VFIO_IRQ_INFO_EVENTFD | VFIO_IRQ_INFO_MASKABLE |
> + VFIO_IRQ_INFO_AUTOMASKED | VFIO_IRQ_INFO_NORESIZE)) {
> + return -EINVAL;
> + }
Similarly, nak. Thanks,
Alex
> + if (info->index > vbasedev->num_irqs) {
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +struct VFIOValidOps vfio_pci_valid_ops = {
> + .validate_get_info = vfio_pci_valid_info,
> + .validate_get_region_info = vfio_pci_valid_region_info,
> + .validate_get_irq_info = vfio_pci_valid_irq_info,
> +};
- [RFC v3 09/19] vfio-user: define socket send functions, (continued)
- [RFC v3 09/19] vfio-user: define socket send functions, John Johnson, 2021/11/08
- [RFC v3 03/19] Add container IO ops vector, John Johnson, 2021/11/08
- [RFC v3 02/19] vfio-user: add VFIO base abstract class, John Johnson, 2021/11/08
- [RFC v3 07/19] vfio-user: connect vfio proxy to remote server, John Johnson, 2021/11/08
- [RFC v3 10/19] vfio-user: get device info, John Johnson, 2021/11/08
- [RFC v3 08/19] vfio-user: define socket receive functions, John Johnson, 2021/11/08
- [RFC v3 05/19] Add validation ops vector, John Johnson, 2021/11/08
- Re: [RFC v3 05/19] Add validation ops vector,
Alex Williamson <=
- [RFC v3 15/19] vfio-user: proxy container connect/disconnect, John Johnson, 2021/11/08
- [RFC v3 18/19] vfio-user: pci reset, John Johnson, 2021/11/08
- [RFC v3 12/19] vfio-user: region read/write, John Johnson, 2021/11/08
- [RFC v3 14/19] vfio-user: get and set IRQs, John Johnson, 2021/11/08
- [RFC v3 19/19] vfio-user: migration support, John Johnson, 2021/11/08
- [RFC v3 16/19] vfio-user: dma map/unmap operations, John Johnson, 2021/11/08
- [RFC v3 01/19] vfio-user: introduce vfio-user protocol specification, John Johnson, 2021/11/08