svn commit: r349815 - stable/12/sys/cam/scsi

[Alexander Motin]

Author: mav
Date: Sun Jul  7 18:31:53 2019
New Revision: 349815
URL: https://svnweb.freebsd.org/changeset/base/349815

Log:
  MFC r349284: Make ELEMENT INDEX validation more strict.
  
  SES specifications tell: "The Additional Element Status descriptors shall
  be in the same order as the status elements in the Enclosure Status
  diagnostic page".  It allows us to question ELEMENT INDEX that is lower
  then values we already processed.  There are many SAS2 enclosures with
  this kind of problem.
  
  While there, add more specific error messages for cases when ELEMENT INDEX
  is obviously wrong.  Also skip elements with INVALID bit set.

Modified:
  stable/12/sys/cam/scsi/scsi_enc_ses.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/cam/scsi/scsi_enc_ses.c
==============================================================================
--- stable/12/sys/cam/scsi/scsi_enc_ses.c	Sun Jul  7 18:29:37 2019	(r349814)
+++ stable/12/sys/cam/scsi/scsi_enc_ses.c	Sun Jul  7 18:31:53 2019	(r349815)
@@ -1681,7 +1681,6 @@ ses_process_elm_addlstatus(enc_softc_t *enc, struct en
 	struct ses_iterator iter, titer;
 	int eip;
 	int err;
-	int ignore_index = 0;
 	int length;
 	int offset;
 	enc_cache_t *enc_cache;
@@ -1752,7 +1751,7 @@ ses_process_elm_addlstatus(enc_softc_t *enc, struct en
 
 		elm_hdr = (struct ses_elm_addlstatus_base_hdr *)&buf[offset];
 		eip = ses_elm_addlstatus_eip(elm_hdr);
-		if (eip && !ignore_index) {
+		if (eip) {
 			struct ses_elm_addlstatus_eip_hdr *eip_hdr;
 			int expected_index, index;
 			ses_elem_index_type_t index_type;
@@ -1765,17 +1764,44 @@ ses_process_elm_addlstatus(enc_softc_t *enc, struct en
 				index_type = SES_ELEM_INDEX_INDIVIDUAL;
 				expected_index = iter.individual_element_index;
 			}
+			if (eip_hdr->element_index < expected_index) {
+				ENC_VLOG(enc, "%s: provided %selement index "
+				    "%d is lower then expected %d\n",
+				    __func__, (eip_hdr->byte2 &
+				    SES_ADDL_EIP_EIIOE) ? "global " : "",
+				    eip_hdr->element_index, expected_index);
+				goto badindex;
+			}
 			titer = iter;
 			telement = ses_iter_seek_to(&titer,
 			    eip_hdr->element_index, index_type);
-			if (telement != NULL &&
-			    (ses_typehasaddlstatus(enc, titer.type_index) !=
-			     TYPE_ADDLSTATUS_NONE ||
-			     titer.type_index > ELMTYP_SAS_CONN)) {
+			if (telement == NULL) {
+				ENC_VLOG(enc, "%s: provided %selement index "
+				    "%d does not exist\n", __func__,
+				    (eip_hdr->byte2 & SES_ADDL_EIP_EIIOE) ?
+				    "global " : "", eip_hdr->element_index);
+				goto badindex;
+			}
+			if (ses_typehasaddlstatus(enc, titer.type_index) ==
+			    TYPE_ADDLSTATUS_NONE) {
+				ENC_VLOG(enc, "%s: provided %selement index "
+				    "%d can't have additional status\n",
+				    __func__,
+				    (eip_hdr->byte2 & SES_ADDL_EIP_EIIOE) ?
+				    "global " : "", eip_hdr->element_index);
+badindex:
+				/*
+				 * If we expected mandatory element, we may
+				 * guess it was just a wrong index and we may
+				 * use the status.  If element was optional,
+				 * then we have no idea where status belongs.
+				 */
+				if (status_type == TYPE_ADDLSTATUS_OPTIONAL)
+					break;
+			} else {
 				iter = titer;
 				element = telement;
-			} else
-				ignore_index = 1;
+			}
 
 			if (eip_hdr->byte2 & SES_ADDL_EIP_EIIOE)
 				index = iter.global_element_index;
@@ -1797,35 +1823,41 @@ ses_process_elm_addlstatus(enc_softc_t *enc, struct en
 		    "type element index=%d, offset=0x%x, "
 		    "byte0=0x%x, length=0x%x\n", __func__,
 		    iter.global_element_index, iter.type_index,
-		    iter.type_element_index, offset, elmpriv->addl.hdr->byte0,
-		    elmpriv->addl.hdr->length);
+		    iter.type_element_index, offset, elm_hdr->byte0,
+		    elm_hdr->length);
 
 		/* Skip to after the length field */
 		offset += sizeof(struct ses_elm_addlstatus_base_hdr);
 
 		/* Make sure the descriptor is within bounds */
-		if ((offset + elmpriv->addl.hdr->length) > length) {
+		if ((offset + elm_hdr->length) > length) {
 			ENC_VLOG(enc, "Element %d Beyond End "
 			    "of Additional Element Status Descriptors\n",
 			    iter.global_element_index);
 			break;
 		}
 
+		/* Skip elements marked as invalid. */
+		if (ses_elm_addlstatus_invalid(elm_hdr)) {
+			offset += elm_hdr->length;
+			continue;
+		}
+
 		/* Advance to the protocol data, skipping eip bytes if needed */
 		offset += (eip * SES_EIP_HDR_EXTRA_LEN);
-		proto_info_len = elmpriv->addl.hdr->length
+		proto_info_len = elm_hdr->length
 			       - (eip * SES_EIP_HDR_EXTRA_LEN);
 
 		/* Errors in this block are ignored as they are non-fatal */
-		switch(ses_elm_addlstatus_proto(elmpriv->addl.hdr)) {
+		switch(ses_elm_addlstatus_proto(elm_hdr)) {
 		case SPSP_PROTO_FC:
-			if (elmpriv->addl.hdr->length == 0)
+			if (elm_hdr->length == 0)
 				break;
 			ses_get_elm_addlstatus_fc(enc, enc_cache,
 						  &buf[offset], proto_info_len);
 			break;
 		case SPSP_PROTO_SAS:
-			if (elmpriv->addl.hdr->length <= 2)
+			if (elm_hdr->length <= 2)
 				break;
 			ses_get_elm_addlstatus_sas(enc, enc_cache,
 						   &buf[offset],
@@ -1836,7 +1868,7 @@ ses_process_elm_addlstatus(enc_softc_t *enc, struct en
 		default:
 			ENC_VLOG(enc, "Element %d: Unknown Additional Element "
 			    "Protocol 0x%x\n", iter.global_element_index,
-			    ses_elm_addlstatus_proto(elmpriv->addl.hdr));
+			    ses_elm_addlstatus_proto(elm_hdr));
 			break;
 		}