Re: [PATCH] io_uring/ublk: report error when unregister operation fails

[Ming Lei <ming.lei@redhat.com>]

On Fri, Feb 28, 2025 at 04:14:31PM -0700, Caleb Sander Mateos wrote:
> Indicate to userspace applications if a UBLK_IO_UNREGISTER_IO_BUF
> command specifies an invalid buffer index by returning an error code.
> Return -EINVAL if no buffer is registered with the given index, and
> -EBUSY if the registered buffer is not a kernel bvec.
> 
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  drivers/block/ublk_drv.c     |  3 +--
>  include/linux/io_uring/cmd.h |  4 ++--
>  io_uring/rsrc.c              | 18 ++++++++++++++----
>  3 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index b5cf92baaf0f..512cbd456817 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -1785,12 +1785,11 @@ static int ublk_register_io_buf(struct io_uring_cmd *cmd,
>  
>  static int ublk_unregister_io_buf(struct io_uring_cmd *cmd,
>  				  const struct ublksrv_io_cmd *ub_cmd,
>  				  unsigned int issue_flags)
>  {
> -	io_buffer_unregister_bvec(cmd, ub_cmd->addr, issue_flags);
> -	return 0;
> +	return io_buffer_unregister_bvec(cmd, ub_cmd->addr, issue_flags);
>  }
>  
>  static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
>  			       unsigned int issue_flags,
>  			       const struct ublksrv_io_cmd *ub_cmd)
> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> index cf8d80d84734..05d7b6145731 100644
> --- a/include/linux/io_uring/cmd.h
> +++ b/include/linux/io_uring/cmd.h
> @@ -127,9 +127,9 @@ static inline struct io_uring_cmd_data *io_uring_cmd_get_async_data(struct io_ur
>  }
>  
>  int io_buffer_register_bvec(struct io_uring_cmd *cmd, struct request *rq,
>  			    void (*release)(void *), unsigned int index,
>  			    unsigned int issue_flags);
> -void io_buffer_unregister_bvec(struct io_uring_cmd *cmd, unsigned int index,
> -			       unsigned int issue_flags);
> +int io_buffer_unregister_bvec(struct io_uring_cmd *cmd, unsigned int index,
> +			      unsigned int issue_flags);
>  
>  #endif /* _LINUX_IO_URING_CMD_H */
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index 45bfb37bca1e..29c0c31092eb 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -975,30 +975,40 @@ int io_buffer_register_bvec(struct io_uring_cmd *cmd, struct request *rq,
>  	io_ring_submit_unlock(ctx, issue_flags);
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(io_buffer_register_bvec);
>  
> -void io_buffer_unregister_bvec(struct io_uring_cmd *cmd, unsigned int index,
> -			       unsigned int issue_flags)
> +int io_buffer_unregister_bvec(struct io_uring_cmd *cmd, unsigned int index,
> +			      unsigned int issue_flags)
>  {
>  	struct io_ring_ctx *ctx = cmd_to_io_kiocb(cmd)->ctx;
>  	struct io_rsrc_data *data = &ctx->buf_table;
>  	struct io_rsrc_node *node;
> +	int ret = 0;
>  
>  	io_ring_submit_lock(ctx, issue_flags);
> -	if (index >= data->nr)
> +	if (index >= data->nr) {
> +		ret = -EINVAL;
>  		goto unlock;
> +	}
>  	index = array_index_nospec(index, data->nr);
>  
>  	node = data->nodes[index];
> -	if (!node || !node->buf->is_kbuf)
> +	if (!node) {
> +		ret = -EINVAL;
>  		goto unlock;
> +	}
> +	if (!node->buf->is_kbuf) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}

Good catch, otherwise, ublk request may never get completed if unreg
command fails, which can happen really as one uring_cmd.

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Thanks,
Ming