Fix err path of handle_dma_unmap() (#597)

* initial dma_unmap test

Signed-off-by: John Levon <john.levon@nutanix.com>
Signed-off-by: Swapnil Ingle <swapnil.ingle@nutanix.com>

* Fix err path of handle_dma_unmap()

Set msg->out_size before successful return. Otherwise in case of error
reply path we may endup setting iovecs[1].iov_len with invalid
iovecs[1].iov_base in tran_sock_reply()

Signed-off-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Reviewed-by: John Levon <john.levon@nutanix.com>

3 files changed, 93 insertions, 5 deletions

diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index c9c78d6..4e6b81f 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -544,6 +544,7 @@ int
 handle_dma_unmap(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg,
                  struct vfio_user_dma_unmap *dma_unmap)
 {
+    size_t out_size;
     int ret;
     char rstr[1024];
 
@@ -562,7 +563,7 @@ handle_dma_unmap(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg,
 
     vfu_log(vfu_ctx, LOG_DEBUG, "removing DMA region %s", rstr);
 
-    msg->out_size = sizeof(*dma_unmap);
+    out_size = sizeof(*dma_unmap);
 
     if (dma_unmap->flags == VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP) {
         if (msg->in_size < sizeof(*dma_unmap) + sizeof(*dma_unmap->bitmap)
@@ -583,15 +584,13 @@ handle_dma_unmap(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg,
          * temporary anyway since we're moving dirty page tracking out of
          * the DMA controller.
          */
-        msg->out_size += sizeof(*dma_unmap->bitmap) + dma_unmap->bitmap->size;
+        out_size += sizeof(*dma_unmap->bitmap) + dma_unmap->bitmap->size;
     } else if (dma_unmap->flags != 0) {
         vfu_log(vfu_ctx, LOG_ERR, "bad flags=%#x", dma_unmap->flags);
         return ERROR_INT(ENOTSUP);
     }
 
-
-
-    msg->out_data = malloc(msg->out_size);
+    msg->out_data = malloc(out_size);
     if (msg->out_data == NULL) {
         return ERROR_INT(ENOMEM);
     }
@@ -622,6 +621,7 @@ handle_dma_unmap(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg,
                 "failed to remove DMA region %s: %m", rstr);
         return ERROR_INT(ret);
     }
+    msg->out_size = out_size;
     return ret;
 }
 
diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py
index da7f1fb..c7e3765 100644
--- a/test/py/libvfio_user.py
+++ b/test/py/libvfio_user.py
@@ -337,6 +337,15 @@ class vfio_user_dma_map(Structure):
         ("size", c.c_uint64),
     ]
 
+class vfio_user_dma_unmap(Structure):
+    _pack_ = 1
+    _fields_ = [
+        ("argsz", c.c_uint32),
+        ("flags", c.c_uint32),
+        ("addr", c.c_uint64),
+        ("size", c.c_uint64),
+    ]
+
 class vfu_dma_info_t(Structure):
     _fields_ = [
         ("iova", iovec_t),
diff --git a/test/py/test_dma_unmap.py b/test/py/test_dma_unmap.py
new file mode 100644
index 0000000..0200c43
--- /dev/null
+++ b/test/py/test_dma_unmap.py
@@ -0,0 +1,79 @@
+#
+# Copyright (c) 2021 Nutanix Inc. All rights reserved.
+#
+# Authors: John Levon <john.levon@nutanix.com>
+#          Swapnil Ingle <swapnil.ingle@nutanix.com>
+#
+#  Redistribution and use in source and binary forms, with or without
+#  modification, are permitted provided that the following conditions are met:
+#      * Redistributions of source code must retain the above copyright
+#        notice, this list of conditions and the following disclaimer.
+#      * Redistributions in binary form must reproduce the above copyright
+#        notice, this list of conditions and the following disclaimer in the
+#        documentation and/or other materials provided with the distribution.
+#      * Neither the name of Nutanix nor the names of its contributors may be
+#        used to endorse or promote products derived from this software without
+#        specific prior written permission.
+#
+#  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+#  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+#  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+#  ARE DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
+#  DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+#  (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+#  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+#  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+#  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+#  OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+#  DAMAGE.
+#
+
+import ctypes
+import errno
+from libvfio_user import *
+import tempfile
+
+ctx = None
+sock = None
+
+def test_dma_unmap_setup():
+    global ctx, sock
+
+    ctx = prepare_ctx_for_dma()
+    assert ctx != None
+    payload = struct.pack("II", 0, 0)
+
+    sock = connect_client(ctx)
+
+    payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()),
+        flags=(VFIO_USER_F_DMA_REGION_READ |
+               VFIO_USER_F_DMA_REGION_WRITE),
+        offset=0, addr=0x1000, size=4096)
+
+    msg(ctx, sock, VFIO_USER_DMA_MAP, payload)
+
+def test_dma_unmap_short_write():
+
+    payload = struct.pack("II", 0, 0)
+
+    msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL)
+
+def test_dma_unmap_bad_argsz():
+
+    payload = vfio_user_dma_unmap(argsz=8, flags=0x2323, addr=0x1000, size=4096)
+
+def test_dma_unmap_invalid_flags():
+
+    payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()),
+                                  flags=0x2323, addr=0x1000, size=4096)
+    msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.ENOTSUP)
+
+def test_dma_unmap():
+
+    payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()),
+                                  flags=0, addr=0x1000, size=4096)
+    msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload)
+
+def test_dma_unmap_cleanup():
+    disconnect_client(ctx, sock)
+    vfu_destroy_ctx(ctx)