allow -1 file descriptor for ioregionfd (#727)

Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Reviewed-by: John Levon <john.levon@nutanix.com>

3 files changed, 111 insertions, 1 deletions

diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index 5a6269a..94524a2 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -483,7 +483,6 @@ vfu_create_ioeventfd(vfu_ctx_t *vfu_ctx, uint32_t region_idx, int fd,
     vfu_reg_info_t *vfu_reg;
 
     assert(vfu_ctx != NULL);
-    assert(fd >= 0);
 
 #ifndef SHADOW_IOEVENTFD
     if (shadow_fd != -1) {
@@ -542,6 +541,15 @@ free_regions(vfu_ctx_t *vfu_ctx)
  * This function is used to add fd's to the fd return array and gives you back
  * the index of the fd that has been added. If the fd is already present it will
  * return the index to that duplicate fd to reduce the number of fd's sent.
+ * The fd must be a valid fd or -1, any other negative value is not permitted.
+ *
+ * out_fds: an array where the fd is stored
+ * nr_out_fds: pointer to memory that contains the size of the array
+ * fd_search: the fd to add
+ *
+ * returns: the array index where the fd is added to, can be the index of an
+ *  existing fd if this is a duplicate fd. If the fd is -1 then the function
+ *  returns -1.
  */
 static int
 add_fd_index(int *out_fds, size_t *nr_out_fds, int fd_search)
@@ -551,6 +559,11 @@ add_fd_index(int *out_fds, size_t *nr_out_fds, int fd_search)
     assert(out_fds != NULL);
     assert(nr_out_fds != NULL);
 
+    assert(fd_search >= -1);
+    if (fd_search == -1) {
+        return -1;
+    }
+
     for (i = 0; i < *nr_out_fds; i++) {
         if (out_fds[i] == fd_search) {
             return i;
diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py
index 73fad5a..8848dbf 100644
--- a/test/py/libvfio_user.py
+++ b/test/py/libvfio_user.py
@@ -50,6 +50,7 @@ from math import log2
 PAGE_SIZE = getpagesize()
 PAGE_SHIFT = int(log2(PAGE_SIZE))
 
+UINT32_MAX = 0xffffffff
 UINT64_MAX = 18446744073709551615
 
 # from linux/pci_regs.h and linux/pci_defs.h
@@ -1237,4 +1238,10 @@ def fail_with_errno(err):
     return side_effect
 
 
+def fds_are_same(fd1: int, fd2: int) -> bool:
+    s1 = os.stat(fd1)
+    s2 = os.stat(fd2)
+    return s1.st_dev == s2.st_dev and s1.st_ino == s2.st_ino
+
+
 # ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: #
diff --git a/test/py/test_device_get_region_io_fds.py b/test/py/test_device_get_region_io_fds.py
index b9c8d93..63b433f 100644
--- a/test/py/test_device_get_region_io_fds.py
+++ b/test/py/test_device_get_region_io_fds.py
@@ -324,4 +324,94 @@ def test_device_get_region_info_cleanup():
         os.close(i)
     vfu_destroy_ctx(ctx)
 
+
+def test_device_get_region_io_fds_invalid_fd():
+    """Tests that an ioregionfd where fd is -1 is a legitimate ioregionfd."""
+    ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB)
+    assert ctx is not None
+
+    ret = vfu_setup_region(ctx, index=VFU_PCI_DEV_BAR0_REGION_IDX, size=0x1000,
+                           flags=(VFU_REGION_FLAG_RW))
+    assert ret == 0
+
+    ret = vfu_realize_ctx(ctx)
+    assert ret == 0
+
+    sock = connect_client(ctx)
+
+    fds = []
+
+    # use valid fd
+    fd = eventfd(0, 0)
+    fds.append(fd)
+    assert vfu_create_ioeventfd(ctx, VFU_PCI_DEV_BAR0_REGION_IDX, fd,
+                                0 * IOEVENT_SIZE, IOEVENT_SIZE, 0, 0) != -1
+
+    # use -1 fd
+    assert vfu_create_ioeventfd(ctx, VFU_PCI_DEV_BAR0_REGION_IDX, -1,
+                                1 * IOEVENT_SIZE, IOEVENT_SIZE, 0, 0) != -1
+
+    # use another valid fd
+    fd = eventfd(0, 0)
+    fds.append(fd)
+    assert vfu_create_ioeventfd(ctx, VFU_PCI_DEV_BAR0_REGION_IDX, fd,
+                                2 * IOEVENT_SIZE, IOEVENT_SIZE, 0, 0) != -1
+
+    # use -1 fd
+    assert vfu_create_ioeventfd(ctx, VFU_PCI_DEV_BAR0_REGION_IDX, -1,
+                                3 * IOEVENT_SIZE, IOEVENT_SIZE, 0, 0) != -1
+
+    # use duplicate valid fd
+    assert vfu_create_ioeventfd(ctx, VFU_PCI_DEV_BAR0_REGION_IDX, fds[1],
+                                4 * IOEVENT_SIZE, IOEVENT_SIZE, 0, 0) != -1
+
+    payload = vfio_user_region_io_fds_request(
+                argsz=len(vfio_user_region_io_fds_reply()) +
+                len(vfio_user_sub_region_ioeventfd()) * 5, flags=0,
+                index=VFU_PCI_DEV_BAR0_REGION_IDX, count=0)
+
+    newfds, ret = msg_fds(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS,
+                          payload, expect=0)
+
+    # two unique fds
+    assert len(newfds) == 2
+    reply, ret = vfio_user_region_io_fds_reply.pop_from_buffer(ret)
+
+    # five ioregionfds
+    assert reply.count == 5
+    ioevents = []
+    for i in range(0, reply.count):
+        ioevent, ret = vfio_user_sub_region_ioeventfd.pop_from_buffer(ret)
+        ioevents.append(ioevent)
+
+    # TODO this assumes that ioregionfds are returned in the reverse order
+    # they're created. It should be straightforward to compare based on IOVA.
+    assert ioevents[0].fd_index == 0
+    assert ioevents[0].gpa_offset == 4 * IOEVENT_SIZE
+    assert ioevents[0].size == IOEVENT_SIZE
+    assert fds_are_same(newfds[0], fds[0])
+
+    assert ioevents[1].fd_index == UINT32_MAX
+    assert ioevents[1].gpa_offset == 3 * IOEVENT_SIZE
+    assert ioevents[1].size == IOEVENT_SIZE
+
+    assert ioevents[2].fd_index == 0
+    assert ioevents[2].gpa_offset == 2 * IOEVENT_SIZE
+    assert ioevents[2].size == IOEVENT_SIZE
+    assert fds_are_same(newfds[0], fds[0])
+
+    assert ioevents[3].fd_index == UINT32_MAX
+    assert ioevents[3].gpa_offset == 1 * IOEVENT_SIZE
+    assert ioevents[3].size == IOEVENT_SIZE
+
+    assert ioevents[4].fd_index == 1
+    assert ioevents[4].gpa_offset == 0 * IOEVENT_SIZE
+    assert ioevents[4].size == IOEVENT_SIZE
+    assert fds_are_same(newfds[1], fds[1])
+
+    # cleanup
+    for fd in fds:
+        os.close(fd)
+    vfu_destroy_ctx(ctx)
+
 # ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: #