d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix use-after-free introduced in r274843.

Browse Source 
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
This commit is contained in:
Alexander Motin 2014-11-22 09:45:32 +00:00
parent c215ad3aa8
commit 73e2e95d94
1 changed files with 45 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
sys/dev/iscsi/iscsi.c
View File

@ -838,8 +838,9 @@ iscsi_pdu_handle_scsi_response(struct icl_pdu *response)
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 icl_pdu *response)
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 icl_pdu *response)
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 icl_pdu *response)
}
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 @@ iscsi_pdu_handle_scsi_response(struct icl_pdu *response)
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 *response)
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 *response)
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 *response)
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 *response)
//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);
}