svn commit: r341484 - head/usr.sbin/bhyve

[Gordon Tetlow]

Author: gordon
Date: Tue Dec  4 18:28:25 2018
New Revision: 341484
URL: https://svnweb.freebsd.org/changeset/base/341484

Log:
  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

Modified:
  head/usr.sbin/bhyve/fwctl.c

Modified: head/usr.sbin/bhyve/fwctl.c
==============================================================================
--- head/usr.sbin/bhyve/fwctl.c	Tue Dec  4 17:57:01 2018	(r341483)
+++ head/usr.sbin/bhyve/fwctl.c	Tue Dec  4 18:28:25 2018	(r341484)
@@ -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);
 	}