15118 bhyve too strict over vmm_data lengths

Reviewed by: Andy Fiddaman <illumos@fiddaman.net>
Reviewed-by: Vitaliy Gusev <gusev.vitaliy@gmail.com>
Reviewed by: Jordan Paige Hendricks <jordan@oxidecomputer.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

11 files changed, 131 insertions, 17 deletions

diff --git a/usr/src/pkg/manifests/system-bhyve-tests.p5m b/usr/src/pkg/manifests/system-bhyve-tests.p5m
index 1a91b5a580..dcb43b3d7b 100644
--- a/usr/src/pkg/manifests/system-bhyve-tests.p5m
+++ b/usr/src/pkg/manifests/system-bhyve-tests.p5m
@@ -57,6 +57,7 @@ dir  path=opt/bhyve-tests/tests/vmm
 file path=opt/bhyve-tests/tests/vmm/auto_destruct mode=0555
 file path=opt/bhyve-tests/tests/vmm/check_iommu mode=0555
 file path=opt/bhyve-tests/tests/vmm/cpuid_ioctl mode=0555
+file path=opt/bhyve-tests/tests/vmm/datarw_constraints mode=0555
 file path=opt/bhyve-tests/tests/vmm/default_capabs mode=0555
 file path=opt/bhyve-tests/tests/vmm/drv_hold mode=0555
 file path=opt/bhyve-tests/tests/vmm/fpu_getset mode=0555
diff --git a/usr/src/test/bhyve-tests/runfiles/default.run b/usr/src/test/bhyve-tests/runfiles/default.run
index ee15dd8854..d674466b61 100644
--- a/usr/src/test/bhyve-tests/runfiles/default.run
+++ b/usr/src/test/bhyve-tests/runfiles/default.run
@@ -24,6 +24,7 @@ user = root
 tests = [
 	'auto_destruct',
 	'cpuid_ioctl',
+	'datarw_constraints',
 	'default_capabs',
 	'drv_hold',
 	'fpu_getset',
diff --git a/usr/src/test/bhyve-tests/tests/vmm/Makefile b/usr/src/test/bhyve-tests/tests/vmm/Makefile
index ee63dd537c..c8d2be3d8b 100644
--- a/usr/src/test/bhyve-tests/tests/vmm/Makefile
+++ b/usr/src/test/bhyve-tests/tests/vmm/Makefile
@@ -26,7 +26,8 @@ PROG =	mem_partial \
 	self_destruct \
 	drv_hold \
 	cpuid_ioctl \
-	default_capabs
+	default_capabs \
+	datarw_constraints
 
 COMMON_OBJS =	common.o
 CLEAN_OBJS =	$(PROG:%=%.o)
diff --git a/usr/src/test/bhyve-tests/tests/vmm/datarw_constraints.c b/usr/src/test/bhyve-tests/tests/vmm/datarw_constraints.c
new file mode 100644
index 0000000000..82b13e2847
--- /dev/null
+++ b/usr/src/test/bhyve-tests/tests/vmm/datarw_constraints.c
@@ -0,0 +1,111 @@
+/*
+ * This file and its contents are supplied under the terms of the
+ * Common Development and Distribution License ("CDDL"), version 1.0.
+ * You may only use this file in accordance with the terms of version
+ * 1.0 of the CDDL.
+ *
+ * A full copy of the text of the CDDL should have accompanied this
+ * source.  A copy of the CDDL is also available via the Internet at
+ * http://www.illumos.org/license/CDDL.
+ */
+
+/*
+ * Copyright 2022 Oxide Computer Company
+ */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <libgen.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <err.h>
+#include <assert.h>
+#include <sys/sysmacros.h>
+#include <stdbool.h>
+
+#include <sys/vmm.h>
+#include <sys/vmm_dev.h>
+#include <sys/vmm_data.h>
+#include <vmmapi.h>
+
+#include "common.h"
+
+static void
+should_eq_u32(const char *field_name, uint32_t a, uint32_t b)
+{
+	if (a != b) {
+		errx(EXIT_FAILURE, "unexpected %s %u != %u",
+		    field_name, a, b);
+	}
+}
+
+int
+main(int argc, char *argv[])
+{
+	const char *suite_name = basename(argv[0]);
+	struct vmctx *ctx;
+
+	ctx = create_test_vm(suite_name);
+	if (ctx == NULL) {
+		errx(EXIT_FAILURE, "could not open test VM");
+	}
+
+	/*
+	 * Check that vmm_data import/export facility is robust in the face of
+	 * potentially invalid inputs
+	 */
+	const int vmfd = vm_get_device_fd(ctx);
+
+	uint8_t buf[sizeof (struct vdi_atpic_v1) + sizeof (int)];
+	struct vm_data_xfer vdx = {
+		.vdx_class = VDC_ATPIC,
+		.vdx_version = 1,
+		.vdx_len = sizeof (struct vdi_atpic_v1),
+		.vdx_data = buf,
+	};
+
+	/* Attempt a valid-sized read first */
+	if (ioctl(vmfd, VM_DATA_READ, &vdx) != 0) {
+		err(EXIT_FAILURE, "valid vmm_dat_read failed");
+	}
+	should_eq_u32("vdx_result_len", vdx.vdx_result_len,
+	    sizeof (struct vdi_atpic_v1));
+
+	/* ... then too-small ... */
+	vdx.vdx_len = sizeof (struct vdi_atpic_v1) - sizeof (int);
+	vdx.vdx_result_len = 0;
+	if (ioctl(vmfd, VM_DATA_READ, &vdx) == 0) {
+		errx(EXIT_FAILURE, "invalid vmm_dat_read should have failed");
+	}
+	int error = errno;
+	if (error != ENOSPC) {
+		errx(EXIT_FAILURE, "expected ENOSPC errno, got %d", error);
+	}
+	/* the "correct" vdx_result_len should still be communicated out */
+	should_eq_u32("vdx_result_len", vdx.vdx_result_len,
+	    sizeof (struct vdi_atpic_v1));
+
+	/*
+	 * ... and too-big to round it out.
+	 *
+	 * This should pass, but still set vdx_result_len to the actual length
+	 */
+	vdx.vdx_len = sizeof (struct vdi_atpic_v1) + sizeof (int);
+	vdx.vdx_result_len = 0;
+	if (ioctl(vmfd, VM_DATA_READ, &vdx) != 0) {
+		err(EXIT_FAILURE, "too-large (but valid) vmm_dat_read failed");
+	}
+	should_eq_u32("vdx_result_len", vdx.vdx_result_len,
+	    sizeof (struct vdi_atpic_v1));
+
+	/*
+	 * The vmm_data_write paths should also be tested, but not until they
+	 * are exposed to the general public without requring mdb -kw settings.
+	 */
+
+	vm_destroy(ctx);
+	(void) printf("%s\tPASS\n", suite_name);
+	return (EXIT_SUCCESS);
+}
diff --git a/usr/src/uts/intel/io/vmm/io/vatpic.c b/usr/src/uts/intel/io/vmm/io/vatpic.c
index 3113c0fa48..8cbb26d3d6 100644
--- a/usr/src/uts/intel/io/vmm/io/vatpic.c
+++ b/usr/src/uts/intel/io/vmm/io/vatpic.c
@@ -803,7 +803,7 @@ vatpic_data_read(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_ATPIC);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_atpic_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_atpic_v1));
 
 	struct vatpic *vatpic = datap;
 	struct vdi_atpic_v1 *out = req->vdr_data;
