diff options
author | John Levon <john.levon@nutanix.com> | 2021-05-26 12:23:29 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-26 12:23:29 +0100 |
commit | 7aca16a2cf2b00b11fbdb51829b47c3838eeb151 (patch) | |
tree | 1c95ed1f49c1fbaa2fcf21eaa523ca5d71bada35 /test | |
parent | 03edc3a44e635377ac238a009c5c76b90af3c970 (diff) | |
download | libvfio-user-7aca16a2cf2b00b11fbdb51829b47c3838eeb151.zip libvfio-user-7aca16a2cf2b00b11fbdb51829b47c3838eeb151.tar.gz libvfio-user-7aca16a2cf2b00b11fbdb51829b47c3838eeb151.tar.bz2 |
improve request header handling
We should require a non-empty payload for every command type except
VFIO_USER_DEVICE_RESET.
We should also reply to the caller with such failures.
Add some testing for is_valid_header(), and move the fd handling test over to it
too.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
Diffstat (limited to 'test')
-rw-r--r-- | test/mocks.c | 41 | ||||
-rw-r--r-- | test/py/libvfio_user.py | 2 | ||||
-rw-r--r-- | test/py/test_device_set_irqs.py | 6 | ||||
-rw-r--r-- | test/py/test_request_errors.py | 124 | ||||
-rw-r--r-- | test/unit-tests.c | 87 |
5 files changed, 132 insertions, 128 deletions
diff --git a/test/mocks.c b/test/mocks.c index 5a2b55f..4f202d5 100644 --- a/test/mocks.c +++ b/test/mocks.c @@ -62,12 +62,9 @@ static struct function funcs[] = { { .name = "dma_controller_add_region" }, { .name = "dma_controller_remove_region" }, { .name = "dma_controller_unmap_region" }, - { .name = "exec_command" }, - { .name = "get_request_header" }, { .name = "handle_dirty_pages" }, { .name = "process_request" }, { .name = "should_exec_command" }, - { .name = "tran_sock_send_iovec" }, { .name = "migration_region_access_registers" }, { .name = "handle_dirty_pages_get" }, { .name = "handle_device_state" }, @@ -164,24 +161,6 @@ dma_controller_unmap_region(dma_controller_t *dma, check_expected(region); } -int -tran_sock_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(); -} - bool device_is_stopped(struct migration *migration) { @@ -193,26 +172,6 @@ device_is_stopped(struct migration *migration) } int -get_request_header(vfu_ctx_t *vfu_ctx, vfu_msg_t **msgp) -{ - check_expected(vfu_ctx); - check_expected(msgp); - return mock(); -} - -int -exec_command(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg) -{ - if (!is_patched("exec_command")) { - return __real_exec_command(vfu_ctx, msg); - } - check_expected(vfu_ctx); - check_expected(msg); - errno = mock(); - return mock(); -} - -int process_request(vfu_ctx_t *vfu_ctx) { diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index a73c228..31e9767 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -442,7 +442,7 @@ def vfio_user_header(cmd, size, no_reply=False, error=False, error_no=0): global msg_id buf = struct.pack("HHIII", msg_id, cmd, SIZEOF_VFIO_USER_HEADER + size, - 0, error_no) + VFIO_USER_F_TYPE_COMMAND, error_no) msg_id += 1 diff --git a/test/py/test_device_set_irqs.py b/test/py/test_device_set_irqs.py index af3a03e..9e54381 100644 --- a/test/py/test_device_set_irqs.py +++ b/test/py/test_device_set_irqs.py @@ -59,6 +59,12 @@ def test_device_set_irqs_setup(): sock = connect_client(ctx) +def test_device_set_irqs_no_irq_set(): + hdr = vfio_user_header(VFIO_USER_DEVICE_SET_IRQS, size=0) + sock.send(hdr) + vfu_run_ctx(ctx) + get_reply(sock, expect=errno.EINVAL) + def test_device_set_irqs_short_write(): payload = struct.pack("II", 0, 0) diff --git a/test/py/test_request_errors.py b/test/py/test_request_errors.py new file mode 100644 index 0000000..fe6f29e --- /dev/null +++ b/test/py/test_request_errors.py @@ -0,0 +1,124 @@ +# +# Copyright (c) 2021 Nutanix Inc. All rights reserved. +# +# Authors: John Levon <john.levon@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. +# + +from libvfio_user import * +import ctypes as c +import errno +import os +import sys + +ctx = None +sock = None + +argsz = len(vfio_irq_set()) + +def test_request_errors_setup(): + global ctx, sock + + ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) + assert ctx != None + + ret = vfu_pci_init(ctx) + assert ret == 0 + + ret = vfu_setup_device_nr_irqs(ctx, VFU_DEV_MSIX_IRQ, 2048) + assert ret == 0 + + ret = vfu_realize_ctx(ctx) + assert ret == 0 + + sock = connect_client(ctx) + +def test_too_small(): + # struct vfio_user_header + hdr = struct.pack("HHIII", 0xbad1, VFIO_USER_DEVICE_SET_IRQS, + SIZEOF_VFIO_USER_HEADER - 1, VFIO_USER_F_TYPE_COMMAND, 0) + + sock.send(hdr) + vfu_run_ctx(ctx) + get_reply(sock, expect=errno.EINVAL) + +def test_too_large(): + # struct vfio_user_header + hdr = struct.pack("HHIII", 0xbad1, VFIO_USER_DEVICE_SET_IRQS, + SERVER_MAX_MSG_SIZE + 1, VFIO_USER_F_TYPE_COMMAND, 0) + + sock.send(hdr) + vfu_run_ctx(ctx) + get_reply(sock, expect=errno.EINVAL) + +def test_unsolicited_reply(): + # struct vfio_user_header + hdr = struct.pack("HHIII", 0xbad2, VFIO_USER_DEVICE_SET_IRQS, + SIZEOF_VFIO_USER_HEADER, VFIO_USER_F_TYPE_REPLY, 0) + + sock.send(hdr) + vfu_run_ctx(ctx) + get_reply(sock, expect=errno.EINVAL) + +def test_bad_command(): + hdr = vfio_user_header(VFIO_USER_MAX, size=1) + + sock.send(hdr + b'\0') + vfu_run_ctx(ctx) + get_reply(sock, expect=errno.EINVAL) + +def test_no_payload(): + hdr = vfio_user_header(VFIO_USER_DEVICE_SET_IRQS, size=0) + sock.send(hdr) + vfu_run_ctx(ctx) + get_reply(sock, expect=errno.EINVAL) + +def test_bad_request_closes_fds(): + payload = vfio_irq_set(argsz=argsz, flags=VFIO_IRQ_SET_ACTION_TRIGGER | + VFIO_IRQ_SET_DATA_BOOL, index=VFU_DEV_MSIX_IRQ, + start=0, count=1) + + fd1 = eventfd() + fd2 = eventfd() + + hdr = vfio_user_header(VFIO_USER_DEVICE_SET_IRQS, size=len(payload)) + sock.sendmsg([hdr + payload], [(socket.SOL_SOCKET, socket.SCM_RIGHTS, + struct.pack("II", fd1, fd2))]) + vfu_run_ctx(ctx) + get_reply(sock, expect=errno.EINVAL) + + # + # It's a little cheesy, but this is just ensuring no fd's remain open past + # the one we just allocated; i.e. free_msg() freed the fds it got. + # + test_fd = eventfd() + assert test_fd == fd2 + 1 + os.close(test_fd) + + os.close(fd1) + os.close(fd2) + +def test_request_errors_cleanup(): + vfu_destroy_ctx(ctx) diff --git a/test/unit-tests.c b/test/unit-tests.c index e712867..4f39228 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -69,6 +69,7 @@ static vfu_msg_t * mkmsg(enum vfio_user_command cmd, void *data, size_t size) { msg.hdr.cmd = cmd; + msg.hdr.msg_size = size; msg.in_data = data; msg.in_size = size; @@ -681,91 +682,6 @@ test_should_exec_command(UNUSED void **state) assert_true(should_exec_command(&vfu_ctx, 0xbeef)); } -static int -check_request_header_msg(const LargestIntegralType value, - const LargestIntegralType cvalue UNUSED) -{ - vfu_msg_t **msgp = (vfu_msg_t **)value; - - *msgp = malloc(sizeof(msg)); - - assert_non_null(*msgp); - - memcpy(*msgp, &msg, sizeof(msg)); - - return 1; -} - -static int -check_exec_command_msg(const LargestIntegralType value, - const LargestIntegralType cvalue UNUSED) -{ - vfu_msg_t *cmsg = (vfu_msg_t *)value; - - int ret = cmsg->nr_in_fds == ARRAY_SIZE(fds) && - cmsg->in_fds[0] == fds[0] && - cmsg->in_fds[1] == fds[1] && - cmsg->in_data == NULL && - cmsg->in_size == 0 && - memcmp(&cmsg->hdr, &msg.hdr, sizeof (msg.hdr)) == 0; - - consume_fd(cmsg->in_fds, cmsg->nr_in_fds, 0); - - return ret; -} - -/* - * Tests that if if exec_command fails then process_request() frees passed file - * descriptors. - */ -static void -test_process_request_free_passed_fds(void **state UNUSED) -{ - tran_sock_t ts = { .fd = 23, .conn_fd = 24 }; - - mkmsg(VFIO_USER_DMA_MAP, NULL, 0); - - fds[0] = 0xab; - fds[1] = 0xcd; - msg.nr_in_fds = 2; - msg.in_fds = malloc(sizeof(int) * msg.nr_in_fds); - assert_non_null(msg.in_fds); - msg.in_fds[0] = fds[0]; - msg.in_fds[1] = fds[1]; - - vfu_ctx.tran = &tran_sock_ops; - vfu_ctx.tran_data = &ts; - - patch("get_request_header"); - expect_value(get_request_header, vfu_ctx, &vfu_ctx); - expect_check(get_request_header, msgp, check_request_header_msg, NULL); - will_return(get_request_header, 0); - - patch("exec_command"); - expect_value(exec_command, vfu_ctx, &vfu_ctx); - expect_check(exec_command, msg, check_exec_command_msg, NULL); - will_return(exec_command, -1); - will_return(exec_command, EREMOTEIO); - - patch("close"); - expect_value(close, fd, fds[1]); - will_return(close, 0); - - patch("tran_sock_send_iovec"); - expect_value(tran_sock_send_iovec, sock, ts.conn_fd); - expect_any(tran_sock_send_iovec, msg_id); - expect_value(tran_sock_send_iovec, is_reply, true); - expect_any(tran_sock_send_iovec, cmd); - expect_any(tran_sock_send_iovec, iovecs); - expect_any(tran_sock_send_iovec, nr_iovecs); - expect_any(tran_sock_send_iovec, fds); - expect_any(tran_sock_send_iovec, count); - expect_any(tran_sock_send_iovec, err); - will_return(tran_sock_send_iovec, 0); - - assert_int_equal(0, process_request(&vfu_ctx)); -} - static void test_dma_controller_dirty_page_get(void **state UNUSED) { @@ -818,7 +734,6 @@ main(void) cmocka_unit_test_setup(test_device_is_stopped_and_copying, setup), cmocka_unit_test_setup(test_cmd_allowed_when_stopped_and_copying, setup), cmocka_unit_test_setup(test_should_exec_command, setup), - cmocka_unit_test_setup(test_process_request_free_passed_fds, setup), cmocka_unit_test_setup(test_dma_controller_dirty_page_get, setup), }; |