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);
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to