don't leak passed file descriptors on failure

Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>

8 files changed, 316 insertions, 44 deletions

diff --git a/lib/irq.c b/lib/irq.c
index 9c8f105..977f8b2 100644
--- a/lib/irq.c
+++ b/lib/irq.c
@@ -337,7 +337,7 @@ handle_device_get_irq_info(vfu_ctx_t *vfu_ctx, uint32_t size,
 
 int
 handle_device_set_irqs(vfu_ctx_t *vfu_ctx, uint32_t size,
-                       int *fds, int nr_fds, struct vfio_irq_set *irq_set)
+                       int *fds, size_t nr_fds, struct vfio_irq_set *irq_set)
 {
     void *data = NULL;
 
@@ -345,13 +345,17 @@ handle_device_set_irqs(vfu_ctx_t *vfu_ctx, uint32_t size,
     assert(irq_set != NULL);
 
     if (size < sizeof *irq_set || size != irq_set->argsz) {
+        vfu_log(vfu_ctx, VFU_ERR, "bad size %d", size);
         return -EINVAL;
     }
 
     switch (irq_set->flags & VFIO_IRQ_SET_DATA_TYPE_MASK) {
         case VFIO_IRQ_SET_DATA_EVENTFD:
             data = fds;
-            if (nr_fds != (int)irq_set->count) {
+            if (nr_fds != irq_set->count) {
+                vfu_log(vfu_ctx, VFU_ERR,
+                        "bad number of FDs, expected=%u, actual=%d", nr_fds,
+                        irq_set->count);
                 return -EINVAL;
             }
             break;
@@ -360,6 +364,8 @@ handle_device_set_irqs(vfu_ctx_t *vfu_ctx, uint32_t size,
             break;
         default:
             // FIXME?
+            vfu_log(vfu_ctx, VFU_ERR, "invalid IRQ type %d",
+                    irq_set->flags & VFIO_IRQ_SET_DATA_TYPE_MASK);
             return -EINVAL;
     }
 
diff --git a/lib/irq.h b/lib/irq.h
index 5f29e9e..0fed648 100644
--- a/lib/irq.h
+++ b/lib/irq.h
@@ -41,7 +41,7 @@ handle_device_get_irq_info(vfu_ctx_t *vfu_ctx, uint32_t size,
                            struct vfio_irq_info *irq_info_out);
 int
 handle_device_set_irqs(vfu_ctx_t *vfu_ctx, uint32_t size,
-                       int *fds, int nr_fds, struct vfio_irq_set *irq_set);
+                       int *fds, size_t nr_fds, struct vfio_irq_set *irq_set);
 
 #endif /* LIB_VFIO_USER_IRQ_H */
 
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index 3a1bd77..b950ed3 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -57,7 +57,6 @@
 #include "migration.h"
 #include "irq.h"
 
-
 void
 vfu_log(vfu_ctx_t *vfu_ctx, vfu_log_lvl_t lvl, const char *fmt, ...)
 {
@@ -507,24 +506,25 @@ handle_device_get_info(vfu_ctx_t *vfu_ctx, uint32_t size,
  * @vfu_ctx: LM context
  * @size: size, in bytes, of the memory pointed to be @dma_regions
  * @map: whether this is a DMA map operation
- * @fds: array of file descriptors. It's length must equal the number of DMA
-         regions, irrespectively if @nr_fds is 0.
- * @nr_fds: size of above array. It must be either 0 or exactly match
- *          the number of DMA regions in @dma_regions.
+ * @fds: array of file descriptors.
+ * @nr_fds: size of above array.
  * @dma_regions: memory that contains the DMA regions to be mapped/unmapped
  *
- * @returns 0 on success, -errno on failure.
+ * @returns 0 on success, -errno on failure. @nr_fds receives the number of
+ * mappable DMA regions that were successfully added.
  */
 int
 handle_dma_map_or_unmap(vfu_ctx_t *vfu_ctx, uint32_t size, bool map,
-                        int *fds, int nr_fds,
+                        int *fds, size_t *nr_fds,
                         struct vfio_user_dma_region *dma_regions)
 {
     int nr_dma_regions;
-    int ret, i, fdi;
+    int ret, i;
+    size_t fdi;
 
     assert(vfu_ctx != NULL);
     assert(fds != NULL);
+    assert(nr_fds != NULL);
 
     if (vfu_ctx->dma == NULL) {
         return 0;
@@ -536,15 +536,19 @@ handle_dma_map_or_unmap(vfu_ctx_t *vfu_ctx, uint32_t size, bool map,
     }
 
     nr_dma_regions = (int)(size / sizeof(struct vfio_user_dma_region));
+    if (nr_dma_regions == 0) {
+        return 0;
+    }
 
     for (i = 0, fdi = 0; i < nr_dma_regions; i++) {
         if (map) {
             int fd = -1;
             if (dma_regions[i].flags == VFIO_USER_F_DMA_REGION_MAPPABLE) {
-                if (fdi == nr_fds) {
-                    return -EINVAL;
+                if (fdi == *nr_fds) {
+                    ret = -EINVAL;
+                    break;
                 }
-                fd = fds[fdi++];
+                fd = fds[fdi];
             }
 
             ret = dma_controller_add_region(vfu_ctx->dma,
@@ -559,13 +563,16 @@ handle_dma_map_or_unmap(vfu_ctx_t *vfu_ctx, uint32_t size, bool map,
                         dma_regions[i].addr + dma_regions[i].size - 1,
                         dma_regions[i].offset, fd,
                         strerror(-ret));
-            } else {
-                vfu_log(vfu_ctx, VFU_DBG,
-                        "added DMA region %#lx-%#lx offset=%#lx fd=%d",
-                        dma_regions[i].addr,
-                        dma_regions[i].addr + dma_regions[i].size - 1,
-                        dma_regions[i].offset, fd);
+                break;
             }
+            if (dma_regions[i].flags == VFIO_USER_F_DMA_REGION_MAPPABLE) {
+                fdi++;
+            }
+            vfu_log(vfu_ctx, VFU_DBG,
+                    "added DMA region %#lx-%#lx offset=%#lx fd=%d",
+                    dma_regions[i].addr,
+                    dma_regions[i].addr + dma_regions[i].size - 1,
+                    dma_regions[i].offset, fd);
         } else {
             ret = dma_controller_remove_region(vfu_ctx->dma,
                                                dma_regions[i].addr,
@@ -577,12 +584,12 @@ handle_dma_map_or_unmap(vfu_ctx_t *vfu_ctx, uint32_t size, bool map,
                         dma_regions[i].addr,
                         dma_regions[i].addr + dma_regions[i].size - 1,
                         strerror(-ret));
-            } else {
-                vfu_log(vfu_ctx, VFU_DBG,
-                        "removed DMA region %#lx-%#lx",
-                        dma_regions[i].addr,
-                        dma_regions[i].addr + dma_regions[i].size - 1);
+                break;
             }
+            vfu_log(vfu_ctx, VFU_DBG,
+                    "removed DMA region %#lx-%#lx",
+                    dma_regions[i].addr,
+                    dma_regions[i].addr + dma_regions[i].size - 1);
         }
         if (ret < 0) {
             return ret;
@@ -592,7 +599,10 @@ handle_dma_map_or_unmap(vfu_ctx_t *vfu_ctx, uint32_t size, bool map,
                              dma_regions[i].size);
         }
     }
-    return 0;
+    if (map) {
+        *nr_fds = fdi;
+    }
+    return ret;
 }
 
 static int
@@ -810,9 +820,9 @@ validate_header(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size)
  * Returns 0 if there is no command to process, -errno if an error occured, or
  * the number of bytes read.
  */
-static int
+int
 get_next_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, int *fds,
-                 int *nr_fds)
+                 size_t *nr_fds)
 {
     int ret;
 
@@ -841,13 +851,14 @@ get_next_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, int *fds,
     }
     return ret;
 }
+UNIT_TEST_SYMBOL(get_next_command);
+#define get_next_command __wrap_get_next_command
 
-static int
+int
 exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
-             int *fds, int *nr_fds,
+             int *fds, size_t *nr_fds,
              struct iovec *_iovecs, struct iovec **iovecs, size_t *nr_iovecs,
              bool *free_iovec_data)
-
 {
     int ret;
     struct vfio_irq_info irq_info;
@@ -906,7 +917,7 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
         case VFIO_USER_DMA_UNMAP:
             ret = handle_dma_map_or_unmap(vfu_ctx, hdr->msg_size,
                                           hdr->cmd == VFIO_USER_DMA_MAP,
-                                          fds, *nr_fds, cmd_data);
+                                          fds, nr_fds, cmd_data);
             break;
         case VFIO_USER_DEVICE_GET_INFO:
             ret = handle_device_get_info(vfu_ctx, hdr->msg_size, &dev_info);
@@ -940,6 +951,9 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
         case VFIO_USER_DEVICE_SET_IRQS:
             ret = handle_device_set_irqs(vfu_ctx, hdr->msg_size, fds, *nr_fds,
                                          cmd_data);
+            if (ret < 0) {
+                *nr_fds = 0;
+            }
             break;
         case VFIO_USER_REGION_READ:
         case VFIO_USER_REGION_WRITE:
@@ -970,14 +984,16 @@ reply:
     free(cmd_data);
     return ret;
 }
+UNIT_TEST_SYMBOL(exec_command);
+#define exec_command __wrap_exec_command
 
-static int
+int
 process_request(vfu_ctx_t *vfu_ctx)
 {
     struct vfio_user_header hdr = { 0, };
     int ret;
     int *fds = NULL;
-    int nr_fds;
+    size_t nr_fds, _nr_fds, i;
     struct iovec _iovecs[2] = { { 0, } };
     struct iovec *iovecs = NULL;
     size_t nr_iovecs = 0;
@@ -1005,8 +1021,9 @@ process_request(vfu_ctx_t *vfu_ctx)
     if (ret <= 0) {
         return ret;
     }
+    _nr_fds = nr_fds;
 
-    ret = exec_command(vfu_ctx, &hdr, ret, fds, &nr_fds, _iovecs, &iovecs,
+    ret = exec_command(vfu_ctx, &hdr, ret, fds, &_nr_fds, _iovecs, &iovecs,
                        &nr_iovecs, &free_iovec_data);
 
     /*
@@ -1014,6 +1031,9 @@ process_request(vfu_ctx_t *vfu_ctx)
      * in the reply message.
      */
     if (ret < 0) {
+        for (i = _nr_fds; i < nr_fds; i++) {
+            close(fds[i]);
+        }
         vfu_log(vfu_ctx, VFU_ERR, "failed to handle command %d: %s", hdr.cmd,
                 strerror(-ret));
     } else {
diff --git a/lib/private.h b/lib/private.h
index 7ffe099..22c6807 100644
--- a/lib/private.h
+++ b/lib/private.h
@@ -47,7 +47,7 @@ struct transport_ops {
     int (*attach)(vfu_ctx_t*);
     int(*detach)(vfu_ctx_t*);
     int (*get_request)(vfu_ctx_t*, struct vfio_user_header*,
-                       int *fds, int *nr_fds);
+                       int *fds, size_t *nr_fds);
 };
 
 typedef enum {
@@ -143,13 +143,26 @@ region_to_offset(uint32_t region);
 
 int
 handle_dma_map_or_unmap(vfu_ctx_t *vfu_ctx, uint32_t size, bool map,
-                        int *fds, int nr_fds,
+                        int *fds, size_t *nr_fds,
                         struct vfio_user_dma_region *dma_regions);
 
 void
 _dma_controller_do_remove_region(dma_controller_t *dma,
                                  dma_memory_region_t *region);
 
+int
+get_next_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, int *fds,
+                 size_t *nr_fds);
+
+int
+exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
+             int *fds, size_t *nr_fds,
+             struct iovec *_iovecs, struct iovec **iovecs, size_t *nr_iovecs,
+             bool *free_iovec_data);
+
+int
+process_request(vfu_ctx_t *vfu_ctx);
+
 #endif /* LIB_VFIO_USER_PRIVATE_H */
 
 /* ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/lib/tran_sock.c b/lib/tran_sock.c
index ad57ccc..74e50f7 100644
--- a/lib/tran_sock.c
+++ b/lib/tran_sock.c
@@ -651,7 +651,7 @@ close_sock(vfu_ctx_t *vfu_ctx)
 
 static int
 get_request_sock(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
-                 int *fds, int *nr_fds)
+                 int *fds, size_t *nr_fds)
 {
     int ret;
     struct iovec iov = {.iov_base = hdr, .iov_len = sizeof *hdr};
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 822e948..23fdd36 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -46,6 +46,12 @@ target_link_libraries(unit-tests PUBLIC
                       "-Wl,--wrap=_dma_controller_do_remove_region")
 target_link_libraries(unit-tests PUBLIC "-Wl,--wrap=dma_controller_add_region")
 target_link_libraries(unit-tests PUBLIC "-Wl,--wrap=dma_map_region")
+target_link_libraries(unit-tests PUBLIC "-Wl,--wrap=device_is_stopped")
+target_link_libraries(unit-tests PUBLIC "-Wl,--wrap=get_next_command")
+target_link_libraries(unit-tests PUBLIC "-Wl,--wrap=exec_command")
+target_link_libraries(unit-tests PUBLIC "-Wl,--wrap=close")
+target_link_libraries(unit-tests PUBLIC "-Wl,--wrap=vfu_send_iovec")
+target_link_libraries(unit-tests PUBLIC "-Wl,--wrap=free")
 
 enable_testing()
 add_test(NAME unit-tests COMMAND unit-tests)
diff --git a/test/mocks.c b/test/mocks.c
index 7852c1d..ac0a986 100644
--- a/test/mocks.c
+++ b/test/mocks.c
@@ -36,6 +36,7 @@
 
 #include "mocks.h"
 #include "dma.h"
+#include "migration.h"
 
 struct function
 {
@@ -56,7 +57,7 @@ __wrap_dma_controller_add_region(dma_controller_t *dma, dma_addr_t dma_addr,
     check_expected(size);
     check_expected(fd);
     check_expected(offset);
-    return 0;
+    return mock();
 }
 
 void *
@@ -67,7 +68,7 @@ __wrap_dma_map_region(dma_memory_region_t *region, int prot, size_t offset,
     check_expected(prot);
     check_expected(offset);
     check_expected(len);
-    return 0;
+    return mock_ptr_type(void*);
 }
 
 void
@@ -78,11 +79,85 @@ __wrap__dma_controller_do_remove_region(dma_controller_t *dma,
     check_expected(region);
 }
 
+bool
+__wrap_device_is_stopped(struct migration *migr)
+{
+    check_expected(migr);
+    return mock();
+}
+
+int
+__wrap_get_next_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
+                        int *fds, size_t *nr_fds)
+{
+    check_expected(vfu_ctx);
+    check_expected(hdr);
+    check_expected(fds);
+    check_expected(nr_fds);
+    return mock();
+}
+
+int
+__wrap_exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
+                    size_t size, int *fds, size_t *nr_fds,
+                    struct iovec *_iovecs, struct iovec **iovecs,
+                    size_t *nr_iovecs, bool *free_iovec_data)
+{
+    check_expected(vfu_ctx);
+    check_expected(hdr);
+    check_expected(size);
+    check_expected(fds);
+    check_expected(nr_fds);
+    check_expected(_iovecs);
+    check_expected(iovecs);
+    check_expected(nr_iovecs);
+    check_expected(free_iovec_data);
+    return mock();
+}
+
+int
+__wrap_close(int fd)
+{
+    check_expected(fd);
+    return mock();
+}
+
+int
+__wrap_vfu_send_iovec(int sock, uint16_t msg_id, bool is_reply,
+                      enum vfio_user_command cmd,
+                      struct iovec *iovecs, size_t nr_iovecs,
+                      int *fds, int count, int err)
+{
+    check_expected(sock);
+    check_expected(msg_id);
+    check_expected(is_reply);
+    check_expected(cmd);
+    check_expected(iovecs);
+    check_expected(nr_iovecs);
+    check_expected(fds);
+    check_expected(count);
+    check_expected(err);
+    return mock();
+}
+
+void
+__wrap_free(void *ptr)
+{
+    assert(false);
+    check_expected(ptr);
+}
+
 /* FIXME should be something faster than unsorted array, look at tsearch(3). */
 static struct function funcs[] = {
     {.addr = &__wrap_dma_controller_add_region},
     {.addr = &__wrap_dma_map_region},
     {.addr = &__wrap__dma_controller_do_remove_region},
+    {.addr = &__wrap_device_is_stopped},
+    {.addr = &__wrap_get_next_command},
+    {.addr = &__wrap_exec_command},
+    {.addr = &__wrap_close},
+    {.addr = &__wrap_vfu_send_iovec},
+    {.addr = &__wrap_free}
 };
 
 static struct function*
diff --git a/test/unit-tests.c b/test/unit-tests.c
index 19375b8..c09c8a9 100644
--- a/test/unit-tests.c
+++ b/test/unit-tests.c
@@ -37,10 +37,13 @@
 #include <stdio.h>
 #include <assert.h>
 #include <alloca.h>
+#include <string.h>
 
 #include "dma.h"
 #include "libvfio-user.h"
 #include "private.h"
+#include "migration.h"
+#include "tran_sock.h"
 
 static void
 test_dma_map_without_dma(void **state __attribute__((unused)))
@@ -51,8 +54,9 @@ test_dma_map_without_dma(void **state __attribute__((unused)))
         .flags = VFIO_USER_F_DMA_REGION_MAPPABLE
     };
     int fd;
+    size_t nr_fds = 1;
 
-    assert_int_equal(0, handle_dma_map_or_unmap(&vfu_ctx, size, true, &fd, 1, &dma_region));
+    assert_int_equal(0, handle_dma_map_or_unmap(&vfu_ctx, size, true, &fd, &nr_fds, &dma_region));
 }
 
 static void
@@ -65,7 +69,9 @@ test_dma_map_mappable_without_fd(void **state __attribute__((unused)))
         .flags = VFIO_USER_F_DMA_REGION_MAPPABLE
     };
     int fd;
