Re: [PATCH 1/9] ublk: keep queue canceling over canceled commands

[Josef Bacik]

On Mon, Sep 28, 2026 at 09:46:04AM -0700, Caleb Sander Mateos wrote:
> On Mon, Sep 28, 2026 at 9:02 AM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
> > +       for (i = 0; i < ub->dev_info.nr_hw_queues; i++)
> > +               WRITE_ONCE(ublk_get_queue(ub, i)->force_abort, false);
>
> I think this has already been done in
> https://lore.kernel.org/linux-block/20260821103047.369522-2-yangxiuwei@xxxxxxxxxx/

Not quite. Yang's patch went in as 8a14be55bdc6 ("ublk: clear
force_abort in ublk_queue_reset_io_flags()") and this series is on top
of it, so that clear is still there. The problem is that this patch has
the ready transition of a UBLK_F_BATCH_IO queue look at ->force_abort to
decide whether the queue has to stay canceling, and it reads the flag
before 8a14be55bdc6 clears it. A ->force_abort left over from the old
server's quiesce then keeps the new server's queue canceling and the
recovery fails, so it has to be cleared earlier, when the old server
goes away.

You're right that clearing it in two places makes no sense though. For
v2 I'll move the clear instead of adding one: drop it from
ublk_queue_reset_io_flags(), since the release work covers Yang's case
too (START_USER_RECOVERY can't happen until the old server has
released /dev/ublkcN), and say so in the changelog.

> > -       io->flags &= ~UBLK_IO_FLAG_CANCELED;
> >         ublk_fill_io_cmd(io, cmd);
> > +       spin_unlock(&ubq->cancel_lock);
>
> And this looks like it may also duplicate
> https://lore.kernel.org/linux-block/20260508123746.242018-1-tom.leiming@xxxxxxxxx/

That one is f7700a4415af ("ublk: fix use-after-free in
ublk_cancel_cmd()"), and it orders ublk_cancel_cmd() against the reset
path, ublk_reset_ch_dev() clearing io->cmd. This hunk is the fetch side,
which f7700a4415af left alone: __ublk_fetch() drops the stale
UBLK_IO_FLAG_CANCELED and then publishes io->cmd. A cancel that runs in
between sets CANCELED again, the fetch clears it, and the queue goes
ready over a command that was already completed. Same lock, different
race. I'll mention f7700a4415af in the changelog so that's clearer. Thanks,

Josef