From 39040a9ec437810b350fe349adc0480ce9a94311 Mon Sep 17 00:00:00 2001
From: Gordon Tetlow <gordon@FreeBSD.org>
Date: Tue, 4 Dec 2018 18:28:25 +0000
Subject: [PATCH] Always treat firmware request and response sizes as unsigned.

This fixes an incomplete bounds check on the guest-supplied request
size where a very large request size could be interpreted as a negative
value and not be caught by the bounds check.

Submitted by:	jhb
Reported by:	Reno Robert
Approved by:	so
Security:	FreeBSD-SA-18:14.bhyve
Security:	CVE-2018-17160
---
 usr.sbin/bhyve/fwctl.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/usr.sbin/bhyve/fwctl.c b/usr.sbin/bhyve/fwctl.c
index 00d6ef86813b..0640bc28ba2b 100644
--- a/usr.sbin/bhyve/fwctl.c
+++ b/usr.sbin/bhyve/fwctl.c
@@ -79,8 +79,8 @@ static u_int ident_idx;
 
 struct op_info {
 	int op;
-	int  (*op_start)(int len);
-	void (*op_data)(uint32_t data, int len);
+	int  (*op_start)(uint32_t len);
+	void (*op_data)(uint32_t data, uint32_t len);
 	int  (*op_result)(struct iovec **data);
 	void (*op_done)(struct iovec *data);
 };
@@ -119,7 +119,7 @@ errop_set(int err)
 }
 
 static int
-errop_start(int len)
+errop_start(uint32_t len)
 {
 	errop_code = ENOENT;
 
@@ -128,7 +128,7 @@ errop_start(int len)
 }
 
 static void
-errop_data(uint32_t data, int len)
+errop_data(uint32_t data, uint32_t len)
 {
 
 	/* ignore */
@@ -188,7 +188,7 @@ static int fget_cnt;
 static size_t fget_size;
 
 static int
-fget_start(int len)
+fget_start(uint32_t len)
 {
 
 	if (len > FGET_STRSZ)
@@ -200,7 +200,7 @@ fget_start(int len)
 }
 
 static void
-fget_data(uint32_t data, int len)
+fget_data(uint32_t data, uint32_t len)
 {
 
 	*((uint32_t *) &fget_str[fget_cnt]) = data;
@@ -285,8 +285,8 @@ static struct req_info {
 	struct op_info *req_op;
 	int	 resp_error;
 	int	 resp_count;
-	int	 resp_size;
-	int	 resp_off;
+	size_t	 resp_size;
+	size_t	 resp_off;
 	struct iovec *resp_biov;
 } rinfo;
 
@@ -346,13 +346,14 @@ fwctl_request_start(void)
 static int
 fwctl_request_data(uint32_t value)
 {
-	int remlen;
 
 	/* Make sure remaining size is >= 0 */
-	rinfo.req_size -= sizeof(uint32_t);
-	remlen = MAX(rinfo.req_size, 0);
+	if (rinfo.req_size <= sizeof(uint32_t))
+		rinfo.req_size = 0;
+	else
+		rinfo.req_size -= sizeof(uint32_t);
 
-	(*rinfo.req_op->op_data)(value, remlen);
+	(*rinfo.req_op->op_data)(value, rinfo.req_size);
 
 	if (rinfo.req_size < sizeof(uint32_t)) {
 		fwctl_request_done();
@@ -401,7 +402,7 @@ static int
 fwctl_response(uint32_t *retval)
 {
 	uint32_t *dp;
-	int remlen;
+	ssize_t remlen;
 
 	switch(rinfo.resp_count) {
 	case 0:
@@ -436,7 +437,7 @@ fwctl_response(uint32_t *retval)
 	}
 
 	if (rinfo.resp_count > 3 &&
-	    rinfo.resp_size - rinfo.resp_off <= 0) {
+	    rinfo.resp_off >= rinfo.resp_size) {
 		fwctl_response_done();
 		return (1);
 	}