Re: [PATCH] nvmet: pci-epf: fix use-after-free in nvmet_pci_epf_exec_iod_work()

On 7/13/26 13:32, Shin'ichiro Kawasaki wrote:
> nvmet_pci_epf_exec_iod_work() submits an I/O command with req->execute()
> and then waits for the command to complete and transfers the data back
> to the host. This wait is not needed for commands that do not transfer
> data from the device to the host. To decide whether that wait is needed,
> it reads iod->data_len and iod->dma_dir after calling req->execute().
> 
> However, once req->execute() is called, the command may complete
> asynchronously on another CPU. For commands that do not require a
> device-to-host data transfer, nvmet_pci_epf_queue_response() calls
> nvmet_pci_epf_complete_iod() directly, which can free the iod before it
> reads iod->data_len and iod->dma_dir, resulting in the KFENCE use-after-
> free:
> 
>  BUG: KFENCE: use-after-free read in nvmet_pci_epf_exec_iod_work+0x288/0x798 [nvmet_pci_epf]

Looks good, but I would do it like this to simplify:

diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index 4e9db96ebfec..485ce759391a 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -1594,6 +1594,7 @@ static void nvmet_pci_epf_exec_iod_work(struct
work_struct *work)
        struct nvmet_pci_epf_iod *iod =
                container_of(work, struct nvmet_pci_epf_iod, work);
        struct nvmet_req *req = &iod->req;
+       bool no_wait;
        int ret;

        if (!iod->ctrl->link_up) {
@@ -1638,14 +1639,16 @@ static void nvmet_pci_epf_exec_iod_work(struct
work_struct *work)
                }
        }

-       req->execute(req);
-
        /*
         * If we do not have data to transfer after the command execution
         * finishes, nvmet_pci_epf_queue_response() will complete the command
         * directly. No need to wait for the completion in this case.
         */
-       if (!iod->data_len || iod->dma_dir != DMA_TO_DEVICE)
+       no_wait = !iod->data_len || iod->dma_dir != DMA_TO_DEVICE;
+
+       req->execute(req);
+
+       if (no_wait)
                return;

        wait_for_completion(&iod->done);


-- 
Damien Le Moal
Western Digital Research