rework login and discovery code to avoid strlen beyond end of data

Checking for the presence of the NUL character should be done
without accessing beyond the PDU datain.  Use memchr instead
of strlen, and compute the length only if a NUL character is
actually there.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini
2013-11-05 14:23:58 +01:00
parent bb0e59055a
commit bfde497565
2 changed files with 42 additions and 23 deletions
+20 -11
View File
@@ -118,25 +118,34 @@ iscsi_process_text_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
pdu->private_data); pdu->private_data);
return -1; return -1;
} }
if (size == 0) {
iscsi_set_error(iscsi, "size == 0 when parsing "
"discovery data");
pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL,
pdu->private_data);
return -1;
}
while (size > 0) { do {
unsigned char *end;
int len; int len;
len = strlen((char *)ptr); end = memchr(ptr, 0, size);
if (end == NULL) {
if (len == 0) { iscsi_set_error(iscsi, "NUL not found after offset %ld "
break; "when parsing discovery data",
} ptr - in->data);
if (len > size) {
iscsi_set_error(iscsi, "len > size when parsing "
"discovery data %d>%d", len, size);
pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL,
pdu->private_data); pdu->private_data);
iscsi_free_discovery_addresses(iscsi, targets); iscsi_free_discovery_addresses(iscsi, targets);
return -1; return -1;
} }
len = end - ptr;
if (len == 0) {
break;
}
/* parse the strings */ /* parse the strings */
if (!strncmp((char *)ptr, "TargetName=", 11)) { if (!strncmp((char *)ptr, "TargetName=", 11)) {
struct iscsi_discovery_address *target; struct iscsi_discovery_address *target;
@@ -187,7 +196,7 @@ iscsi_process_text_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
ptr += len + 1; ptr += len + 1;
size -= len + 1; size -= len + 1;
} } while (size > 0);
pdu->callback(iscsi, SCSI_STATUS_GOOD, targets, pdu->private_data); pdu->callback(iscsi, SCSI_STATUS_GOOD, targets, pdu->private_data);
iscsi_free_discovery_addresses(iscsi, targets); iscsi_free_discovery_addresses(iscsi, targets);
+22 -12
View File
@@ -981,30 +981,40 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) { if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) {
iscsi->expcmdsn = expcmdsn; iscsi->expcmdsn = expcmdsn;
} }
if (size == 0) {
iscsi_set_error(iscsi, "size == 0 when parsing "
"login data");
pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL,
pdu->private_data);
return -1;
}
/* XXX here we should parse the data returned in case the target /* XXX here we should parse the data returned in case the target
* renegotiated some some parameters. * renegotiated some some parameters.
* we should also do proper handshaking if the target is not yet * we should also do proper handshaking if the target is not yet
* prepared to transition to the next stage * prepared to transition to the next stage
*/ */
while (size > 0) { do {
char *end;
int len; int len;
len = strlen(ptr); end = memchr(ptr, 0, size);
if (end == NULL) {
if (len == 0) { iscsi_set_error(iscsi, "NUL not found after offset %ld "
break; "when parsing login data",
} (unsigned char *)ptr - in->data);
if (len > size) {
iscsi_set_error(iscsi, "len > size when parsing "
"login data %d>%d", len, size);
pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL,
pdu->private_data); pdu->private_data);
return -1; return -1;
} }
len = end - ptr;
if (len == 0) {
break;
}
/* parse the strings */ /* parse the strings */
if (!strncmp(ptr, "TargetAddress=", 14)) { if (!strncmp(ptr, "TargetAddress=", 14)) {
strncpy(iscsi->target_address,ptr+14,MAX_STRING_SIZE); strncpy(iscsi->target_address,ptr+14,MAX_STRING_SIZE);
@@ -1077,7 +1087,7 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu,
ptr += len + 1; ptr += len + 1;
size -= len + 1; size -= len + 1;
} } while (size > 0);
if (status == SCSI_STATUS_REDIRECT && iscsi->target_address[0]) { if (status == SCSI_STATUS_REDIRECT && iscsi->target_address[0]) {
ISCSI_LOG(iscsi, 2, "target requests redirect to %s",iscsi->target_address); ISCSI_LOG(iscsi, 2, "target requests redirect to %s",iscsi->target_address);