Skip to content

Commit

Permalink
nvmet: pci-epf: Set NVMET_PCI_EPF_Q_LIVE when a queue is fully created
Browse files Browse the repository at this point in the history
The function nvmet_pci_epf_create_sq() use test_and_set_bit() to check
that a submission queue is not already live and if not, set the
NVMET_PCI_EPF_Q_LIVE queue flag to declare the sq live (ready to use).
However, this is done on entry to the function, before the submission
queue is actually fully initialized and ready to use. This creates a
race situation with the function nvmet_pci_epf_poll_sqs_work() which
looks at the NVMET_PCI_EPF_Q_LIVE queue flag to poll the submission
queue when it is live. This race can lead to invalid DMA transfers if
nvmet_pci_epf_poll_sqs_work() runs after the NVMET_PCI_EPF_Q_LIVE flag
is set but before setting the sq pci address and doorbell ofset.

Avoid this race by only testing the NVMET_PCI_EPF_Q_LIVE flag on entry
to nvmet_pci_epf_create_sq() and setting it after the submission queue
is fully setup before nvmet_pci_epf_create_sq() returns success.
Since the function nvmet_pci_epf_create_cq() also has the same racy flag
setting pattern, also make a similar change in that function.

Fixes: 0faa0fe ("nvmet: New NVMe PCI endpoint function target driver")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
  • Loading branch information
Damien Le Moal authored and Keith Busch committed Mar 10, 2025
1 parent 3f674e7 commit bf9b802
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions drivers/nvme/target/pci-epf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,7 @@ static u16 nvmet_pci_epf_create_cq(struct nvmet_ctrl *tctrl,
struct nvmet_pci_epf_queue *cq = &ctrl->cq[cqid];
u16 status;

if (test_and_set_bit(NVMET_PCI_EPF_Q_LIVE, &cq->flags))
if (test_bit(NVMET_PCI_EPF_Q_LIVE, &cq->flags))
return NVME_SC_QID_INVALID | NVME_STATUS_DNR;

if (!(flags & NVME_QUEUE_PHYS_CONTIG))
Expand Down Expand Up @@ -1300,14 +1300,15 @@ static u16 nvmet_pci_epf_create_cq(struct nvmet_ctrl *tctrl,
if (status != NVME_SC_SUCCESS)
goto err;

set_bit(NVMET_PCI_EPF_Q_LIVE, &cq->flags);

dev_dbg(ctrl->dev, "CQ[%u]: %u entries of %zu B, IRQ vector %u\n",
cqid, qsize, cq->qes, cq->vector);

return NVME_SC_SUCCESS;

err:
clear_bit(NVMET_PCI_EPF_Q_IRQ_ENABLED, &cq->flags);
clear_bit(NVMET_PCI_EPF_Q_LIVE, &cq->flags);
return status;
}

Expand All @@ -1333,7 +1334,7 @@ static u16 nvmet_pci_epf_create_sq(struct nvmet_ctrl *tctrl,
struct nvmet_pci_epf_queue *sq = &ctrl->sq[sqid];
u16 status;

if (test_and_set_bit(NVMET_PCI_EPF_Q_LIVE, &sq->flags))
if (test_bit(NVMET_PCI_EPF_Q_LIVE, &sq->flags))
return NVME_SC_QID_INVALID | NVME_STATUS_DNR;

if (!(flags & NVME_QUEUE_PHYS_CONTIG))
Expand All @@ -1355,7 +1356,7 @@ static u16 nvmet_pci_epf_create_sq(struct nvmet_ctrl *tctrl,

status = nvmet_sq_create(tctrl, &sq->nvme_sq, sqid, sq->depth);
if (status != NVME_SC_SUCCESS)
goto out_clear_bit;
return status;

sq->iod_wq = alloc_workqueue("sq%d_wq", WQ_UNBOUND,
min_t(int, sq->depth, WQ_MAX_ACTIVE), sqid);
Expand All @@ -1365,15 +1366,15 @@ static u16 nvmet_pci_epf_create_sq(struct nvmet_ctrl *tctrl,
goto out_destroy_sq;
}

set_bit(NVMET_PCI_EPF_Q_LIVE, &sq->flags);

dev_dbg(ctrl->dev, "SQ[%u]: %u entries of %zu B\n",
sqid, qsize, sq->qes);

return NVME_SC_SUCCESS;

out_destroy_sq:
nvmet_sq_destroy(&sq->nvme_sq);
out_clear_bit:
clear_bit(NVMET_PCI_EPF_Q_LIVE, &sq->flags);
return status;
}

Expand Down

0 comments on commit bf9b802

Please sign in to comment.