| Message ID | 20221124030454.476152-5-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ublk_drv: add mechanism for supporting unprivileged ublk device | expand |
On 2022/11/24 11:04, Ming Lei wrote: > Userspace side only knows device ID, but the associated path of ublkc* and > ublkb* could be changed by udev, and that depends on userspace's policy, so > add parameter of UBLK_PARAM_TYPE_DEVT for retrieving major/minor of the > ublkc* and ublkb*, then user may figure out major/minor of the ublk disks > he/she owns. With major/minor, it is easy to find the device node path. > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > drivers/block/ublk_drv.c | 24 +++++++++++++++++++++++- > include/uapi/linux/ublk_cmd.h | 13 +++++++++++++ > 2 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c > index 9d1578384cba..04a28a2f2e1f 100644 > --- a/drivers/block/ublk_drv.c > +++ b/drivers/block/ublk_drv.c > @@ -54,7 +54,8 @@ > | 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) > +#define UBLK_PARAM_TYPE_ALL (UBLK_PARAM_TYPE_BASIC | \ > + UBLK_PARAM_TYPE_DISCARD | UBLK_PARAM_TYPE_DEVT) > > struct ublk_rq_data { > struct llist_node node; > @@ -255,6 +256,10 @@ static int ublk_validate_params(const struct ublk_device *ub) > return -EINVAL; > } > > + /* dev_t is read-only */ > + if (ub->params.types & UBLK_PARAM_TYPE_DEVT) > + WARN_ON_ONCE(1); Hi, Ming ublk_validate_params() is only called by ublk_ctrl_set_params(). Why not return -EINVAL here since UBLK_PARAM_TYPE_DEVT is not allowed in ublk_ctrl_set_params(). Then the user may know he has made a mistake. Regards, Zhang
On Fri, Nov 25, 2022 at 03:13:58PM +0800, Ziyang Zhang wrote: > On 2022/11/24 11:04, Ming Lei wrote: > > Userspace side only knows device ID, but the associated path of ublkc* and > > ublkb* could be changed by udev, and that depends on userspace's policy, so > > add parameter of UBLK_PARAM_TYPE_DEVT for retrieving major/minor of the > > ublkc* and ublkb*, then user may figure out major/minor of the ublk disks > > he/she owns. With major/minor, it is easy to find the device node path. > > > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > --- > > drivers/block/ublk_drv.c | 24 +++++++++++++++++++++++- > > include/uapi/linux/ublk_cmd.h | 13 +++++++++++++ > > 2 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c > > index 9d1578384cba..04a28a2f2e1f 100644 > > --- a/drivers/block/ublk_drv.c > > +++ b/drivers/block/ublk_drv.c > > @@ -54,7 +54,8 @@ > > | 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) > > +#define UBLK_PARAM_TYPE_ALL (UBLK_PARAM_TYPE_BASIC | \ > > + UBLK_PARAM_TYPE_DISCARD | UBLK_PARAM_TYPE_DEVT) > > > > struct ublk_rq_data { > > struct llist_node node; > > @@ -255,6 +256,10 @@ static int ublk_validate_params(const struct ublk_device *ub) > > return -EINVAL; > > } > > > > + /* dev_t is read-only */ > > + if (ub->params.types & UBLK_PARAM_TYPE_DEVT) > > + WARN_ON_ONCE(1); > Hi, Ming > > ublk_validate_params() is only called by ublk_ctrl_set_params(). > Why not return -EINVAL here since UBLK_PARAM_TYPE_DEVT is not > allowed in ublk_ctrl_set_params(). Then the user may know he has > made a mistake. Yeah, it is better to return -EINVAL here. Thanks, Ming
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index 9d1578384cba..04a28a2f2e1f 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -54,7 +54,8 @@ | 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) +#define UBLK_PARAM_TYPE_ALL (UBLK_PARAM_TYPE_BASIC | \ + UBLK_PARAM_TYPE_DISCARD | UBLK_PARAM_TYPE_DEVT) struct ublk_rq_data { struct llist_node node; @@ -255,6 +256,10 @@ static int ublk_validate_params(const struct ublk_device *ub) return -EINVAL; } + /* dev_t is read-only */ + if (ub->params.types & UBLK_PARAM_TYPE_DEVT) + WARN_ON_ONCE(1); + return 0; } @@ -1777,6 +1782,22 @@ static int ublk_ctrl_get_dev_info(struct ublk_device *ub, return 0; } +/* TYPE_DEVT is readonly, so fill it up before returning to userspace */ +static void ublk_ctrl_fill_params_devt(struct ublk_device *ub) +{ + ub->params.devt.char_major = MAJOR(ub->cdev_dev.devt); + ub->params.devt.char_minor = MINOR(ub->cdev_dev.devt); + + if (ub->ub_disk) { + ub->params.devt.disk_major = MAJOR(disk_devt(ub->ub_disk)); + ub->params.devt.disk_minor = MINOR(disk_devt(ub->ub_disk)); + } else { + ub->params.devt.disk_major = 0; + ub->params.devt.disk_minor = 0; + } + ub->params.types |= UBLK_PARAM_TYPE_DEVT; +} + static int ublk_ctrl_get_params(struct ublk_device *ub, struct io_uring_cmd *cmd) { @@ -1798,6 +1819,7 @@ static int ublk_ctrl_get_params(struct ublk_device *ub, ph.len = sizeof(struct ublk_params); mutex_lock(&ub->mutex); + ublk_ctrl_fill_params_devt(ub); if (copy_to_user(argp, &ub->params, ph.len)) ret = -EFAULT; else diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h index 8f88e3a29998..4e38b9aa0293 100644 --- a/include/uapi/linux/ublk_cmd.h +++ b/include/uapi/linux/ublk_cmd.h @@ -214,6 +214,17 @@ struct ublk_param_discard { __u16 reserved0; }; +/* + * read-only, can't set via UBLK_CMD_SET_PARAMS, disk_devt is available + * after device is started + */ +struct ublk_param_devt { + __u32 char_major; + __u32 char_minor; + __u32 disk_major; + __u32 disk_minor; +}; + struct ublk_params { /* * Total length of parameters, userspace has to set 'len' for both @@ -224,10 +235,12 @@ struct ublk_params { __u32 len; #define UBLK_PARAM_TYPE_BASIC (1 << 0) #define UBLK_PARAM_TYPE_DISCARD (1 << 1) +#define UBLK_PARAM_TYPE_DEVT (1 << 2) __u32 types; /* types of parameter included */ struct ublk_param_basic basic; struct ublk_param_discard discard; + struct ublk_param_devt devt; }; #endif
Userspace side only knows device ID, but the associated path of ublkc* and ublkb* could be changed by udev, and that depends on userspace's policy, so add parameter of UBLK_PARAM_TYPE_DEVT for retrieving major/minor of the ublkc* and ublkb*, then user may figure out major/minor of the ublk disks he/she owns. With major/minor, it is easy to find the device node path. Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/block/ublk_drv.c | 24 +++++++++++++++++++++++- include/uapi/linux/ublk_cmd.h | 13 +++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-)