Re: [PATCH 0/8] ublk: don't dispatch to canceled io commands

[Josef Bacik <josef@toxicpanda.com>]

On Thu, Oct 01, 2026 at 07:54:14AM -0500, Ming Lei wrote:
> Hi,
>
> Josef reported three ways in which ublk dispatches a request to a
> canceled io command, which oopses on the NULL io->cmd.

Thanks Ming. I ran 1-8 on top of for-next (d70609a2f68c) under QEMU with
KASAN and lockdep: my reproducers for the three routes plus the restart,
recovery and QUIESCE_DEV controls, the ublk selftests including your
generic_18, and my ADD vs STOP_DEV/DEL_DEV race script. No oops or
lockdep report anywhere, and generic_18 passes. A few things though.

1. kublk's batch daemon dies when PREP_IO_CMDS hits the new STOPPING
   check. With patch 6, __ublk_fetch() returns UBLK_IO_RES_ABORT while
   UB_STATE_STOPPING is set, and batch PREP goes through it, so a PREP
   racing STOP_DEV completes with -ENODEV. kublk asserts cqe->res == 0
   in ublk_batch_compl_commit_cmd(), the daemon aborts before it writes
   the start eventfd, and the parent of `kublk add` sits in
   eventfd_read() forever. My race script hangs on the first or second
   iteration with -b; the per-io servers handle the abort fine. The
   kernel side is fine, but kublk needs to fail the queue cleanly there,
   and generic_18 has no batch mode, which is why it doesn't see it.

2. Routes 2 and 3 still bring the device up. START_DEV returns 0 and
   every request on the new disk fails, and after END_USER_RECOVERY the
   device is LIVE with the reads on the dead task's queue parked forever
   with plain USER_RECOVERY (generic_18 accepts both, and turns off the
   partition scan, which is what parks under disk->open_mutex otherwise).
   I'll send a follow-up on top of your series that returns -ENODEV from
   START_DEV and END_USER_RECOVERY while ub->canceling is set; with it
   generic_18 still passes.

3. The same follow-up has ublk_start_cancel() read ub->ub_disk under
   cancel_mutex, with START_DEV publishing it in the same section. Today
   it samples the disk before taking the mutex, so a server dying during
   its own START_DEV can mark the queues without quiescing a disk that
   START_DEV published in between, with first I/O already past the
   canceling check. Narrow, and older than this series.

4. Patch 5: the release work decides there is no disk before it takes
   ub->mutex and doesn't look again, so a START_DEV that slips in with
   the exiting server's pid can go LIVE first and the work then resets
   a LIVE device. It needs START_DEV pending while the node is released,
   e.g. a separate control process, and nothing oopses (every queue is
   still canceling), but the device ends up LIVE with requests failing
   or parked until STOP_DEV/DEL_DEV.

5. Patch 6 changes STOP_DEV's contract: a server that still holds
   /dev/ublkcN after STOP_DEV gets ABORT on FETCH/PREP and -EBUSY on
   START_DEV until it closes the node, even if it had fetched nothing.
   That's fine by me, but it should go in Documentation/block/ublk.rst,
   since servers that STOP_DEV and restart on the same fd will see it.

For 1-6, with kublk fixed for 1:

Tested-by: Josef Bacik <josef@toxicpanda.com>

Thanks,
Josef