rework login and discovery code to avoid strlen beyond end of data Message-id: <1383729402-27559-11-git-send-email-pbonzini@redhat.com> Patchwork-id: 55505 O-Subject: [PATCH 10/11] rework login and discovery code to avoid strlen beyond end of data Bugzilla: 1026820 RH-Acked-by: Miroslav Rezanina RH-Acked-by: Orit Wasserman RH-Acked-by: Stefan Hajnoczi 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 (cherry picked from commit bfde49756524bd3748234dc6dfa8015e37176e9b) Conflicts: lib/login.c --- lib/discovery.c | 31 ++++++++++++++++++++----------- lib/login.c | 32 +++++++++++++++++++++----------- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/lib/discovery.c b/lib/discovery.c index 1ddf8ef..7396e71 100644 --- a/lib/discovery.c +++ b/lib/discovery.c @@ -112,25 +112,34 @@ iscsi_process_text_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, pdu->private_data); 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; - len = strlen((char *)ptr); - - if (len == 0) { - break; - } - - if (len > size) { - iscsi_set_error(iscsi, "len > size when parsing " - "discovery data %d>%d", len, size); + end = memchr(ptr, 0, size); + if (end == NULL) { + iscsi_set_error(iscsi, "NUL not found after offset %td " + "when parsing discovery data", + ptr - in->data); pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, pdu->private_data); iscsi_free_discovery_addresses(iscsi, targets); return -1; } + len = end - ptr; + if (len == 0) { + break; + } + /* parse the strings */ if (!strncmp((char *)ptr, "TargetName=", 11)) { struct iscsi_discovery_address *target; @@ -181,7 +190,7 @@ iscsi_process_text_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, ptr += len + 1; size -= len + 1; - } + } while (size > 0); pdu->callback(iscsi, SCSI_STATUS_GOOD, targets, pdu->private_data); iscsi_free_discovery_addresses(iscsi, targets); diff --git a/lib/login.c b/lib/login.c index 22a7408..8b696b0 100644 --- a/lib/login.c +++ b/lib/login.c @@ -1000,29 +1000,39 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, iscsi->maxcmdsn = maxcmdsn; } + 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 * renegotiated some some parameters. * we should also do proper handshaking if the target is not yet * prepared to transition to the next stage */ - while (size > 0) { + do { + char *end; int len; - len = strlen(ptr); - - if (len == 0) { - break; - } - - if (len > size) { - iscsi_set_error(iscsi, "len > size when parsing " - "login data %d>%d", len, size); + end = memchr(ptr, 0, size); + if (end == NULL) { + iscsi_set_error(iscsi, "NUL not found after offset %td " + "when parsing login data", + (unsigned char *)ptr - in->data); pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, pdu->private_data); return -1; } + len = end - ptr; + if (len == 0) { + break; + } + /* parse the strings */ if (!strncmp(ptr, "TargetAddress=", 14)) { strncpy(iscsi->target_address,ptr+14,MAX_STRING_SIZE); @@ -1095,7 +1105,7 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, ptr += len + 1; size -= len + 1; - } + } while (size > 0); if (status == SCSI_STATUS_REDIRECT && iscsi->target_address[0]) { ISCSI_LOG(iscsi, 2, "target requests redirect to %s",iscsi->target_address);