* [PULL 1/2] hw/nvme: fix oob memory read in fdp events log
2023-08-07 11:54 [PULL 0/2] hw/nvme fixes Klaus Jensen
@ 2023-08-07 11:54 ` Klaus Jensen
2023-08-07 11:54 ` [PULL 2/2] hw/nvme: fix compliance issue wrt. iosqes/iocqes Klaus Jensen
2023-08-07 20:36 ` [PULL 0/2] hw/nvme fixes Richard Henderson
2 siblings, 0 replies; 4+ messages in thread
From: Klaus Jensen @ 2023-08-07 11:54 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Keith Busch, Hanna Reitz, qemu-block,
Klaus Jensen, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Klaus Jensen
From: Klaus Jensen <k.jensen@samsung.com>
As reported by Trend Micro's Zero Day Initiative, an oob memory read
vulnerability exists in nvme_fdp_events(). The host-provided offset is
not verified.
Fix this.
This is only exploitable when Flexible Data Placement mode (fdp=on) is
enabled.
Fixes: CVE-2023-4135
Fixes: 73064edfb864 ("hw/nvme: flexible data placement emulation")
Reported-by: Trend Micro's Zero Day Initiative
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/nvme/ctrl.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index f2e5a2fa737b..e9b5a55811b8 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -5120,6 +5120,11 @@ static uint16_t nvme_fdp_events(NvmeCtrl *n, uint32_t endgrpid,
}
log_size = sizeof(NvmeFdpEventsLog) + ebuf->nelems * sizeof(NvmeFdpEvent);
+
+ if (off >= log_size) {
+ return NVME_INVALID_FIELD | NVME_DNR;
+ }
+
trans_len = MIN(log_size - off, buf_len);
elog = g_malloc0(log_size);
elog->num_events = cpu_to_le32(ebuf->nelems);
--
2.41.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PULL 2/2] hw/nvme: fix compliance issue wrt. iosqes/iocqes
2023-08-07 11:54 [PULL 0/2] hw/nvme fixes Klaus Jensen
2023-08-07 11:54 ` [PULL 1/2] hw/nvme: fix oob memory read in fdp events log Klaus Jensen
@ 2023-08-07 11:54 ` Klaus Jensen
2023-08-07 20:36 ` [PULL 0/2] hw/nvme fixes Richard Henderson
2 siblings, 0 replies; 4+ messages in thread
From: Klaus Jensen @ 2023-08-07 11:54 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Keith Busch, Hanna Reitz, qemu-block,
Klaus Jensen, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Klaus Jensen
From: Klaus Jensen <k.jensen@samsung.com>
As of prior to this patch, the controller checks the value of CC.IOCQES
and CC.IOSQES prior to enabling the controller. As reported by Ben in
GitLab issue #1691, this is not spec compliant. The controller should
only check these values when queues are created.
This patch moves these checks to nvme_create_cq(). We do not need to
check it in nvme_create_sq() since that will error out if the completion
queue is not already created.
Also, since the controller exclusively supports SQEs of size 64 bytes
and CQEs of size 16 bytes, hard code that.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1691
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/nvme/ctrl.c | 46 ++++++++++++--------------------------------
hw/nvme/nvme.h | 9 +++++++--
hw/nvme/trace-events | 1 +
3 files changed, 20 insertions(+), 36 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index e9b5a55811b8..d217ae91b506 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1507,7 +1507,7 @@ static void nvme_post_cqes(void *opaque)
req->cqe.status = cpu_to_le16((req->status << 1) | cq->phase);
req->cqe.sq_id = cpu_to_le16(sq->sqid);
req->cqe.sq_head = cpu_to_le16(sq->head);
- addr = cq->dma_addr + cq->tail * n->cqe_size;
+ addr = cq->dma_addr + (cq->tail << NVME_CQES);
ret = pci_dma_write(PCI_DEVICE(n), addr, (void *)&req->cqe,
sizeof(req->cqe));
if (ret) {
@@ -5300,10 +5300,18 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeRequest *req)
uint16_t qsize = le16_to_cpu(c->qsize);
uint16_t qflags = le16_to_cpu(c->cq_flags);
uint64_t prp1 = le64_to_cpu(c->prp1);
+ uint32_t cc = ldq_le_p(&n->bar.cc);
+ uint8_t iocqes = NVME_CC_IOCQES(cc);
+ uint8_t iosqes = NVME_CC_IOSQES(cc);
trace_pci_nvme_create_cq(prp1, cqid, vector, qsize, qflags,
NVME_CQ_FLAGS_IEN(qflags) != 0);
+ if (iosqes != NVME_SQES || iocqes != NVME_CQES) {
+ trace_pci_nvme_err_invalid_create_cq_entry_size(iosqes, iocqes);
+ return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
+ }
+
if (unlikely(!cqid || cqid > n->conf_ioqpairs || n->cq[cqid] != NULL)) {
trace_pci_nvme_err_invalid_create_cq_cqid(cqid);
return NVME_INVALID_QID | NVME_DNR;
@@ -7000,7 +7008,7 @@ static void nvme_process_sq(void *opaque)
}
while (!(nvme_sq_empty(sq) || QTAILQ_EMPTY(&sq->req_list))) {
- addr = sq->dma_addr + sq->head * n->sqe_size;
+ addr = sq->dma_addr + (sq->head << NVME_SQES);
if (nvme_addr_read(n, addr, (void *)&cmd, sizeof(cmd))) {
trace_pci_nvme_err_addr_read(addr);
trace_pci_nvme_err_cfs();
@@ -7225,34 +7233,6 @@ static int nvme_start_ctrl(NvmeCtrl *n)
NVME_CAP_MPSMAX(cap));
return -1;
}
- if (unlikely(NVME_CC_IOCQES(cc) <
- NVME_CTRL_CQES_MIN(n->id_ctrl.cqes))) {
- trace_pci_nvme_err_startfail_cqent_too_small(
- NVME_CC_IOCQES(cc),
- NVME_CTRL_CQES_MIN(cap));
- return -1;
- }
- if (unlikely(NVME_CC_IOCQES(cc) >
- NVME_CTRL_CQES_MAX(n->id_ctrl.cqes))) {
- trace_pci_nvme_err_startfail_cqent_too_large(
- NVME_CC_IOCQES(cc),
- NVME_CTRL_CQES_MAX(cap));
- return -1;
- }
- if (unlikely(NVME_CC_IOSQES(cc) <
- NVME_CTRL_SQES_MIN(n->id_ctrl.sqes))) {
- trace_pci_nvme_err_startfail_sqent_too_small(
- NVME_CC_IOSQES(cc),
- NVME_CTRL_SQES_MIN(cap));
- return -1;
- }
- if (unlikely(NVME_CC_IOSQES(cc) >
- NVME_CTRL_SQES_MAX(n->id_ctrl.sqes))) {
- trace_pci_nvme_err_startfail_sqent_too_large(
- NVME_CC_IOSQES(cc),
- NVME_CTRL_SQES_MAX(cap));
- return -1;
- }
if (unlikely(!NVME_AQA_ASQS(aqa))) {
trace_pci_nvme_err_startfail_asqent_sz_zero();
return -1;
@@ -7265,8 +7245,6 @@ static int nvme_start_ctrl(NvmeCtrl *n)
n->page_bits = page_bits;
n->page_size = page_size;
n->max_prp_ents = n->page_size / sizeof(uint64_t);
- n->cqe_size = 1 << NVME_CC_IOCQES(cc);
- n->sqe_size = 1 << NVME_CC_IOSQES(cc);
nvme_init_cq(&n->admin_cq, n, acq, 0, 0, NVME_AQA_ACQS(aqa) + 1, 1);
nvme_init_sq(&n->admin_sq, n, asq, 0, 0, NVME_AQA_ASQS(aqa) + 1);
@@ -8235,8 +8213,8 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
id->cctemp = cpu_to_le16(NVME_TEMPERATURE_CRITICAL);
- id->sqes = (0x6 << 4) | 0x6;
- id->cqes = (0x4 << 4) | 0x4;
+ id->sqes = (NVME_SQES << 4) | NVME_SQES;
+ id->cqes = (NVME_CQES << 4) | NVME_CQES;
id->nn = cpu_to_le32(NVME_MAX_NAMESPACES);
id->oncs = cpu_to_le16(NVME_ONCS_WRITE_ZEROES | NVME_ONCS_TIMESTAMP |
NVME_ONCS_FEATURES | NVME_ONCS_DSM |
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 209e8f5b4c08..5f2ae7b28b9c 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -30,6 +30,13 @@
#define NVME_FDP_MAX_EVENTS 63
#define NVME_FDP_MAXPIDS 128
+/*
+ * The controller only supports Submission and Completion Queue Entry Sizes of
+ * 64 and 16 bytes respectively.
+ */
+#define NVME_SQES 6
+#define NVME_CQES 4
+
QEMU_BUILD_BUG_ON(NVME_MAX_NAMESPACES > NVME_NSID_BROADCAST - 1);
typedef struct NvmeCtrl NvmeCtrl;
@@ -530,8 +537,6 @@ typedef struct NvmeCtrl {
uint32_t page_size;
uint16_t page_bits;
uint16_t max_prp_ents;
- uint16_t cqe_size;
- uint16_t sqe_size;
uint32_t max_q_ents;
uint8_t outstanding_aers;
uint32_t irq_status;
diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
index 9afddf3b951c..3a67680c6ad1 100644
--- a/hw/nvme/trace-events
+++ b/hw/nvme/trace-events
@@ -168,6 +168,7 @@ pci_nvme_err_invalid_create_cq_size(uint16_t size) "failed creating completion q
pci_nvme_err_invalid_create_cq_addr(uint64_t addr) "failed creating completion queue, addr=0x%"PRIx64""
pci_nvme_err_invalid_create_cq_vector(uint16_t vector) "failed creating completion queue, vector=%"PRIu16""
pci_nvme_err_invalid_create_cq_qflags(uint16_t qflags) "failed creating completion queue, qflags=%"PRIu16""
+pci_nvme_err_invalid_create_cq_entry_size(uint8_t iosqes, uint8_t iocqes) "iosqes %"PRIu8" iocqes %"PRIu8""
pci_nvme_err_invalid_identify_cns(uint16_t cns) "identify, invalid cns=0x%"PRIx16""
pci_nvme_err_invalid_getfeat(int dw10) "invalid get features, dw10=0x%"PRIx32""
pci_nvme_err_invalid_setfeat(uint32_t dw10) "invalid set features, dw10=0x%"PRIx32""
--
2.41.0
^ permalink raw reply related [flat|nested] 4+ messages in thread