-    assert_int_equal(-EINVAL, handle_dma_map_or_unmap(&vfu_ctx, size, true, &fd, 0, &dma_region));
+    size_t nr_fds = 0;
+
+    assert_int_equal(-EINVAL, handle_dma_map_or_unmap(&vfu_ctx, size, true, &fd, &nr_fds, &dma_region));
 }
 
 static void
@@ -82,14 +88,16 @@ test_dma_map_without_fd(void **state __attribute__((unused)))
         .offset = 0x8badf00d
     };
     int fd;
+    size_t nr_fds = 0;
 
     patch(dma_controller_add_region);
+    will_return(__wrap_dma_controller_add_region, 0);
     expect_value(__wrap_dma_controller_add_region, dma, vfu_ctx.dma);
     expect_value(__wrap_dma_controller_add_region, dma_addr, r.addr);
     expect_value(__wrap_dma_controller_add_region, size, r.size);
     expect_value(__wrap_dma_controller_add_region, fd, -1);
     expect_value(__wrap_dma_controller_add_region, offset, r.offset);
-    assert_int_equal(0, handle_dma_map_or_unmap(&vfu_ctx, size, true, &fd, 0, &r));
+    assert_int_equal(0, handle_dma_map_or_unmap(&vfu_ctx, size, true, &fd, &nr_fds, &r));
 }
 
 /*
@@ -116,8 +124,11 @@ test_dma_add_regions_mixed(void **state __attribute__((unused)))
         }
     };
     int fd = 0x8badf00d;
+    size_t nr_fds = 1;
 
     patch(dma_controller_add_region);
+    will_return(__wrap_dma_controller_add_region, 0);
+    will_return(__wrap_dma_controller_add_region, 0);
     expect_value(__wrap_dma_controller_add_region, dma, vfu_ctx.dma);
     expect_value(__wrap_dma_controller_add_region, dma_addr, r[0].addr);
     expect_value(__wrap_dma_controller_add_region, size, r[0].size);
@@ -129,9 +140,73 @@ test_dma_add_regions_mixed(void **state __attribute__((unused)))
     expect_value(__wrap_dma_controller_add_region, fd, fd);
     expect_value(__wrap_dma_controller_add_region, offset, r[1].offset);
 
-    assert_int_equal(0, handle_dma_map_or_unmap(&vfu_ctx, sizeof r, true, &fd, 1, r));
+    assert_int_equal(0, handle_dma_map_or_unmap(&vfu_ctx, sizeof r, true, &fd, &nr_fds, r));
 }
 
+/*
+ * Tests that handle_dma_map_or_unmap sets the correct number of file
+ * descriptors when failing halfway through.
+ */
+static void
+test_dma_add_regions_mixed_partial_failure(void **state __attribute__((unused)))
+{
+    dma_controller_t dma = { 0 };
+    vfu_ctx_t vfu_ctx = { .dma = &dma };
+    dma.vfu_ctx = &vfu_ctx;
+    struct vfio_user_dma_region r[3] = {
+        [0] = {
+            .addr = 0xdeadbeef,
+            .size = 0x1000,
+            .offset = 0
+        },
+        [1] = {
+            .addr = 0xcafebabe,
+            .size = 0x1000,
+            .offset = 0x1000,
+            .flags = VFIO_USER_F_DMA_REGION_MAPPABLE
+        },
+        [2] = {
+            .addr = 0xbabecafe,
+            .size = 0x1000,
+            .offset = 0x2000,
+            .flags = VFIO_USER_F_DMA_REGION_MAPPABLE
+        }
+    };
+    int fds[] = {0x8badf00d, 0xbad8f00d};
+    size_t nr_fds = 2;
+
+    patch(dma_controller_add_region);
+
+    /* 1st region */
+    expect_value(__wrap_dma_controller_add_region, dma, vfu_ctx.dma);
+    expect_value(__wrap_dma_controller_add_region, dma_addr, r[0].addr);
+    expect_value(__wrap_dma_controller_add_region, size, r[0].size);
+    expect_value(__wrap_dma_controller_add_region, fd, -1);
+    expect_value(__wrap_dma_controller_add_region, offset, r[0].offset);
+    will_return(__wrap_dma_controller_add_region, 0);
+
+    /* 2nd region */
+    expect_value(__wrap_dma_controller_add_region, dma, vfu_ctx.dma);
+    expect_value(__wrap_dma_controller_add_region, dma_addr, r[1].addr);
+    expect_value(__wrap_dma_controller_add_region, size, r[1].size);
+    expect_value(__wrap_dma_controller_add_region, fd, fds[0]);
+    expect_value(__wrap_dma_controller_add_region, offset, r[1].offset);
+    will_return(__wrap_dma_controller_add_region, 0);
+
+    /* 3rd region */
+    expect_value(__wrap_dma_controller_add_region, dma, vfu_ctx.dma);
+    expect_value(__wrap_dma_controller_add_region, dma_addr, r[2].addr);
+    expect_value(__wrap_dma_controller_add_region, size, r[2].size);
+    expect_value(__wrap_dma_controller_add_region, fd, fds[1]);
+    expect_value(__wrap_dma_controller_add_region, offset, r[2].offset);
+    will_return(__wrap_dma_controller_add_region, -0x1234);
+
+    assert_int_equal(-0x1234,
+                     handle_dma_map_or_unmap(&vfu_ctx,
+                                             ARRAY_SIZE(r) * sizeof(struct vfio_user_dma_region),
+                                             true, fds, &nr_fds, r));
+    assert_int_equal(1, nr_fds);
+}
 
 static void
 test_dma_controller_add_region_no_fd(void **state __attribute__((unused)))
