UBLK_U_IO_FETCH_IO_CMDS has the same order problem as the per-io commands: ublk_batch_attach() links the fetch command into fcmd_head and marks it cancelable only after dropping evts_lock. A cancel from the control path (ublk_batch_cancel_queue()) can take it in between and complete it, and the later mark puts a completed request on io_uring's cancelable list. Mark it before linking it. evts_lock orders the mark before the link, which is where the cancel finds it. Batch commands are not bounced to task work, so they can run from io-wq without uring_lock, and io_uring's cancel walk can now find a fetch command before it is linked. Two things make that safe: - initialize fcmd->node: it came from kzalloc(), so list_empty() saw a linked node and ublk_batch_cancel_cmd() would list_del_init() NULL pointers - on the -ENODEV path, complete the command with io_uring_cmd_done() before freeing fcmd: that removes it from the cancelable list under uring_lock, after which nothing can see fcmd Also use data->cmd instead of fcmd->cmd after dropping evts_lock: once fcmd is linked and not the active one, a control-path cancel may complete and free it. Fixes: a4d883755399 ("ublk: add UBLK_U_IO_FETCH_IO_CMDS for batch I/O processing") Cc: stable@vger.kernel.org # v7.0+ Tested-by: Josef Bacik Signed-off-by: Ming Lei --- drivers/block/ublk_drv.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index 054760f73c54..e57c0626b84f 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -815,6 +815,8 @@ ublk_batch_alloc_fcmd(struct io_uring_cmd *cmd) if (fcmd) { fcmd->cmd = cmd; fcmd->buf_group = READ_ONCE(cmd->sqe->buf_index); + /* a cancel may look at it before it is linked */ + INIT_LIST_HEAD(&fcmd->node); } return fcmd; } @@ -3914,6 +3916,15 @@ static int ublk_batch_attach(struct ublk_queue *ubq, bool free = false; struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(data->cmd); + /* + * Mark it cancelable before linking it into fcmd_head, where a cancel + * from the control path can take and complete it: see + * ublk_prep_cancel(). evts_lock orders the mark before the link. + */ + pdu->ubq = ubq; + pdu->fcmd = fcmd; + io_uring_cmd_mark_cancelable(fcmd->cmd, data->issue_flags); + spin_lock(&ubq->evts_lock); if (unlikely(ubq->force_abort || ubq->canceling)) { free = true; @@ -3924,14 +3935,12 @@ static int ublk_batch_attach(struct ublk_queue *ubq, spin_unlock(&ubq->evts_lock); if (unlikely(free)) { + /* off the cancelable list first, then nothing can see fcmd */ + io_uring_cmd_done(data->cmd, -ENODEV, data->issue_flags); ublk_batch_free_fcmd(fcmd); - return -ENODEV; + return -EIOCBQUEUED; } - pdu->ubq = ubq; - pdu->fcmd = fcmd; - io_uring_cmd_mark_cancelable(fcmd->cmd, data->issue_flags); - if (!new_fcmd) goto out; @@ -3939,9 +3948,12 @@ static int ublk_batch_attach(struct ublk_queue *ubq, * If the two fetch commands are originated from same io_ring_ctx, * run batch dispatch directly. Otherwise, schedule task work for * doing it. + * + * Use data->cmd, not fcmd->cmd: once fcmd is linked and not active, + * a cancel from the control path may complete and free it. */ if (io_uring_cmd_ctx_handle(new_fcmd->cmd) == - io_uring_cmd_ctx_handle(fcmd->cmd)) { + io_uring_cmd_ctx_handle(data->cmd)) { data->cmd = new_fcmd->cmd; ublk_batch_dispatch(ubq, data, new_fcmd); } else { -- 2.55.0