git: 87702e38a4b4 - releng/13.1 - bhyve: Fully reset the fwctl state machine if the guest requests a reset.

The branch releng/13.1 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=87702e38a4b4fe990b06f39b5cf267fb564c367e

commit 87702e38a4b4fe990b06f39b5cf267fb564c367e
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-06-29 18:27:12 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-08-01 19:48:26 +0000

    bhyve: Fully reset the fwctl state machine if the guest requests a reset.
    
    If a guest tries to reset the fwctl device while a pending request was
    in flight, the fwctl state machine can be left in an incomplete state.
    Specifically, rinfo is not cleared.
    
    Normally the state machine for fwctl alternates between REQ (receiving
    request) and RESP (sending response) and ignores port writes while in
    RESP or port reads while in REQ.  Once a guest completes the writes to
    the port to send a request, the state machine transitions to RESP and
    ignores future writes.
    
    However, if a guest writes a full request and then resets the fwctl
    device, the state would transition to REQ without draining the pending
    response or discarding the received request.  Instead, additional
    port writes after the reset were treated as new payload bytes, but
    were appended to the previously-received request and could overflow
    the fget_str buffer.
    
    To fix, fully reset the fwctl state machine if the guest requests a
    reset.
    
    admbugs:        998
    Approved by:    so
    Reviewed by:    markj
    Reported by:    Omri Ben Bassat <t-benbassato@microsoft.com>
    Security:       FreeBSD-SA-23:07.bhyve
    Security:       CVE-2023-3494
    
    (cherry picked from commit bed3ae1d7863ac1e0b1e82ae7bf952937e921efe)
    (cherry picked from commit 9fe302d78109b12867bd933bb68cd900c9940b7d)
---
 usr.sbin/bhyve/fwctl.c | 40 +++++++++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/usr.sbin/bhyve/fwctl.c b/usr.sbin/bhyve/fwctl.c
index 7c27d7e92532..d1ceed0e2daf 100644
--- a/usr.sbin/bhyve/fwctl.c
+++ b/usr.sbin/bhyve/fwctl.c
@@ -66,13 +66,12 @@ __FBSDID("$FreeBSD$");
 /*
  * Back-end state-machine
  */
-enum state {
-	DORMANT,
+static enum state {
 	IDENT_WAIT,
 	IDENT_SEND,
 	REQ,
 	RESP
-} be_state = DORMANT;
+} be_state;
 
 static uint8_t sig[] = { 'B', 'H', 'Y', 'V' };
 static u_int ident_idx;
@@ -203,7 +202,8 @@ static void
 fget_data(uint32_t data, uint32_t len)
 {
 
-	*((uint32_t *) &fget_str[fget_cnt]) = data;
+	assert(fget_cnt + sizeof(uint32_t) <= sizeof(fget_str));
+	memcpy(&fget_str[fget_cnt], &data, sizeof(data));
 	fget_cnt += sizeof(uint32_t);
 }
 
@@ -347,7 +347,8 @@ static int
 fwctl_request_data(uint32_t value)
 {
 
-	/* Make sure remaining size is >= 0 */
+	/* Make sure remaining size is > 0 */
+	assert(rinfo.req_size > 0);
 	if (rinfo.req_size <= sizeof(uint32_t))
 		rinfo.req_size = 0;
 	else
@@ -445,6 +446,28 @@ fwctl_response(uint32_t *retval)
 	return (0);
 }
 
+static void
+fwctl_reset(void)
+{
+
+	switch (be_state) {
+	case RESP:
+		/* If a response was generated but not fully read, discard it. */
+		fwctl_response_done();
+		break;
+	case REQ:
+		/* Discard partially-received request. */
+		memset(&rinfo, 0, sizeof(rinfo));
+		break;
+	case IDENT_WAIT:
+	case IDENT_SEND:
+		break;
+	}
+
+	be_state = IDENT_SEND;
+	ident_idx = 0;
+}
+
 
 /*
  * i/o port handling.
@@ -472,18 +495,13 @@ fwctl_inb(void)
 static void
 fwctl_outw(uint16_t val)
 {
-	if (be_state == DORMANT) {
-		return;
-	}
-
 	if (val == 0) {
 		/*
 		 * The guest wants to read the signature. It's possible that the
 		 * guest is unaware of the fwctl state at this moment. For that
 		 * reason, reset the state machine unconditionally.
 		 */
-		be_state = IDENT_SEND;
-		ident_idx = 0;
+		fwctl_reset();
 	}
 }