Re: [PATCH] ublk: configure io buffer before taking uring_cmd

On Fri, Jul 24, 2026 at 3:05 AM Yang Xiuwei <[email protected]> wrote:
>
> With UBLK_F_AUTO_BUF_REG, ublk_config_io_buf() may fail on invalid
> auto buffer register data in sqe->addr. Calling ublk_fill_io_cmd()
> first leaves UBLK_IO_FLAG_ACTIVE set while io_uring has already
> completed the uring_cmd. The tag can no longer be fetched, and
> device teardown may hang or complete a stale command.
>
> Fixes: 52460dda3a77 ("ublk: move auto buffer register handling into one dedicated helper")
> Signed-off-by: Yang Xiuwei <[email protected]>
> ---
>  drivers/block/ublk_drv.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index c2c11f2a01e7..d932a4026031 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -3295,9 +3295,9 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_device *ub,
>          * FETCH, so it is fine even for IO_URING_F_NONBLOCK.
>          */
>         mutex_lock(&ub->mutex);
> -       ret = __ublk_fetch(cmd, ub, io, q_id);
> +       ret = ublk_config_io_buf(ub, io, cmd, buf_addr, NULL);
>         if (!ret)
> -               ret = ublk_config_io_buf(ub, io, cmd, buf_addr, NULL);
> +               ret = __ublk_fetch(cmd, ub, io, q_id);

I'm not sure this ordering works either. __ublk_fetch() is responsible
for checking that the I/O is in a valid state to fetch (device not yet
started, I/O not yet fetched). It wouldn't be safe to overwrite
io->buf before this check. Probably it would make sense to perform the
sqe->addr validation before calling __ublk_fetch() and then make
ublk_config_io_buf() infallible.

Best,
Caleb


>         if (!ret)
>                 ublk_mark_io_ready(ub, q_id, io);
>         mutex_unlock(&ub->mutex);
> @@ -3447,19 +3447,18 @@ static int ublk_ch_uring_cmd_local(struct io_uring_cmd *cmd,
>                 if (ret)
>                         goto out;
>                 io->res = result;
> -               req = ublk_fill_io_cmd(io, cmd);
>                 ret = ublk_config_io_buf(ub, io, cmd, addr, &buf_idx);
>                 if (buf_idx != UBLK_INVALID_BUF_IDX)
>                         io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
> +               if (ret)
> +                       goto out;
> +               req = ublk_fill_io_cmd(io, cmd);
>                 compl = ublk_need_complete_req(ub, io);
>
>                 if (req_op(req) == REQ_OP_ZONE_APPEND)
>                         req->__sector = addr;
>                 if (compl)
>                         __ublk_complete_rq(req, io, ublk_dev_need_map_io(ub), NULL);
> -
> -               if (ret)
> -                       goto out;
>                 break;
>         case UBLK_IO_NEED_GET_DATA:
>                 /*
> --
> 2.25.1
>
>