git: adb7b541aea1 - releng/13.3 - bhyve/nvme: Fix Infinite loop in queue processing

From: Ed Maste <emaste_at_FreeBSD.org>
Date: Tue, 29 Oct 2024 18:53:41 UTC
The branch releng/13.3 has been updated by emaste:

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

commit adb7b541aea13de4a53096966354b1c14a196bba
Author:     Chuck Tuffli <chuck@FreeBSD.org>
AuthorDate: 2024-10-13 13:58:48 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2024-10-29 18:52:54 +0000

    bhyve/nvme: Fix Infinite loop in queue processing
    
    In the functions pci_nvme_handle_admin_cmd and pci_nvme_handle_io_cmd
    infinite loops are possible in the bhyve process if the sq->tail value
    is greater than sq->size.
    
    An attacker could overload the host CPU.
    
    Fix is to validate that doorbell values:
     - Are for a valid (i.e., created) queue
     - Are not the same as the previous value
     - Fit within the available capacity
    
    The emulation will generate an Asynchronous Event Notification (Invalid
    Doorbell or Invalid Doorbell Value) if enabled and ignore the doorbell
    update.
    
    While in the neighborhood, remove a redundant bounds check.
    
    Reported by:    Synacktiv
    MFC after:      1 week
    Security:       HYP-14
    Security:       FreeBSD-SA-24:17.bhyve
    Approved by:    so
    Sponsored by:   The Alpha-Omega Project
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D46064
    
    (cherry picked from commit 5374b9e146811757540e35553a7712c5b9b29239)
    (cherry picked from commit 86ba5941b132c73476a2a1b76ae53902a027b81c)
    (cherry picked from commit df1a36fdfae603ce298b8396ae3388d337c3c5b3)
---
 usr.sbin/bhyve/pci_nvme.c | 81 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 73 insertions(+), 8 deletions(-)

diff --git a/usr.sbin/bhyve/pci_nvme.c b/usr.sbin/bhyve/pci_nvme.c
index cbe4d87b6f60..536f34ddb3ee 100644
--- a/usr.sbin/bhyve/pci_nvme.c
+++ b/usr.sbin/bhyve/pci_nvme.c
@@ -267,6 +267,17 @@ struct pci_nvme_aer {
 	uint16_t	cid;	/* Command ID of the submitted AER */
 };
 
+/** Asynchronous Event Information - Error */
+typedef enum {
+	PCI_NVME_AEI_ERROR_INVALID_DB,
+	PCI_NVME_AEI_ERROR_INVALID_DB_VALUE,
+	PCI_NVME_AEI_ERROR_DIAG_FAILURE,
+	PCI_NVME_AEI_ERROR_PERSISTANT_ERR,
+	PCI_NVME_AEI_ERROR_TRANSIENT_ERR,
+	PCI_NVME_AEI_ERROR_FIRMWARE_LOAD_ERR,
+	PCI_NVME_AEI_ERROR_MAX,
+} pci_nvme_async_event_info_error;
+
 /** Asynchronous Event Information - Notice */
 typedef enum {
 	PCI_NVME_AEI_NOTICE_NS_ATTR_CHANGED = 0,
@@ -2841,6 +2852,38 @@ complete:
 	pthread_mutex_unlock(&sq->mtx);
 }
 
+/*
+ * Check for invalid doorbell write values
+ * See NVM Express Base Specification, revision 2.0
+ * "Asynchronous Event Information - Error Status" for details
+ */
+static bool
+pci_nvme_sq_doorbell_valid(struct nvme_submission_queue *sq, uint64_t value)
+{
+	uint64_t	capacity;
+
+	/*
+	 * Queue empty : head == tail
+	 * Queue full  : head is one more than tail accounting for wrap
+	 * Therefore, can never have more than (size - 1) entries
+	 */
+	if (sq->head == sq->tail)
+		capacity = sq->size - 1;
+	else if (sq->head > sq->tail)
+		capacity = sq->size - (sq->head - sq->tail) - 1;
+	else
+		capacity = sq->tail - sq->head - 1;
+
+	if ((value == sq->tail) ||	/* same as previous */
+	    (value > capacity))	{	/* exceeds queue capacity */
+		EPRINTLN("%s: SQ size=%u head=%u tail=%u capacity=%lu value=%lu",
+		    __func__, sq->size, sq->head, sq->tail, capacity, value);
+		return false;
+	}
+
+	return true;
+}
+
 static void
 pci_nvme_handle_doorbell(struct pci_nvme_softc* sc,
 	uint64_t idx, int is_sq, uint64_t value)
@@ -2853,22 +2896,34 @@ pci_nvme_handle_doorbell(struct pci_nvme_softc* sc,
 			WPRINTF("%s queue index %lu overflow from "
 			         "guest (max %u)",
 			         __func__, idx, sc->num_squeues);
+			pci_nvme_aen_post(sc, PCI_NVME_AE_TYPE_ERROR,
+			    PCI_NVME_AEI_ERROR_INVALID_DB);
+			return;
+		}
+
+		if (sc->submit_queues[idx].qbase == NULL) {
+			WPRINTF("%s write to SQ %lu before created", __func__,
+			    idx);
+			pci_nvme_aen_post(sc, PCI_NVME_AE_TYPE_ERROR,
+			    PCI_NVME_AEI_ERROR_INVALID_DB);
+			return;
+		}
+
+		if (!pci_nvme_sq_doorbell_valid(&sc->submit_queues[idx], value)) {
+			EPRINTLN("%s write to SQ %lu of %lu invalid", __func__,
+			    idx, value);
+			pci_nvme_aen_post(sc, PCI_NVME_AE_TYPE_ERROR,
+			    PCI_NVME_AEI_ERROR_INVALID_DB_VALUE);
 			return;
 		}
 
 		atomic_store_short(&sc->submit_queues[idx].tail,
 		                   (uint16_t)value);
 
-		if (idx == 0) {
+		if (idx == 0)
 			pci_nvme_handle_admin_cmd(sc, value);
-		} else {
+		else {
 			/* submission queue; handle new entries in SQ */
-			if (idx > sc->num_squeues) {
-				WPRINTF("%s SQ index %lu overflow from "
-				         "guest (max %u)",
-				         __func__, idx, sc->num_squeues);
-				return;
-			}
 			pci_nvme_handle_io_cmd(sc, (uint16_t)idx);
 		}
 	} else {
@@ -2876,6 +2931,16 @@ pci_nvme_handle_doorbell(struct pci_nvme_softc* sc,
 			WPRINTF("%s queue index %lu overflow from "
 			         "guest (max %u)",
 			         __func__, idx, sc->num_cqueues);
+			pci_nvme_aen_post(sc, PCI_NVME_AE_TYPE_ERROR,
+			    PCI_NVME_AEI_ERROR_INVALID_DB);
+			return;
+		}
+
+		if (sc->compl_queues[idx].qbase == NULL) {
+			WPRINTF("%s write to CQ %lu before created", __func__,
+			    idx);
+			pci_nvme_aen_post(sc, PCI_NVME_AE_TYPE_ERROR,
+			    PCI_NVME_AEI_ERROR_INVALID_DB);
 			return;
 		}