@@ -178,6 +253,81 @@ test_dma_controller_remove_region_no_fd(void **state __attribute__((unused)))
 }
 
 /*
+ * Tests that if if exec_command fails then process_request frees passed file
+ * descriptors.
+ */
+static void
+test_process_command_free_passed_fds(void **state __attribute__((unused)))
+{
+    int fds[] = {0xab, 0xcd};
+    int set_fds(const long unsigned int value,
+                const long unsigned int data __attribute__((unused)))
+    {
+        assert(value != 0);
+        memcpy((int*)value, fds, ARRAY_SIZE(fds) * sizeof(int));
+        return 1;
+    }
+    int set_nr_fds(const long unsigned int value,
+                   const long unsigned int data)
+    {
+        int *nr_fds = (int*)value;
+        assert(nr_fds != NULL);
+        if ((void*)data == &get_next_command) {
+            *nr_fds = ARRAY_SIZE(fds);
+        } else if ((void*)data == &exec_command) {
+            *nr_fds = 1;
+        }
+        return 1;
+    }
+
+    vfu_ctx_t vfu_ctx = {
+        .conn_fd = 0xcafebabe,
+        .migration = (struct migration*)0x8badf00d
+    };
+
+    patch(device_is_stopped);
+    expect_value(__wrap_device_is_stopped, migr, vfu_ctx.migration);
+    will_return(__wrap_device_is_stopped, false);
+
+    patch(get_next_command);
+    expect_value(__wrap_get_next_command, vfu_ctx, &vfu_ctx);
+    expect_any(__wrap_get_next_command, hdr);
+    expect_check(__wrap_get_next_command, fds, &set_fds, &get_next_command);
+    expect_check(__wrap_get_next_command, nr_fds, &set_nr_fds, &get_next_command);
+    will_return(__wrap_get_next_command, 0x0000beef);
+
+    patch(exec_command);
+    expect_value(__wrap_exec_command, vfu_ctx, &vfu_ctx);
+    expect_any(__wrap_exec_command, hdr);
+    expect_value(__wrap_exec_command, size, 0x0000beef);
+    expect_any(__wrap_exec_command, fds);
+    expect_check(__wrap_exec_command, nr_fds, &set_nr_fds, &exec_command);
+    expect_any(__wrap_exec_command, _iovecs);
+    expect_any(__wrap_exec_command, iovecs);
+    expect_any(__wrap_exec_command, nr_iovecs);
+    expect_any(__wrap_exec_command, free_iovec_data);
+    will_return(__wrap_exec_command, -0x1234);
+
+    patch(close);
+    expect_value(__wrap_close, fd, 0xcd);
+    will_return(__wrap_close, 0);
+
+    patch(vfu_send_iovec);
+    expect_value(__wrap_vfu_send_iovec, sock, vfu_ctx.conn_fd);
+    expect_any(__wrap_vfu_send_iovec, msg_id);
+    expect_value(__wrap_vfu_send_iovec, is_reply, true);
+    expect_any(__wrap_vfu_send_iovec, cmd);
+    expect_any(__wrap_vfu_send_iovec, iovecs);
+    expect_any(__wrap_vfu_send_iovec, nr_iovecs);
+    expect_any(__wrap_vfu_send_iovec, fds);
+    expect_any(__wrap_vfu_send_iovec, count);
+    expect_any(__wrap_vfu_send_iovec, err);
+    will_return(__wrap_vfu_send_iovec, 0);
+
+    assert_int_equal(0, process_request(&vfu_ctx));
+}
+
+/*
  * FIXME we shouldn't have to specify a setup function explicitly for each unit
  * test, cmocka should provide that. E.g. cmocka_run_group_tests enables us to
  * run a function before/after ALL unit tests have finished, we can extend it
@@ -196,8 +346,10 @@ int main(void)
         cmocka_unit_test_setup(test_dma_map_mappable_without_fd, setup),
         cmocka_unit_test_setup(test_dma_map_without_fd, setup),
         cmocka_unit_test_setup(test_dma_add_regions_mixed, setup),
+        cmocka_unit_test_setup(test_dma_add_regions_mixed_partial_failure, setup),
         cmocka_unit_test_setup(test_dma_controller_add_region_no_fd, setup),
         cmocka_unit_test_setup(test_dma_controller_remove_region_no_fd, setup),
+        cmocka_unit_test_setup(test_process_command_free_passed_fds, setup),
     };
 
     return cmocka_run_group_tests(tests, NULL, NULL);