svn commit: r274845 - head/sys/dev/iscsi

Alexander Motin mav at FreeBSD.org
Sat Nov 22 09:45:33 UTC 2014


Author: mav
Date: Sat Nov 22 09:45:32 2014
New Revision: 274845
URL: https://svnweb.freebsd.org/changeset/base/274845

Log:
  Fix use-after-free introduced in r274843.
  
  I've missed that iscsi_outstanding_remove() frees the second pointer,
  so it should no longer be used.  And in fact we don't really need to.
  
  MFC after:	2 weeks

Modified:
  head/sys/dev/iscsi/iscsi.c

Modified: head/sys/dev/iscsi/iscsi.c
==============================================================================
--- head/sys/dev/iscsi/iscsi.c	Sat Nov 22 09:38:18 2014	(r274844)
+++ head/sys/dev/iscsi/iscsi.c	Sat Nov 22 09:45:32 2014	(r274845)
@@ -838,8 +838,9 @@ iscsi_pdu_handle_scsi_response(struct ic
 	struct iscsi_bhs_scsi_response *bhssr;
 	struct iscsi_outstanding *io;
 	struct iscsi_session *is;
+	union ccb *ccb;
 	struct ccb_scsiio *csio;
-	size_t data_segment_len;
+	size_t data_segment_len, received;
 	uint16_t sense_len;
 
 	is = PDU_SESSION(response);
@@ -854,38 +855,40 @@ iscsi_pdu_handle_scsi_response(struct ic
 		return;
 	}
 
+	ccb = io->io_ccb;
+	received = io->io_received;
 	iscsi_outstanding_remove(is, io);
 	ISCSI_SESSION_UNLOCK(is);
 
 	if (bhssr->bhssr_response != BHSSR_RESPONSE_COMMAND_COMPLETED) {
 		ISCSI_SESSION_WARN(is, "service response 0x%x", bhssr->bhssr_response);
- 		if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
- 			xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+ 		if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+ 			xpt_freeze_devq(ccb->ccb_h.path, 1);
 			ISCSI_SESSION_DEBUG(is, "freezing devq");
 		}
- 		io->io_ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
+ 		ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
 	} else if (bhssr->bhssr_status == 0) {
-		io->io_ccb->ccb_h.status = CAM_REQ_CMP;
+		ccb->ccb_h.status = CAM_REQ_CMP;
 	} else {
- 		if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
- 			xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+ 		if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+ 			xpt_freeze_devq(ccb->ccb_h.path, 1);
 			ISCSI_SESSION_DEBUG(is, "freezing devq");
 		}
- 		io->io_ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR | CAM_DEV_QFRZN;
-		io->io_ccb->csio.scsi_status = bhssr->bhssr_status;
+ 		ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR | CAM_DEV_QFRZN;
+		ccb->csio.scsi_status = bhssr->bhssr_status;
 	}
 
-	csio = &io->io_ccb->csio;
+	csio = &ccb->csio;
 	data_segment_len = icl_pdu_data_segment_length(response);
 	if (data_segment_len > 0) {
 		if (data_segment_len < sizeof(sense_len)) {
 			ISCSI_SESSION_WARN(is, "truncated data segment (%zd bytes)",
 			    data_segment_len);
-			if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
-				xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+			if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+				xpt_freeze_devq(ccb->ccb_h.path, 1);
 				ISCSI_SESSION_DEBUG(is, "freezing devq");
 			}
-			io->io_ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
+			ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
 			goto out;
 		}
 		icl_pdu_get_data(response, 0, &sense_len, sizeof(sense_len));
@@ -898,11 +901,11 @@ iscsi_pdu_handle_scsi_response(struct ic
 			ISCSI_SESSION_WARN(is, "truncated data segment "
 			    "(%zd bytes, should be %zd)",
 			    data_segment_len, sizeof(sense_len) + sense_len);
-			if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
-				xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+			if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+				xpt_freeze_devq(ccb->ccb_h.path, 1);
 				ISCSI_SESSION_DEBUG(is, "freezing devq");
 			}
-			io->io_ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
+			ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
 			goto out;
 		} else if (sizeof(sense_len) + sense_len < data_segment_len)
 			ISCSI_SESSION_WARN(is, "oversize data segment "
@@ -915,7 +918,7 @@ iscsi_pdu_handle_scsi_response(struct ic
 		}
 		icl_pdu_get_data(response, sizeof(sense_len), &csio->sense_data, sense_len);
 		csio->sense_resid = csio->sense_len - sense_len;
-		io->io_ccb->ccb_h.status |= CAM_AUTOSNS_VALID;
+		ccb->ccb_h.status |= CAM_AUTOSNS_VALID;
 	}
 
 out:
