Re: [PATCH] ublk: set_params: properly check if parameters can be applied

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

On Tue, Mar 04, 2025 at 02:34:26PM -0700, Uday Shankar wrote:
> The parameters set by the set_params call are only applied to the block
> device in the start_dev call. So if a device has already been started, a
> subsequently issued set_params on that device will not have the desired
> effect, and should return an error. There is an existing check for this
> - set_params fails on devices in the LIVE state. But this check is not
> sufficient to cover the recovery case. In this case, the device will be
> in the QUIESCED or FAIL_IO states, so set_params will succeed. But this
> success is misleading, because the parameters will not be applied, since
> the device has already been started (by a previous ublk server). The bit
> UB_STATE_USED is set on completion of the start_dev; use it to detect
> and fail set_params commands which arrive too late to be applied (after
> start_dev).
> 
> Signed-off-by: Uday Shankar <ushankar@purestorage.com>
> ---
>  drivers/block/ublk_drv.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 2955900ee713c5d8f3cbc2a69f6f6058348e5253..aa34594c76ad02b1480b9ef4a2bd52a095ca6f3f 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2782,9 +2782,12 @@ static int ublk_ctrl_set_params(struct ublk_device *ub,
>  	if (ph.len > sizeof(struct ublk_params))
>  		ph.len = sizeof(struct ublk_params);
>  
> -	/* parameters can only be changed when device isn't live */
>  	mutex_lock(&ub->mutex);
> -	if (ub->dev_info.state == UBLK_S_DEV_LIVE) {
> +	if (test_bit(UB_STATE_USED, &ub->state)) {
> +		/*
> +		 * Parameters can only be changed when device hasn't
> +		 * been started yet
> +		 */
>  		ret = -EACCES;
>  	} else if (copy_from_user(&ub->params, argp, ph.len)) {
>  		ret = -EFAULT;

Good catch,

Fixes: 0aa73170eba5 ("ublk_drv: add SET_PARAMS/GET_PARAMS control command")
Reviewed-by: Ming Lei <ming.lei@redhat.com>


Thanks,
Ming