diff options
author | John Levon <john.levon@nutanix.com> | 2021-02-16 16:11:35 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-02-16 16:11:35 +0000 |
commit | b4baf039588830dfd580a59e7d05efbc65cb6d03 (patch) | |
tree | e54da2a141a202362c3313200de16d4bef35ca9b | |
parent | 4b8d6d3898c28ff87f32b59e48e7e73158e786f8 (diff) | |
download | libvfio-user-b4baf039588830dfd580a59e7d05efbc65cb6d03.zip libvfio-user-b4baf039588830dfd580a59e7d05efbc65cb6d03.tar.gz libvfio-user-b4baf039588830dfd580a59e7d05efbc65cb6d03.tar.bz2 |
fix DEVICE_GET_INFO specification and handling (#344)
The specification for DEVICE_GET_INFO differed from the implementation. After
some discussion, fix the spec such that the struct should be passed in with
->argsz set.
As it happened, the implementation was also wrong: we weren't actually checking
the incoming ->argsz for validation, but we should.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
-rw-r--r-- | docs/vfio-user.rst | 11 | ||||
-rw-r--r-- | lib/libvfio-user.c | 27 | ||||
-rw-r--r-- | lib/private.h | 3 | ||||
-rw-r--r-- | test/unit-tests.c | 47 |
4 files changed, 51 insertions, 37 deletions
diff --git a/docs/vfio-user.rst b/docs/vfio-user.rst index e3adc7a..f4562ef 100644 --- a/docs/vfio-user.rst +++ b/docs/vfio-user.rst @@ -714,7 +714,7 @@ Message format +--------------+----------------------------+ | Command | 4 | +--------------+----------------------------+ -| Message size | 16 in command, 32 in reply | +| Message size | 32 | +--------------+----------------------------+ | Flags | Reply bit set in reply | +--------------+----------------------------+ @@ -724,9 +724,8 @@ Message format +--------------+----------------------------+ This command message is sent by the client to the server to query for basic -information about the device. Only the message header is needed in the command -message. The VFIO device info structure is defined in ``<linux/vfio.h>`` -(``struct vfio_device_info``). +information about the device. The VFIO device info structure is defined in +``<linux/vfio.h>`` (``struct vfio_device_info``). VFIO device info format ^^^^^^^^^^^^^^^^^^^^^^^ @@ -751,7 +750,9 @@ VFIO device info format | num_irqs | 28 | 4 | +-------------+--------+--------------------------+ -* *argsz* is the size of the VFIO device info structure. +* *argsz* is the size of the VFIO device info structure. This is the only field +that should be set to non-zero in the request, identifying the client's expected +size. Currently this is a fixed value. * *flags* contains the following device attributes. * VFIO_DEVICE_FLAGS_RESET indicates that the device supports the diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index 2dccd62..0125cef 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -427,23 +427,27 @@ handle_device_get_region_info(vfu_ctx_t *vfu_ctx, uint32_t size, } int -handle_device_get_info(vfu_ctx_t *vfu_ctx, uint32_t size, - struct vfio_device_info *dev_info) +handle_device_get_info(vfu_ctx_t *vfu_ctx, uint32_t in_size, + struct vfio_device_info *in_dev_info, + struct vfio_device_info *out_dev_info) { assert(vfu_ctx != NULL); - assert(dev_info != NULL); + assert(in_dev_info != NULL); + assert(out_dev_info != NULL); - if (size < sizeof *dev_info) { + if (in_size < sizeof (*in_dev_info) || + in_dev_info->argsz < sizeof (*in_dev_info)) { return -EINVAL; } - dev_info->argsz = sizeof *dev_info; - dev_info->flags = VFIO_DEVICE_FLAGS_PCI | VFIO_DEVICE_FLAGS_RESET; - dev_info->num_regions = vfu_ctx->nr_regions; - dev_info->num_irqs = VFU_DEV_NUM_IRQS; + out_dev_info->argsz = sizeof (*in_dev_info); + out_dev_info->flags = VFIO_DEVICE_FLAGS_PCI | VFIO_DEVICE_FLAGS_RESET; + out_dev_info->num_regions = vfu_ctx->nr_regions; + out_dev_info->num_irqs = VFU_DEV_NUM_IRQS; - vfu_log(vfu_ctx, LOG_DEBUG, "sent devinfo flags %#x, num_regions %d, num_irqs" - " %d", dev_info->flags, dev_info->num_regions, dev_info->num_irqs); + vfu_log(vfu_ctx, LOG_DEBUG, "devinfo flags %#x, num_regions %d, " + "num_irqs %d", out_dev_info->flags, out_dev_info->num_regions, + out_dev_info->num_irqs); return 0; } @@ -789,7 +793,8 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size, ret = -ENOMEM; break; } - ret = handle_device_get_info(vfu_ctx, cmd_data_size, dev_info); + ret = handle_device_get_info(vfu_ctx, cmd_data_size, cmd_data, + dev_info); if (ret >= 0) { _iovecs[1].iov_base = dev_info; _iovecs[1].iov_len = dev_info->argsz; diff --git a/lib/private.h b/lib/private.h index 787cae9..edd7b68 100644 --- a/lib/private.h +++ b/lib/private.h @@ -177,7 +177,8 @@ consume_fd(int *fds, size_t nr_fds, size_t index); int handle_device_get_info(vfu_ctx_t *vfu_ctx, uint32_t size, - struct vfio_device_info *dev_info); + struct vfio_device_info *in_dev_info, + struct vfio_device_info *out_dev_info); long dev_get_reginfo(vfu_ctx_t *vfu_ctx, uint32_t index, uint32_t argsz, diff --git a/test/unit-tests.c b/test/unit-tests.c index 1fdd241..eda8c49 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -989,16 +989,19 @@ test_pci_ext_caps(void **state __attribute__((unused))) } static void -test_device_get_info(void **state __attribute__((unused))) +test_device_get_info(void **state UNUSED) { - vfu_ctx_t vfu_ctx = { .nr_regions = 0xdeadbeef}; - struct vfio_device_info d = { 0 }; - - assert_int_equal(0, handle_device_get_info(&vfu_ctx, sizeof d, &d)); - assert_int_equal(sizeof d, d.argsz); - assert_int_equal(VFIO_DEVICE_FLAGS_PCI | VFIO_DEVICE_FLAGS_RESET, d.flags); - assert_int_equal(vfu_ctx.nr_regions, d.num_regions); - assert_int_equal(VFU_DEV_NUM_IRQS, d.num_irqs); + vfu_ctx_t vfu_ctx = { .nr_regions = 0xdeadbeef }; + struct vfio_device_info d_in = { .argsz = sizeof (d_in) }; + struct vfio_device_info d_out; + + assert_int_equal(0, handle_device_get_info(&vfu_ctx, sizeof (d_in), + &d_in, &d_out)); + assert_int_equal(sizeof (d_out), d_out.argsz); + assert_int_equal(VFIO_DEVICE_FLAGS_PCI | VFIO_DEVICE_FLAGS_RESET, + d_out.flags); + assert_int_equal(vfu_ctx.nr_regions, d_out.num_regions); + assert_int_equal(VFU_DEV_NUM_IRQS, d_out.num_irqs); } /* @@ -1006,20 +1009,24 @@ test_device_get_info(void **state __attribute__((unused))) * with more fields. */ static void -test_device_get_info_compat(void **state __attribute__((unused))) +test_device_get_info_compat(void **state UNUSED) { - vfu_ctx_t vfu_ctx = { .nr_regions = 0xdeadbeef}; - struct vfio_device_info d = { 0 }; - - /* more fields */ - assert_int_equal(0, handle_device_get_info(&vfu_ctx, (sizeof d) + 1, &d)); - assert_int_equal(sizeof d, d.argsz); - assert_int_equal(VFIO_DEVICE_FLAGS_PCI | VFIO_DEVICE_FLAGS_RESET, d.flags); - assert_int_equal(vfu_ctx.nr_regions, d.num_regions); - assert_int_equal(VFU_DEV_NUM_IRQS, d.num_irqs); + vfu_ctx_t vfu_ctx = { .nr_regions = 0xdeadbeef }; + struct vfio_device_info d_in = { .argsz = sizeof (d_in) + 1 }; + struct vfio_device_info d_out; + + assert_int_equal(0, handle_device_get_info(&vfu_ctx, sizeof (d_in) + 1, + &d_in, &d_out)); + assert_int_equal(sizeof (d_out), d_out.argsz); + assert_int_equal(VFIO_DEVICE_FLAGS_PCI | VFIO_DEVICE_FLAGS_RESET, + d_out.flags); + assert_int_equal(vfu_ctx.nr_regions, d_out.num_regions); + assert_int_equal(VFU_DEV_NUM_IRQS, d_out.num_irqs); /* fewer fields */ - assert_int_equal(-EINVAL, handle_device_get_info(&vfu_ctx, (sizeof d) - 1, &d)); + assert_int_equal(-EINVAL, + handle_device_get_info(&vfu_ctx, (sizeof (d_in)) - 1, + &d_in, &d_out)); } |