Date: Sat, 22 Nov 2014 09:45:32 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r274845 - head/sys/dev/iscsi Message-ID: <201411220945.sAM9jWh7059986@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201411220945.sAM9jWh7059986>