Re: [Xen-devel] [PATCH 1/4] pvSCSI : Add white list to SCSI command emulation

[Steven Smith <steven.smith@xxxxxxxxxx>]

> > > + bitmap[READ_CAPACITY] = VSCSIIF_NEED_CMD_EXEC;
> > > + pre_function[READ_CAPACITY] = NULL;
> > > + post_function[READ_CAPACITY] = NULL;
> > > +
> > > + bitmap[READ_10] = VSCSIIF_NEED_CMD_EXEC;
> > > + pre_function[READ_10] = NULL;
> > > + post_function[READ_10] = NULL;
> > > +
> > > + bitmap[WRITE_10] = VSCSIIF_NEED_CMD_EXEC;
> > > + pre_function[WRITE_10] = NULL;
> > > + post_function[WRITE_10] = NULL;
> > Do you know what happens if the SCSI CDB is inconsistent with the
> > scatter list?  For instance, if the CDB says to read 16 sectors but
> > the frontend-supplied buffer is only big enough for 8, are we going to
> > end up DMAing over random memory?
> I consider that native SCSI driver, which generates DMA request, will 
> reject such the inconsistent request. The native driver generates 
> multiple CDB requests according to the number of scatter/gather 
> segments. At the same time, doesn't the native driver check such the 
> inconsistency?
> So, if there are any inconsistency, pvSCSI backend driver will just 
> get an error from native driver.
> 
> Perhaps, do you warry about inconsitency between size of some segment
> frontend driver says, and size of the segment the frontend driver
> actually allocated?
Suppose the frontend generates a READ command with the CDB
TransferLength field set to 128 sectors, but the nr_segments field of
the vscsiif_request_t set to 1.  scsiback will then a scsi_cmnd whose
CDB says to read 128 sectors, but whose associated scatter-gather list
only has room for 8, and pass that off to blk_execute_rq_nowait().
What happens to the other 120 sectors?  Hopefully, something will
detect the inconsistency and make the request fail, but I can't really
see what.  It would be very bad if the request made it down to the
physical card, which tried to satisfy it by DMAing the 120 sectors
into some random bit of memory.  I've had a quick look around
drivers/scsi and I can't immediately see anything which tries to do
the required kind of consistency checking.  On the other hand, I'd
have expected sg.c to need something, at least, and it doesn't, so
perhaps it's safe to rely on the hardware to do the checks.

Actually doing the checking could prove tricky, as well, because of
commands like READ6 where you need to know the type of device which is
being addressed.  For block devices, the command has the logical block
address in bytes 2 and 3, with the transfer length in byte 4, but
streaming devices use all three bytes to encode the length (because
block address is meaningless on stream devices).  That's going to make
it tricky to design a generic validation routine.  Hmm.

Steven.

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel