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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Thanks,
Ming