@@ -923,20 +926,19 @@ out:
 		csio->resid = ntohl(bhssr->bhssr_residual_count);
 
 	if ((csio->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_IN) {
-		KASSERT(io->io_received <= csio->dxfer_len,
-		    ("io->io_received > csio->dxfer_len"));
-		if (io->io_received < csio->dxfer_len) {
-			if (csio->resid != csio->dxfer_len - io->io_received) {
+		KASSERT(received <= csio->dxfer_len,
+		    ("received > csio->dxfer_len"));
+		if (received < csio->dxfer_len) {
+			if (csio->resid != csio->dxfer_len - received) {
 				ISCSI_SESSION_WARN(is, "underflow mismatch: "
 				    "target indicates %d, we calculated %zd",
-				    csio->resid,
-				    csio->dxfer_len - io->io_received);
+				    csio->resid, csio->dxfer_len - received);
 			}
-			csio->resid = csio->dxfer_len - io->io_received;
+			csio->resid = csio->dxfer_len - received;
 		}
 	}
 
-	xpt_done(io->io_ccb);
+	xpt_done(ccb);
 	icl_pdu_free(response);
 }
 
@@ -978,8 +980,9 @@ iscsi_pdu_handle_data_in(struct icl_pdu 
 	struct iscsi_bhs_data_in *bhsdi;
 	struct iscsi_outstanding *io;
 	struct iscsi_session *is;
+	union ccb *ccb;
 	struct ccb_scsiio *csio;
-	size_t data_segment_len, received;
+	size_t data_segment_len, received, oreceived;
 	
 	is = PDU_SESSION(response);
 	bhsdi = (struct iscsi_bhs_data_in *)response->ip_bhs;
@@ -1018,7 +1021,8 @@ iscsi_pdu_handle_data_in(struct icl_pdu 
 		return;
 	}
 
-	csio = &io->io_ccb->csio;
+	ccb = io->io_ccb;
+	csio = &ccb->csio;
 
 	if (io->io_received + data_segment_len > csio->dxfer_len) {
 		ISCSI_SESSION_WARN(is, "oversize data segment (%zd bytes "
@@ -1030,13 +1034,14 @@ iscsi_pdu_handle_data_in(struct icl_pdu 
 		return;
 	}
 
-	received = io->io_received;
+	oreceived = io->io_received;
 	io->io_received += data_segment_len;
+	received = io->io_received;
 	if ((bhsdi->bhsdi_flags & BHSDI_FLAGS_S) != 0)
 		iscsi_outstanding_remove(is, io);
 	ISCSI_SESSION_UNLOCK(is);
 
-	icl_pdu_get_data(response, 0, csio->data_ptr + received, data_segment_len);
+	icl_pdu_get_data(response, 0, csio->data_ptr + oreceived, data_segment_len);
 
 	/*
 	 * XXX: Check DataSN.
@@ -1052,32 +1057,31 @@ iscsi_pdu_handle_data_in(struct icl_pdu 
 
 	//ISCSI_SESSION_DEBUG(is, "got S flag; status 0x%x", bhsdi->bhsdi_status);
 	if (bhsdi->bhsdi_status == 0) {
-		io->io_ccb->ccb_h.status = CAM_REQ_CMP;
+		ccb->ccb_h.status = CAM_REQ_CMP;
 	} else {
-		if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
-			xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+		if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+			xpt_freeze_devq(ccb->ccb_h.path, 1);
 			ISCSI_SESSION_DEBUG(is, "freezing devq");
 		}
-		io->io_ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR | CAM_DEV_QFRZN;
+		ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR | CAM_DEV_QFRZN;
 		csio->scsi_status = bhsdi->bhsdi_status;
 	}
 
 	if ((csio->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_IN) {
-		KASSERT(io->io_received <= csio->dxfer_len,
-		    ("io->io_received > csio->dxfer_len"));
-		if (io->io_received < csio->dxfer_len) {
+		KASSERT(received <= csio->dxfer_len,
+		    ("received > csio->dxfer_len"));
+		if (received < csio->dxfer_len) {
 			csio->resid = ntohl(bhsdi->bhsdi_residual_count);
-			if (csio->resid != csio->dxfer_len - io->io_received) {
+			if (csio->resid != csio->dxfer_len - received) {
 				ISCSI_SESSION_WARN(is, "underflow mismatch: "
 				    "target indicates %d, we calculated %zd",
-				    csio->resid,
-				    csio->dxfer_len - io->io_received);
+				    csio->resid, csio->dxfer_len - received);
 			}
-			csio->resid = csio->dxfer_len - io->io_received;
+			csio->resid = csio->dxfer_len - received;
 		}
 	}
 
-	xpt_done(io->io_ccb);
+	xpt_done(ccb);
 	icl_pdu_free(response);
 }
 


More information about the svn-src-head mailing list