Re: [Devel] [PATCH VZ10 2/5] drivers/vhost/blk: report correct used-ring lengths

[Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>]

> We are expected to return amount of bytes written to the guest,
> which also includes status.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-138640
> Fixes: 40a5928ec730 ("drivers/vhost: vhost-blk accelerator for virtio-blk guests")
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
>
> diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
> index 3def988cdf18..ae6b916f96e1 100644
> --- a/drivers/vhost/blk.c
> +++ b/drivers/vhost/blk.c
> @@ -439,12 +439,14 @@ static int vhost_blk_req_submit(struct vhost_blk_req *req)
>  
>  static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>  				struct virtio_blk_outhdr *hdr,
> -				u16 head, u16 total_iov_nr)
> +				u16 head, u16 out, u16 in)
>  {
>  	struct vhost_blk *blk = container_of(vq->dev, struct vhost_blk, dev);
>  	struct vhost_blk_vq *blk_vq = container_of(vq, struct vhost_blk_vq, vq);
>  	struct vhost_blk_req *req;
> +	u16 total_iov_nr = out + in;
>  	struct iov_iter iter;
> +	size_t in_len;
>  	int ret, len;
>  	u8 status;
>  
> @@ -455,8 +457,9 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>  	req->sector	= hdr->sector;
>  	req->iov	= blk_vq->iov;
>  	req->bio_err	= 0;
> +	in_len		= iov_length(vq->iov + out, in);
>  
> -	if (iov_length(vq->iov, total_iov_nr) < sizeof(status))
> +	if (in_len < sizeof(status) || in_len > INT_MAX)
>  		return -EINVAL;
>  
>  	req->len	= iov_length(vq->iov, total_iov_nr) - sizeof(status);
> @@ -492,14 +495,14 @@ static int vhost_blk_req_handle(struct vhost_virtqueue *vq,
>  		ret = vhost_blk_set_status(req, status);
>  		if (ret)
>  			break;
> -		vhost_add_used_and_signal(&blk->dev, vq, head, len);
> +		vhost_add_used_and_signal(&blk->dev, vq, head, in_len);
>  		break;
>  	default:
>  		status = VIRTIO_BLK_S_UNSUPP;
>  		ret = vhost_blk_set_status(req, status);
>  		if (ret)
>  			break;
> -		vhost_add_used_and_signal(&blk->dev, vq, head, 0);
> +		vhost_add_used_and_signal(&blk->dev, vq, head, sizeof(status));
>  	}
>  
>  	return ret;
> @@ -555,7 +558,7 @@ static void vhost_blk_handle_guest_kick(struct vhost_work *work)
>  			break;
>  		}
>  
> -		ret = vhost_blk_req_handle(vq, &hdr, head, out + in);
> +		ret = vhost_blk_req_handle(vq, &hdr, head, out, in);
>  		if (ret == -EAGAIN || ret == -ENOMEM) {
>  			vhost_discard_vq_desc(vq, 1);
>  			vhost_poll_queue(&vq->poll);
> @@ -604,7 +607,7 @@ static void vhost_blk_handle_host_kick(struct vhost_work *work)
>  		if (vhost_blk_set_status(req, status)) {
>  			vhostblk_vq_err(blk, vq, "Failed to write status");
>  		} else {
> -			vhost_add_used(vq, req->head, req->len);
> +			vhost_add_used(vq, req->head, req->len + sizeof(status));
Is req->len + sizeof(status) the right thing to use for write requests?
We should only write the status byte not status + payload length

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>