@@ -864,7 +864,7 @@ vatpic_data_write(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_ATPIC);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_atpic_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_atpic_v1));
 
 	struct vatpic *vatpic = datap;
 	const struct vdi_atpic_v1 *src = req->vdr_data;
diff --git a/usr/src/uts/intel/io/vmm/io/vatpit.c b/usr/src/uts/intel/io/vmm/io/vatpit.c
index 99c4035e1c..b6218935db 100644
--- a/usr/src/uts/intel/io/vmm/io/vatpit.c
+++ b/usr/src/uts/intel/io/vmm/io/vatpit.c
@@ -502,7 +502,7 @@ vatpit_data_read(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_ATPIT);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_atpit_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_atpit_v1));
 
 	struct vatpit *vatpit = datap;
 	struct vdi_atpit_v1 *out = req->vdr_data;
@@ -556,7 +556,7 @@ vatpit_data_write(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_ATPIT);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_atpit_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_atpit_v1));
 
 	struct vatpit *vatpit = datap;
 	const struct vdi_atpit_v1 *src = req->vdr_data;
diff --git a/usr/src/uts/intel/io/vmm/io/vhpet.c b/usr/src/uts/intel/io/vmm/io/vhpet.c
index 979b3aa8fe..d946dbe691 100644
--- a/usr/src/uts/intel/io/vmm/io/vhpet.c
+++ b/usr/src/uts/intel/io/vmm/io/vhpet.c
@@ -744,7 +744,7 @@ vhpet_data_read(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_HPET);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_hpet_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_hpet_v1));
 
 	struct vhpet *vhpet = datap;
 	struct vdi_hpet_v1 *out = req->vdr_data;
@@ -789,7 +789,7 @@ static enum vhpet_validation_error
 vhpet_data_validate(const vmm_data_req_t *req, struct vm *vm)
 {
 	ASSERT(req->vdr_version == 1 &&
-	    req->vdr_len == sizeof (struct vdi_hpet_v1));
+	    req->vdr_len >= sizeof (struct vdi_hpet_v1));
 	const struct vdi_hpet_v1 *src = req->vdr_data;
 
 	/* LegacyReplacement Routing is not supported */
