Re: [PATCH-v3 5/9] vhost/scsi: Add ANY_LAYOUT vhost_virtqueue callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2015-02-04 at 02:41 -0800, Nicholas A. Bellinger wrote:
> On Wed, 2015-02-04 at 10:42 +0100, Michael S. Tsirkin wrote:
> > On Wed, Feb 04, 2015 at 01:40:25AM -0800, Nicholas A. Bellinger wrote:
> > > > > +		/*
> > > > > +		 * Any associated T10_PI bytes for the outgoing / incoming
> > > > > +		 * payloads are included in calculation of exp_data_len here.
> > > > > +		 */
> > > > > +		if (out_size > req_size) {
> > > > > +			data_direction = DMA_TO_DEVICE;
> > > > > +			exp_data_len = out_size - req_size;
> > > > > +		} else if (in_size > rsp_size) {
> > > > > +			data_direction = DMA_FROM_DEVICE;
> > > > > +			exp_data_len = in_size - rsp_size;
> > > > > +		} else {
> > > > > +			data_direction = DMA_NONE;
> > > > > +			exp_data_len = 0;
> > > > > +		}
> > > > 
> > > > We must validate this doesn't cause exp_data_len to be negative.
> > > > 
> > > 
> > > AFAICT, exp_data_len is always >= 0 here.
> > 
> > What guarantees out_size > req_size and in_size > rsp_size,
> > respectively?
> > 
> 
> Mmm, point taken.
> 
> So moving this part after copy_from_iter() ensures that at least
> req_size bytes exists of out_size.  Making this change now.
> 
> For in_size > rsp_size there is no guarantee, and falls back to
> data_direction = DMA_NONE + exp_data_len = 0;
> 
> Is this what you had in mind..?
> 

Something akin to:

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index f72fc56..daf10b7 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1047,30 +1047,12 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
 			target = &v_req.lun[1];
 		}
 		/*
-		 * Determine data_direction by calculating the total outgoing
-		 * iovec sizes / incoming iovec sizes vs. virtio-scsi request /
-		 * response headers respectively.
-		 *
 		 * FIXME: Not correct for BIDI operation
 		 */
 		out_size = iov_length(vq->iov, out);
 		in_size = iov_length(&vq->iov[out], in);
 
 		/*
-		 * Any associated T10_PI bytes for the outgoing / incoming
-		 * payloads are included in calculation of exp_data_len here.
-		 */
-		if (out_size > req_size) {
-			data_direction = DMA_TO_DEVICE;
-			exp_data_len = out_size - req_size;
-		} else if (in_size > rsp_size) {
-			data_direction = DMA_FROM_DEVICE;
-			exp_data_len = in_size - rsp_size;
-		} else {
-			data_direction = DMA_NONE;
-			exp_data_len = 0;
-		}
-		/*
 		 * Copy over the virtio-scsi request header, which for a
 		 * ANY_LAYOUT enabled guest may span multiple iovecs, or a
 		 * single iovec may contain both the header + outgoing
@@ -1102,8 +1084,9 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
 			continue;
 		}
 		/*
-		 * Determine start of T10_PI or data payload iovec based upon
-		 * data_direction.
+		 * Determine data_direction by calculating the total outgoing
+		 * iovec sizes + incoming iovec sizes vs. virtio-scsi request +
+		 * response headers respectively.
 		 *
 		 * For DMA_TO_DEVICE this is out_iter, which is already pointing
 		 * to the right place.
@@ -1111,16 +1094,27 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
 		 * For DMA_FROM_DEVICE, the iovec will be just past the end
 		 * of the virtio-scsi response header in either the same
 		 * or immediately following iovec.
+		 *
+		 * Any associated T10_PI bytes for the outgoing / incoming
+		 * payloads are included in calculation of exp_data_len here.
 		 */
 		prot_bytes = 0;
 
-		if (data_direction == DMA_TO_DEVICE) {
+		if (out_size > req_size) {
+			data_direction = DMA_TO_DEVICE;
+			exp_data_len = out_size - req_size;
 			data_iter = out_iter;
-		} else if (data_direction == DMA_FROM_DEVICE) {
+		} else if (in_size > rsp_size) {
+			data_direction = DMA_FROM_DEVICE;
+			exp_data_len = in_size - rsp_size;
+
 			iov_iter_init(&in_iter, READ, &vq->iov[out], in,
 				      rsp_size + exp_data_len);
 			iov_iter_advance(&in_iter, rsp_size);
 			data_iter = in_iter;
+		} else {
+			data_direction = DMA_NONE;
+			exp_data_len = 0;
 		}
 		/*
 		 * If T10_PI header + payload is present, setup prot_iter values

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux