From nobody Sun Oct 13 14:00:21 2024 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4XRMSj6mvMz5Z0Pw; Sun, 13 Oct 2024 14:00:21 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4XRMSj6HTGz52gm; Sun, 13 Oct 2024 14:00:21 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1728828021; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=f1nN+xqr0GtKV7wntx/eSIgGIEe0k4ArYM3qGlZa/Mc=; b=yG6KR6y+gqu7LsY0CccVXITSu3WmQ+MbK5hDCA+lYSUUpJ6R3311zpXI1ojpg+MXaH7yQ9 rQTguZ6uTDr12kjYwJyStl9xIq9eo+z5nbxHh3+K04WxbwXCzOJ57ddmo/977/A3e1gPLj 0Cs/f15jqSZMOmyI3ZUmWedn1PajWFXniCHRpGesuypoCRYKkf08K5uMwncHtKZEwsC1qj Wpiz3/62JmSxUse3H3b5ShjuAghHlpIpgO/asAPXf5OLvyBrtCfad13VBQvdBmduPneYME H+zRPT0S9+omkLnKXUr+UMi+4n7Sr+x7sn0cYQUZkMUUu38deDf6GGTQ3TXgTQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1728828021; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=f1nN+xqr0GtKV7wntx/eSIgGIEe0k4ArYM3qGlZa/Mc=; b=TmrtHWkcPO7i8oy5x46+Gml6X/5o2LBf2AhRnk1o0wCrR6rfb+Ha73Yk/PKDO1tm1GYYj8 0oRrTsfqkuJuFPb9YnoFZ7fkbvIvqtVx2fQ0G1imAxk3qc8MqLPu+ejpRPDAbEm15+0iw9 VRdjGrKzLJuVErBa+BopSJJTxpLZBI5JVn2IMk5xxVhpgxbFQ8YrKHgDJ4gNiVxjdIEm3s Hu7egRjejQjgvZwfdVJC45M2AwS2wpUbXAfSsUR9KobKurgeV5PsH+bJdKrFuvsJK2Npp5 XmFb6KTdCE7zi7CvTzpvJiK2CVzObYq0ZyvWYgI4shdNnFU4z/BjcvON3gz7qg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1728828021; a=rsa-sha256; cv=none; b=cP3x7hD9ozhpZHGZStjxHKcAvfnLVzsfzXxeIDQt5tA8khKIfIifKYJE4LEuvzjls2w2nJ CSWrMfcjW1XDAIx2ud/NVWvLpE2KLDwAGZUrZATgou55DOhgpxLEX0Czj1Zxx0isCgMhff UxosBpz2xmnFMb/2BZyn/2PJ6qFig80WXf9GuaQFLBUfkg3msnG2Ndzo9ojRIdckLig4gk xsnqumy9YKqSCF+j+qnYb+nnxauzSmzAVl6L0TVIBctfo59tdfkAbGIOqnAjYKm96hG6GC S0mUacDmrBGNrp/zwh2aVFbDV0+uNg37jfzm+jWRjjBQXchOf2tvrJfr579SSQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4XRMSj5t7SzxFj; Sun, 13 Oct 2024 14:00:21 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 49DE0Lsd073352; Sun, 13 Oct 2024 14:00:21 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 49DE0LGA073349; Sun, 13 Oct 2024 14:00:21 GMT (envelope-from git) Date: Sun, 13 Oct 2024 14:00:21 GMT Message-Id: <202410131400.49DE0LGA073349@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Chuck Tuffli Subject: git: 5374b9e14681 - main - bhyve/nvme: Fix Infinite loop in queue processing List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: chuck X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 5374b9e146811757540e35553a7712c5b9b29239 Auto-Submitted: auto-generated The branch main has been updated by chuck: URL: https://cgit.FreeBSD.org/src/commit/?id=5374b9e146811757540e35553a7712c5b9b29239 commit 5374b9e146811757540e35553a7712c5b9b29239 Author: Chuck Tuffli AuthorDate: 2024-10-13 13:58:48 +0000 Commit: Chuck Tuffli CommitDate: 2024-10-13 13:58:50 +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 Sponsored by: Alpha-Omega Project Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D46064 --- 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 b8eb24d91b49..7e6c8bc9d719 100644 --- a/usr.sbin/bhyve/pci_nvme.c +++ b/usr.sbin/bhyve/pci_nvme.c @@ -265,6 +265,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, @@ -2784,6 +2795,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) @@ -2796,22 +2839,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 { @@ -2819,6 +2874,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; }