Author: sephe
Date: Fri Jul 15 02:29:10 2016
New Revision: 302863
URL: https://svnweb.freebsd.org/changeset/base/302863
Log:
MFC 302541,302605
302541
hyperv/stor: Fix the INQUIRY checks
Don't check the area that the host has not filled.
PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209443
PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210425
Submitted by: Hongjiang Zhang <honzhan microsoft com>
Reviewed by: sephe, Dexuan Cui <decui microsoft com>
Sponsored by: Microsoft OSTC
Differential Revision: https://reviews.freebsd.org/D6955
302605
hyperv/stor: Save the response status and xfer length properly.
The current command response handling discards status and xfer
length unconditionally, so that all of the commands would be
considered successful, even if errors happened. When errors
really happens, this causes all kinds of wiredness, since the
buffer will not be filled on the host side and sense data will
be ignored.
Most of the time, errors do not happen, however, error does
happen for the request sent immediately after the disk resizing.
Discarding the SCSI status (SCSI_STATUS_CHECK_COND) and sense
data (capacity changes) prevents the disk resizing from working
properly.
This commit saves the response status and xfer length properly
for later use.
Submitted by: Dexuan Cui <decui microsoft com>
Noticed by: sephe
Sponsored by: Microsoft OSTC
Differential Revision: https://reviews.freebsd.org/D7181
Modified:
stable/10/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c
Directory Properties:
stable/10/ (props changed)
Modified: stable/10/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c
==============================================================================
--- stable/10/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c Fri Jul 15
02:24:39 2016 (r302862)
+++ stable/10/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c Fri Jul 15
02:29:10 2016 (r302863)
@@ -805,6 +805,13 @@ hv_storvsc_on_iocompletion(struct storvs
vm_srb = &vstor_packet->u.vm_srb;
+ /*
+ * Copy some fields of the host's response into the request structure,
+ * because the fields will be used later in storvsc_io_done().
+ */
+ request->vstor_packet.u.vm_srb.scsi_status = vm_srb->scsi_status;
+ request->vstor_packet.u.vm_srb.transfer_len = vm_srb->transfer_len;
+
if (((vm_srb->scsi_status & 0xFF) == SCSI_STATUS_CHECK_COND) &&
(vm_srb->srb_status & SRB_STATUS_AUTOSENSE_VALID)) {
/* Autosense data available */
@@ -1939,62 +1946,24 @@ create_storvsc_request(union ccb *ccb, s
}
/*
- * Modified based on scsi_print_inquiry which is responsible to
- * print the detail information for scsi_inquiry_data.
- *
+ * SCSI Inquiry checks qualifier and type.
+ * If qualifier is 011b, means the device server is not capable
+ * of supporting a peripheral device on this logical unit, and
+ * the type should be set to 1Fh.
+ *
* Return 1 if it is valid, 0 otherwise.
*/
static inline int
is_inquiry_valid(const struct scsi_inquiry_data *inq_data)
{
uint8_t type;
- char vendor[16], product[48], revision[16];
-
- /*
- * Check device type and qualifier
- */
- if (!(SID_QUAL_IS_VENDOR_UNIQUE(inq_data) ||
- SID_QUAL(inq_data) == SID_QUAL_LU_CONNECTED))
+ if (SID_QUAL(inq_data) != SID_QUAL_LU_CONNECTED) {
return (0);
-
+ }
type = SID_TYPE(inq_data);
- switch (type) {
- case T_DIRECT:
- case T_SEQUENTIAL:
- case T_PRINTER:
- case T_PROCESSOR:
- case T_WORM:
- case T_CDROM:
- case T_SCANNER:
- case T_OPTICAL:
- case T_CHANGER:
- case T_COMM:
- case T_STORARRAY:
- case T_ENCLOSURE:
- case T_RBC:
- case T_OCRW:
- case T_OSD:
- case T_ADC:
- break;
- case T_NODEVICE:
- default:
+ if (type == T_NODEVICE) {
return (0);
}
-
- /*
- * Check vendor, product, and revision
- */
- cam_strvis(vendor, inq_data->vendor, sizeof(inq_data->vendor),
- sizeof(vendor));
- cam_strvis(product, inq_data->product, sizeof(inq_data->product),
- sizeof(product));
- cam_strvis(revision, inq_data->revision, sizeof(inq_data->revision),
- sizeof(revision));
- if (strlen(vendor) == 0 ||
- strlen(product) == 0 ||
- strlen(revision) == 0)
- return (0);
-
return (1);
}
@@ -2071,7 +2040,6 @@ storvsc_io_done(struct hv_storvsc_reques
ccb->ccb_h.status &= ~CAM_STATUS_MASK;
if (vm_srb->scsi_status == SCSI_STATUS_OK) {
const struct scsi_generic *cmd;
-
/*
* Check whether the data for INQUIRY cmd is valid or
* not. Windows 10 and Windows 2016 send all zero
@@ -2080,23 +2048,59 @@ storvsc_io_done(struct hv_storvsc_reques
cmd = (const struct scsi_generic *)
((ccb->ccb_h.flags & CAM_CDB_POINTER) ?
csio->cdb_io.cdb_ptr : csio->cdb_io.cdb_bytes);
- if (cmd->opcode == INQUIRY &&
- /*
- * XXX: Temporary work around disk hot plugin on win2k12r2,
- * only filtering the invalid disk on win10 or 2016 server.
- * So, the hot plugin on win10 and 2016 server needs
- * to be fixed.
+ if (cmd->opcode == INQUIRY) {
+ /*
+ * The host of Windows 10 or 2016 server will response
+ * the inquiry request with invalid data for unexisted
device:
+ [0x7f 0x0 0x5 0x2 0x1f ... ]
+ * But on windows 2012 R2, the response is:
+ [0x7f 0x0 0x0 0x0 0x0 ]
+ * That is why here wants to validate the inquiry response.
+ * The validation will skip the INQUIRY whose response is
short,
+ * which is less than SHORT_INQUIRY_LENGTH (36).
+ *
+ * For more information about INQUIRY, please refer to:
+ *
ftp://ftp.avc-pioneer.com/Mtfuji_7/Proposal/Jun09/INQUIRY.pdf
*/
- vmstor_proto_version == VMSTOR_PROTOCOL_VERSION_WIN10 &&
- is_inquiry_valid(
- (const struct scsi_inquiry_data *)csio->data_ptr) == 0) {
+ const struct scsi_inquiry_data *inq_data =
+ (const struct scsi_inquiry_data *)csio->data_ptr;
+ uint8_t* resp_buf = (uint8_t*)csio->data_ptr;
+ /* Get the buffer length reported by host */
+ int resp_xfer_len = vm_srb->transfer_len;
+ /* Get the available buffer length */
+ int resp_buf_len = resp_xfer_len >= 5 ? resp_buf[4] + 5 : 0;
+ int data_len = (resp_buf_len < resp_xfer_len) ?
resp_buf_len : resp_xfer_len;
+ if (data_len < SHORT_INQUIRY_LENGTH) {
+ ccb->ccb_h.status |= CAM_REQ_CMP;
+ if (bootverbose && data_len >= 5) {
+ mtx_lock(&sc->hs_lock);
+ xpt_print(ccb->ccb_h.path,
+ "storvsc skips the validation for short
inquiry (%d)"
+ " [%x %x %x %x %x]\n",
+
data_len,resp_buf[0],resp_buf[1],resp_buf[2],
+ resp_buf[3],resp_buf[4]);
+ mtx_unlock(&sc->hs_lock);
+ }
+ } else if (is_inquiry_valid(inq_data) == 0) {
ccb->ccb_h.status |= CAM_DEV_NOT_THERE;
+ if (bootverbose && data_len >= 5) {
+ mtx_lock(&sc->hs_lock);
+ xpt_print(ccb->ccb_h.path,
+ "storvsc uninstalled invalid device"
+ " [%x %x %x %x %x]\n",
+
resp_buf[0],resp_buf[1],resp_buf[2],resp_buf[3],resp_buf[4]);
+ mtx_unlock(&sc->hs_lock);
+ }
+ } else {
+ ccb->ccb_h.status |= CAM_REQ_CMP;
if (bootverbose) {
mtx_lock(&sc->hs_lock);
xpt_print(ccb->ccb_h.path,
- "storvsc uninstalled device\n");
+ "storvsc has passed inquiry response (%d)
validation\n",
+ data_len);
mtx_unlock(&sc->hs_lock);
}
+ }
} else {
ccb->ccb_h.status |= CAM_REQ_CMP;
}
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
