Skip to content

Commit

Permalink
bhyve/nvme: Fix Infinite loop in queue processing
Browse files Browse the repository at this point in the history
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
Sponsored by:	Alpha-Omega Project
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D46064

(cherry picked from commit 5374b9e)
(cherry picked from commit 86ba594)
  • Loading branch information
Chuck Tuffli authored and emaste committed Oct 19, 2024
1 parent 9ecbda8 commit df1a36f
Showing 1 changed file with 73 additions and 8 deletions.
81 changes: 73 additions & 8 deletions usr.sbin/bhyve/pci_nvme.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -2841,6 +2852,38 @@ pci_nvme_handle_io_cmd(struct pci_nvme_softc* sc, uint16_t idx)
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)
Expand All @@ -2853,29 +2896,51 @@ 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 {
if (idx > sc->num_cqueues) {
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;
}

Expand Down

0 comments on commit df1a36f

Please sign in to comment.