From 15021faf19ece8f586382016aed05540f9fe8028 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 25 Oct 2018 20:35:20 +0200 Subject: [PATCH 1/5] lib: properly pass through NOP-In data segment data_pos corresponds to the data_segment_length (+ padding), so should always be passed to the NOP-In callback if greater than zero. Fixes: https://github.com/sahlberg/libiscsi/issues/278 Signed-off-by: David Disseldorp --- lib/nop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/nop.c b/lib/nop.c index b2104cd..b740c7f 100644 --- a/lib/nop.c +++ b/lib/nop.c @@ -163,7 +163,7 @@ iscsi_process_nop_out_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, data.data = NULL; data.size = 0; - if (in->data_pos > ISCSI_HEADER_SIZE) { + if (in->data_pos) { data.data = in->data; data.size = in->data_pos; } From d7530757df4df3fe65915e94545f00a646479e41 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 25 Oct 2018 23:17:21 +0200 Subject: [PATCH 2/5] examples: don't assume NOP-In data is zero terminated Signed-off-by: David Disseldorp --- examples/iscsiclient.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/iscsiclient.c b/examples/iscsiclient.c index 476a925..a61d6e0 100644 --- a/examples/iscsiclient.c +++ b/examples/iscsiclient.c @@ -86,7 +86,8 @@ void nop_out_cb(struct iscsi_context *iscsi, int status, void *command_data, voi printf("NOP-IN status:%d\n", status); if (data->size > 0) { - printf("NOP-IN data:%s\n", data->data); + printf("NOP-IN (%zu) data:%.*s\n", + data->size, (int)data->size, data->data); } printf("Send SYNCHRONIZECACHE10\n"); task = iscsi_synchronizecache10_task(iscsi, 2, 0, 0, 0, 0, synccache10_cb, private_data); From 96dc6e7ebd85ca6867da4814da056b9c366139ce Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 25 Oct 2018 22:41:41 +0200 Subject: [PATCH 3/5] socket: check for malloc failure before dereference Signed-off-by: David Disseldorp --- lib/socket.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/socket.c b/lib/socket.c index 4aa0d9a..f1fca1b 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -601,11 +601,15 @@ iscsi_read_from_socket(struct iscsi_context *iscsi) do { if (iscsi->incoming == NULL) { iscsi->incoming = iscsi_szmalloc(iscsi, sizeof(struct iscsi_in_pdu)); - iscsi->incoming->hdr = iscsi_smalloc(iscsi, ISCSI_HEADER_SIZE); if (iscsi->incoming == NULL) { iscsi_set_error(iscsi, "Out-of-memory: failed to malloc iscsi_in_pdu"); return -1; } + iscsi->incoming->hdr = iscsi_smalloc(iscsi, ISCSI_HEADER_SIZE); + if (iscsi->incoming->hdr == NULL) { + iscsi_set_error(iscsi, "Out-of-memory"); + return -1; + } } in = iscsi->incoming; From 009892b017bab10c61cfdc30ea45641dc79e7b4c Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 25 Oct 2018 22:56:10 +0200 Subject: [PATCH 4/5] socket: calculate hdr_size once per PDU process loop ISCSI_HEADER_SIZE is determined based on the iscsi->header_digest setting, which may change via iscsi_process_pdu(). Signed-off-by: David Disseldorp --- lib/socket.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/socket.c b/lib/socket.c index f1fca1b..dd6dbb6 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -596,16 +596,17 @@ static int iscsi_read_from_socket(struct iscsi_context *iscsi) { struct iscsi_in_pdu *in; - ssize_t data_size, count, padding_size; + ssize_t hdr_size, data_size, count, padding_size; do { + hdr_size = ISCSI_HEADER_SIZE; if (iscsi->incoming == NULL) { iscsi->incoming = iscsi_szmalloc(iscsi, sizeof(struct iscsi_in_pdu)); if (iscsi->incoming == NULL) { iscsi_set_error(iscsi, "Out-of-memory: failed to malloc iscsi_in_pdu"); return -1; } - iscsi->incoming->hdr = iscsi_smalloc(iscsi, ISCSI_HEADER_SIZE); + iscsi->incoming->hdr = iscsi_smalloc(iscsi, hdr_size); if (iscsi->incoming->hdr == NULL) { iscsi_set_error(iscsi, "Out-of-memory"); return -1; @@ -614,11 +615,11 @@ iscsi_read_from_socket(struct iscsi_context *iscsi) in = iscsi->incoming; /* first we must read the header, including any digests */ - if (in->hdr_pos < ISCSI_HEADER_SIZE) { + if (in->hdr_pos < hdr_size) { /* try to only read the header, the socket is nonblocking, so * no need to limit the read to what is available in the socket */ - count = ISCSI_HEADER_SIZE - in->hdr_pos; + count = hdr_size - in->hdr_pos; count = recv(iscsi->fd, &in->hdr[in->hdr_pos], count, 0); if (count == 0) { /* remote side has closed the socket. */ @@ -635,7 +636,7 @@ iscsi_read_from_socket(struct iscsi_context *iscsi) in->hdr_pos += count; } - if (in->hdr_pos < ISCSI_HEADER_SIZE) { + if (in->hdr_pos < hdr_size) { /* we don't have the full header yet, so return */ break; } From 9d31150e9d01f7a91f55a0c7221245551a9b9f6b Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 25 Oct 2018 23:19:48 +0200 Subject: [PATCH 5/5] socket: improve ISCSI_HEADER_SIZE readability The ISCSI_HEADER_SIZE macro accesses iscsi->header_size, so pass it in as a parameter to make it easier to follow callers. Signed-off-by: David Disseldorp --- include/iscsi-private.h | 4 ++-- lib/pdu.c | 4 ++-- lib/socket.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/iscsi-private.h b/include/iscsi-private.h index a96caae..8ace255 100644 --- a/include/iscsi-private.h +++ b/include/iscsi-private.h @@ -43,8 +43,8 @@ extern "C" { #define ISCSI_RAW_HEADER_SIZE 48 #define ISCSI_DIGEST_SIZE 4 -#define ISCSI_HEADER_SIZE (ISCSI_RAW_HEADER_SIZE \ - + (iscsi->header_digest == ISCSI_HEADER_DIGEST_NONE?0:ISCSI_DIGEST_SIZE)) +#define ISCSI_HEADER_SIZE(hdr_digest) (ISCSI_RAW_HEADER_SIZE \ + + (hdr_digest == ISCSI_HEADER_DIGEST_NONE?0:ISCSI_DIGEST_SIZE)) #define SMALL_ALLOC_MAX_FREE (128) /* must be power of 2 */ diff --git a/lib/pdu.c b/lib/pdu.c index 441efa7..043aa75 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -111,7 +111,7 @@ iscsi_allocate_pdu(struct iscsi_context *iscsi, enum iscsi_opcode opcode, return NULL; } - pdu->outdata.size = ISCSI_HEADER_SIZE; + pdu->outdata.size = ISCSI_HEADER_SIZE(iscsi->header_digest); pdu->outdata.data = iscsi_szmalloc(iscsi, pdu->outdata.size); if (pdu->outdata.data == NULL) { @@ -236,7 +236,7 @@ iscsi_pdu_add_data(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, /* update data segment length */ scsi_set_uint32(&pdu->outdata.data[4], pdu->outdata.size - - ISCSI_HEADER_SIZE); + - ISCSI_HEADER_SIZE(iscsi->header_digest)); return 0; } diff --git a/lib/socket.c b/lib/socket.c index dd6dbb6..42db72b 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -599,7 +599,7 @@ iscsi_read_from_socket(struct iscsi_context *iscsi) ssize_t hdr_size, data_size, count, padding_size; do { - hdr_size = ISCSI_HEADER_SIZE; + hdr_size = ISCSI_HEADER_SIZE(iscsi->header_digest); if (iscsi->incoming == NULL) { iscsi->incoming = iscsi_szmalloc(iscsi, sizeof(struct iscsi_in_pdu)); if (iscsi->incoming == NULL) {