On Mon, Jun 17, 2024 at 01:44:48PM -0600, Uday Shankar wrote:
> Only certain combinations of recovery flags are valid. For example,
> setting UBLK_F_USER_RECOVERY_REISSUE without also setting
> UBLK_F_USER_RECOVERY is currently silently equivalent to not setting any
> recovery flags. Check for such issues and fail add_dev if they are
> detected.
>
> Signed-off-by: Uday Shankar <ushankar@xxxxxxxxxxxxxxx>
> ---
> drivers/block/ublk_drv.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 4e159948c912..2752a9afe9d4 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -59,6 +59,9 @@
> | UBLK_F_USER_COPY \
> | UBLK_F_ZONED)
>
> +#define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
> + | UBLK_F_USER_RECOVERY_REISSUE)
> +
> /* All UBLK_PARAM_TYPE_* should be included here */
> #define UBLK_PARAM_TYPE_ALL \
> (UBLK_PARAM_TYPE_BASIC | UBLK_PARAM_TYPE_DISCARD | \
> @@ -2341,6 +2344,18 @@ static int ublk_ctrl_add_dev(struct io_uring_cmd *cmd)
> else if (!(info.flags & UBLK_F_UNPRIVILEGED_DEV))
> return -EPERM;
>
> + /* forbid nonsense combinations of recovery flags */
> + switch (info.flags & UBLK_F_ALL_RECOVERY_FLAGS) {
> + case 0:
> + case UBLK_F_USER_RECOVERY:
> + case (UBLK_F_USER_RECOVERY | UBLK_F_USER_RECOVERY_REISSUE):
> + break;
> + default:
> + pr_warn("%s: invalid recovery flags %llx\n", __func__,
> + info.flags & UBLK_F_ALL_RECOVERY_FLAGS);
> + return -EINVAL;
> + }
> +
It could be cleaner and more readable to check the fail condition only:
if ((info.flags & UBLK_F_USER_RECOVERY_REISSUE) &&
!(info.flags & UBLK_F_USER_RECOVERY)) {
...
}
Thanks,
Ming
