On Mon, Nov 21, 2022 at 10:59:23AM +0200, Alvaro Karsz wrote: > Implement the VIRTIO_BLK_F_LIFETIME feature for VirtIO block devices. > > This commit introduces a new ioctl command, VBLK_LIFETIME.
How about naming it VBLK_GET_LIFETIME? It's clearer what the ioctl does and it follows the name of virtio-blk request type. > > VBLK_LIFETIME ioctl asks for the block device to provide lifetime > information by sending a VIRTIO_BLK_T_GET_LIFETIME command to the device. > > lifetime information fields: > > - pre_eol_info: specifies the percentage of reserved blocks that are > consumed. > optional values following virtio spec: > *) 0 - undefined. > *) 1 - normal, < 80% of reserved blocks are consumed. > *) 2 - warning, 80% of reserved blocks are consumed. > *) 3 - urgent, 90% of reserved blocks are consumed. > > - device_lifetime_est_typ_a: this field refers to wear of SLC cells and > is provided in increments of 10used, and so > on, thru to 11 meaning estimated lifetime > exceeded. All values above 11 are reserved. > > - device_lifetime_est_typ_b: this field refers to wear of MLC cells and is > provided with the same semantics as > device_lifetime_est_typ_a. > > The data received from the device will be sent as is to the user. > No data check/decode is done by virtblk. > > Signed-off-by: Alvaro Karsz <[email protected]> > -- > v2: > - Remove (void *) casting. > - Fix comments format in virtio_blk.h. > - Set ioprio value for legacy devices for REQ_OP_DRV_IN > operations. > -- > --- > drivers/block/virtio_blk.c | 100 ++++++++++++++++++++++++++++++-- > include/uapi/linux/virtio_blk.h | 32 ++++++++++ > 2 files changed, 127 insertions(+), 5 deletions(-) > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > index 19da5defd73..9aa37677b65 100644 > --- a/drivers/block/virtio_blk.c > +++ b/drivers/block/virtio_blk.c > @@ -101,6 +101,18 @@ static inline blk_status_t virtblk_result(struct > virtblk_req *vbr) > } > } > > +static inline int virtblk_ioctl_result(struct virtblk_req *vbr) > +{ > + switch (vbr->status) { > + case VIRTIO_BLK_S_OK: > + return 0; > + case VIRTIO_BLK_S_UNSUPP: > + return -ENOTTY; ENOTTY already has meaning for ioctl(2): ENOTTY fd is not associated with a character special device. ENOTTY The specified request does not apply to the kind of object that the file descriptor fd references. Use ENOTSUP instead? > + default: > + return -EIO; > + } > +} > + > static inline struct virtio_blk_vq *get_virtio_blk_vq(struct blk_mq_hw_ctx > *hctx) > { > struct virtio_blk *vblk = hctx->queue->queuedata; > @@ -218,6 +230,7 @@ static blk_status_t virtblk_setup_cmd(struct > virtio_device *vdev, > u32 type; > > vbr->out_hdr.sector = 0; > + vbr->out_hdr.ioprio = cpu_to_virtio32(vdev, req_get_ioprio(req)); > > switch (req_op(req)) { > case REQ_OP_READ: > @@ -244,15 +257,14 @@ static blk_status_t virtblk_setup_cmd(struct > virtio_device *vdev, > type = VIRTIO_BLK_T_SECURE_ERASE; > break; > case REQ_OP_DRV_IN: > - type = VIRTIO_BLK_T_GET_ID; > - break; > + /* type is set in virtblk_get_id/virtblk_ioctl_lifetime */ > + return 0; > default: > WARN_ON_ONCE(1); > return BLK_STS_IOERR; > } > > vbr->out_hdr.type = cpu_to_virtio32(vdev, type); > - vbr->out_hdr.ioprio = cpu_to_virtio32(vdev, req_get_ioprio(req)); > > if (type == VIRTIO_BLK_T_DISCARD || type == VIRTIO_BLK_T_WRITE_ZEROES || > type == VIRTIO_BLK_T_SECURE_ERASE) { > @@ -459,12 +471,16 @@ static int virtblk_get_id(struct gendisk *disk, char > *id_str) > struct virtio_blk *vblk = disk->private_data; > struct request_queue *q = vblk->disk->queue; > struct request *req; > + struct virtblk_req *vbr; > int err; > > req = blk_mq_alloc_request(q, REQ_OP_DRV_IN, 0); > if (IS_ERR(req)) > return PTR_ERR(req); > > + vbr = blk_mq_rq_to_pdu(req); > + vbr->out_hdr.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_GET_ID); > + > err = blk_rq_map_kern(q, req, id_str, VIRTIO_BLK_ID_BYTES, GFP_KERNEL); > if (err) > goto out; > @@ -508,6 +524,79 @@ static int virtblk_getgeo(struct block_device *bd, > struct hd_geometry *geo) > return ret; > } > > +/* Get lifetime information from device */ > +static int virtblk_ioctl_lifetime(struct virtio_blk *vblk, unsigned long arg) > +{ > + struct request_queue *q = vblk->disk->queue; > + struct request *req = NULL; > + struct virtblk_req *vbr; > + struct virtio_blk_lifetime lifetime; > + int ret; > + > + /* The virtio_blk_lifetime struct fields follow virtio spec. > + * There is no check/decode on values received from the device. > + * The data is sent as is to the user. > + */ In terms of security, any process with access to the block device node is allowed to get the lifetime? Usually only privileged processes have access to block device nodes, but I wanted to check whether anyone can think of a scenario where this is not okay. For example, a virtio-blk device may have a partition that an untrusted process like a database or key-value store accesses. Can the untrusted process read the lifetime information of the entire device? > + > + /* This ioctl is allowed only if VIRTIO_BLK_F_LIFETIME > + * feature is negotiated. > + */ > + if (!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_LIFETIME)) > + return -ENOTTY; > + > + memset(&lifetime, 0, sizeof(lifetime)); > + > + req = blk_mq_alloc_request(q, REQ_OP_DRV_IN, 0); > + if (IS_ERR(req)) > + return PTR_ERR(req); > + > + /* Write the correct type */ > + vbr = blk_mq_rq_to_pdu(req); > + vbr->out_hdr.type = cpu_to_virtio32(vblk->vdev, > VIRTIO_BLK_T_GET_LIFETIME); > + > + ret = blk_rq_map_kern(q, req, &lifetime, sizeof(lifetime), GFP_KERNEL); > + if (ret) > + goto out; > + > + blk_execute_rq(req, false); > + > + ret = virtblk_ioctl_result(blk_mq_rq_to_pdu(req)); > + if (ret) > + goto out; > + > + /* Pass the data to the user */ > + if (copy_to_user((void __user *)arg, &lifetime, sizeof(lifetime))) { > + ret = -EFAULT; > + goto out; > + } It's unusual for an ioctl to produce a struct that's not in CPU endianness. I think the kernel should deal with endianness here. > + > +out: > + blk_mq_free_request(req); > + return ret; > +} > + > +static int virtblk_ioctl(struct block_device *bd, fmode_t mode, > + unsigned int cmd, unsigned long arg) > +{ > + struct virtio_blk *vblk = bd->bd_disk->private_data; > + int ret; > + > + mutex_lock(&vblk->vdev_mutex); We need to check that vblk->vdev is non-NULL before accessing it in virtblk_ioctl_lifetime(): if (!vblk->vdev) { mutex_unlock(&vblk->dev_mutex); return -ENXIO; } Without the check I expect virtblk_ioctl_lifetime() to dereference a NULL pointer. > + > + switch (cmd) { > + case VBLK_LIFETIME: > + ret = virtblk_ioctl_lifetime(vblk, arg); > + break; > + default: > + ret = -ENOTTY; > + break; > + } > + > + mutex_unlock(&vblk->vdev_mutex); > + > + return ret; > +} > + > static void virtblk_free_disk(struct gendisk *disk) > { > struct virtio_blk *vblk = disk->private_data; > @@ -520,6 +609,7 @@ static void virtblk_free_disk(struct gendisk *disk) > static const struct block_device_operations virtblk_fops = { > .owner = THIS_MODULE, > .getgeo = virtblk_getgeo, > + .ioctl = virtblk_ioctl, > .free_disk = virtblk_free_disk, > }; > > @@ -1239,7 +1329,7 @@ static unsigned int features_legacy[] = { > VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, > VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE, > VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES, > - VIRTIO_BLK_F_SECURE_ERASE, > + VIRTIO_BLK_F_SECURE_ERASE, VIRTIO_BLK_F_LIFETIME, > } > ; > static unsigned int features[] = { > @@ -1247,7 +1337,7 @@ static unsigned int features[] = { > VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, > VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE, > VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES, > - VIRTIO_BLK_F_SECURE_ERASE, > + VIRTIO_BLK_F_SECURE_ERASE, VIRTIO_BLK_F_LIFETIME, > }; > > static struct virtio_driver virtio_blk = { > diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h > index 58e70b24b50..c769930d269 100644 > --- a/include/uapi/linux/virtio_blk.h > +++ b/include/uapi/linux/virtio_blk.h > @@ -40,6 +40,7 @@ > #define VIRTIO_BLK_F_MQ 12 /* support more than one vq */ > #define VIRTIO_BLK_F_DISCARD 13 /* DISCARD is supported */ > #define VIRTIO_BLK_F_WRITE_ZEROES 14 /* WRITE ZEROES is supported */ > +#define VIRTIO_BLK_F_LIFETIME 15 /* Storage lifetime information is > supported */ > #define VIRTIO_BLK_F_SECURE_ERASE 16 /* Secure Erase is supported */ > > /* Legacy feature bits */ > @@ -165,6 +166,9 @@ struct virtio_blk_config { > /* Get device ID command */ > #define VIRTIO_BLK_T_GET_ID 8 > > +/* Get lifetime information command */ > +#define VIRTIO_BLK_T_GET_LIFETIME 10 > + > /* Discard command */ > #define VIRTIO_BLK_T_DISCARD 11 > > @@ -206,6 +210,30 @@ struct virtio_blk_discard_write_zeroes { > __le32 flags; > }; > > +/* Get lifetime information struct for each request */ > +struct virtio_blk_lifetime { > + /* > + * specifies the percentage of reserved blocks that are consumed. > + * optional values following virtio spec: > + * 0 - undefined > + * 1 - normal, < 80% of reserved blocks are consumed > + * 2 - warning, 80% of reserved blocks are consumed > + * 3 - urgent, 90% of reserved blocks are consumed > + */ > + __le16 pre_eol_info; > + /* > + * this field refers to wear of SLC cells and is provided in increments > of 10used, > + * and so on, thru to 11 meaning estimated lifetime exceeded. All > values above 11 > + * are reserved > + */ > + __le16 device_lifetime_est_typ_a; > + /* > + * this field refers to wear of MLC cells and is provided with the same > semantics as > + * device_lifetime_est_typ_a > + */ > + __le16 device_lifetime_est_typ_b; > +}; > + > #ifndef VIRTIO_BLK_NO_LEGACY > struct virtio_scsi_inhdr { > __virtio32 errors; > @@ -219,4 +247,8 @@ struct virtio_scsi_inhdr { > #define VIRTIO_BLK_S_OK 0 > #define VIRTIO_BLK_S_IOERR 1 > #define VIRTIO_BLK_S_UNSUPP 2 > + > +/* Virtblk ioctl commands */ > +#define VBLK_LIFETIME _IOR('r', 0, struct virtio_blk_lifetime) > + > #endif /* _LINUX_VIRTIO_BLK_H */ > -- > 2.32.0 >
signature.asc
Description: PGP signature
_______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