@@ -859,7 +859,7 @@ vhpet_data_write(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_HPET);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_hpet_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_hpet_v1));
 
 	struct vhpet *vhpet = datap;
 
diff --git a/usr/src/uts/intel/io/vmm/io/vioapic.c b/usr/src/uts/intel/io/vmm/io/vioapic.c
index b4cde71a81..00e56ebb0b 100644
--- a/usr/src/uts/intel/io/vmm/io/vioapic.c
+++ b/usr/src/uts/intel/io/vmm/io/vioapic.c
@@ -458,7 +458,7 @@ vioapic_data_read(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_IOAPIC);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_ioapic_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_ioapic_v1));
 
 	struct vioapic *vioapic = datap;
 	struct vdi_ioapic_v1 *out = req->vdr_data;
@@ -480,7 +480,7 @@ vioapic_data_write(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_IOAPIC);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_ioapic_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_ioapic_v1));
 
 	struct vioapic *vioapic = datap;
 	const struct vdi_ioapic_v1 *src = req->vdr_data;
diff --git a/usr/src/uts/intel/io/vmm/io/vlapic.c b/usr/src/uts/intel/io/vmm/io/vlapic.c
index 3127bede2f..e6b5f3be00 100644
--- a/usr/src/uts/intel/io/vmm/io/vlapic.c
+++ b/usr/src/uts/intel/io/vmm/io/vlapic.c
@@ -1713,7 +1713,7 @@ vlapic_data_read(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_LAPIC);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_lapic_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_lapic_v1));
 
 	struct vlapic *vlapic = datap;
 	struct vdi_lapic_v1 *out = req->vdr_data;
@@ -1807,7 +1807,7 @@ static enum vlapic_validation_error
 vlapic_data_validate(const struct vlapic *vlapic, const vmm_data_req_t *req)
 {
 	ASSERT(req->vdr_version == 1 &&
-	    req->vdr_len == sizeof (struct vdi_lapic_v1));
+	    req->vdr_len >= sizeof (struct vdi_lapic_v1));
 	const struct vdi_lapic_v1 *src = req->vdr_data;
 
 	if ((src->vl_esr_pending & ~APIC_VALID_MASK_ESR) != 0 ||
@@ -1865,7 +1865,7 @@ vlapic_data_write(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_LAPIC);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_lapic_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_lapic_v1));
 
 	struct vlapic *vlapic = datap;
 	if (vlapic_data_validate(vlapic, req) != VVE_OK) {
diff --git a/usr/src/uts/intel/io/vmm/io/vpmtmr.c b/usr/src/uts/intel/io/vmm/io/vpmtmr.c
index cb8713c9d0..514f495874 100644
--- a/usr/src/uts/intel/io/vmm/io/vpmtmr.c
+++ b/usr/src/uts/intel/io/vmm/io/vpmtmr.c
@@ -161,7 +161,7 @@ vpmtmr_data_read(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_PM_TIMER);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_pm_timer_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_pm_timer_v1));
 
 	struct vpmtmr *vpmtmr = datap;
 	struct vdi_pm_timer_v1 *out = req->vdr_data;
@@ -177,7 +177,7 @@ vpmtmr_data_write(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_PM_TIMER);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_pm_timer_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_pm_timer_v1));
 
 	struct vpmtmr *vpmtmr = datap;
 	const struct vdi_pm_timer_v1 *src = req->vdr_data;
diff --git a/usr/src/uts/intel/io/vmm/io/vrtc.c b/usr/src/uts/intel/io/vmm/io/vrtc.c
index 906b449ddc..644532f077 100644
--- a/usr/src/uts/intel/io/vmm/io/vrtc.c
+++ b/usr/src/uts/intel/io/vmm/io/vrtc.c
@@ -975,7 +975,7 @@ vrtc_data_read(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_RTC);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_rtc_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_rtc_v1));
 
 	struct vrtc *vrtc = datap;
 	struct vdi_rtc_v1 *out = req->vdr_data;
@@ -999,7 +999,7 @@ vrtc_data_write(void *datap, const vmm_data_req_t *req)
 {
 	VERIFY3U(req->vdr_class, ==, VDC_RTC);
 	VERIFY3U(req->vdr_version, ==, 1);
-	VERIFY3U(req->vdr_len, ==, sizeof (struct vdi_rtc_v1));
+	VERIFY3U(req->vdr_len, >=, sizeof (struct vdi_rtc_v1));
 
 	struct vrtc *vrtc = datap;
 	const struct vdi_rtc_v1 *src = req->vdr_data;