From cd09c0f17dc0907fd27bdc3cf8f9a53121ca1b0b Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 28 Nov 2012 10:37:28 +0100 Subject: [PATCH] PDU use serial32 arithmetic for cmdsn, maxcmdsn and expcmdsn. RFC3720 says that cmdsn comparison must be done using serial32 arithmetic. This will definetly avoid a deadlock if cmdsn wraps from 2^32-1 to 0. Signed-off-by: Peter Lieven --- include/iscsi-private.h | 3 +++ lib/iscsi-command.c | 6 +++--- lib/login.c | 6 +++--- lib/pdu.c | 18 ++++++++++++++++++ lib/socket.c | 4 ++-- lib/task_mgmt.c | 2 +- 6 files changed, 30 insertions(+), 9 deletions(-) diff --git a/include/iscsi-private.h b/include/iscsi-private.h index 93335f4..2928a36 100644 --- a/include/iscsi-private.h +++ b/include/iscsi-private.h @@ -325,6 +325,9 @@ iscsi_log_message(struct iscsi_context *iscsi, int level, const char *format, .. void iscsi_add_to_outqueue(struct iscsi_context *iscsi, struct iscsi_pdu *pdu); +int +iscsi_serial32_compare(u_int32_t s1, u_int32_t s2); + #ifdef __cplusplus } #endif diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index 310e656..c6b6fe4 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -335,7 +335,7 @@ iscsi_process_scsi_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, } maxcmdsn = scsi_get_uint32(&in->hdr[32]); - if (maxcmdsn > iscsi->maxcmdsn) { + if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) { iscsi->maxcmdsn = maxcmdsn; } @@ -452,7 +452,7 @@ iscsi_process_scsi_data_in(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, } maxcmdsn = scsi_get_uint32(&in->hdr[32]); - if (maxcmdsn > iscsi->maxcmdsn) { + if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) { iscsi->maxcmdsn = maxcmdsn; } @@ -542,7 +542,7 @@ iscsi_process_r2t(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, len = scsi_get_uint32(&in->hdr[44]); maxcmdsn = scsi_get_uint32(&in->hdr[32]); - if (maxcmdsn > iscsi->maxcmdsn) { + if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) { iscsi->maxcmdsn = maxcmdsn; } diff --git a/lib/login.c b/lib/login.c index bf85b73..c918747 100644 --- a/lib/login.c +++ b/lib/login.c @@ -950,10 +950,10 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, iscsi->statsn = scsi_get_uint16(&in->hdr[24]); maxcmdsn = scsi_get_uint32(&in->hdr[32]); - if (maxcmdsn > iscsi->maxcmdsn) { + if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) { iscsi->maxcmdsn = maxcmdsn; } - + /* 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 @@ -1136,7 +1136,7 @@ struct iscsi_in_pdu *in) uint32_t maxcmdsn; maxcmdsn = scsi_get_uint32(&in->hdr[32]); - if (maxcmdsn > iscsi->maxcmdsn) { + if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) { iscsi->maxcmdsn = maxcmdsn; } diff --git a/lib/pdu.c b/lib/pdu.c index 947c0fc..c28cd4f 100644 --- a/lib/pdu.c +++ b/lib/pdu.c @@ -30,6 +30,24 @@ #include "scsi-lowlevel.h" #include "slist.h" +/* This adds 32-bit serial comparision as defined in RFC1982. + * It returns 0 for equality, 1 if s1 is greater than s2 and + * -1 if s1 is less than s2. According to RFC1982 section 3.2 + * there are rare cases where the result of the comparision is + * undefined e.g. when s1 = 0 and s2=2^31. This cases should + * not happen in iSCSI protocol. + */ +int +iscsi_serial32_compare(u_int32_t s1, u_int32_t s2) { + if (s1 == s2) return 0; + if (s1 < s2 && s2-s1 < (u_int32_t)1<<31) return -1; + if (s1 > s2 && s1-s2 < (u_int32_t)1<<31) return 1; + if (s1 > s2 && s1-s2 > (u_int32_t)1<<31) return -1; + if (s1 < s2 && s2-s1 > (u_int32_t)1<<31) return 1; + /* undefined result */ + return -1; +} + struct iscsi_pdu * iscsi_allocate_pdu_with_itt_flags_size(struct iscsi_context *iscsi, enum iscsi_opcode opcode, enum iscsi_opcode response_opcode, uint32_t itt, uint32_t flags, size_t payload_size) diff --git a/lib/socket.c b/lib/socket.c index e1a417a..c14e590 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -302,7 +302,7 @@ iscsi_which_events(struct iscsi_context *iscsi) { int events = iscsi->is_connected ? POLLIN : POLLOUT; - if (iscsi->outqueue && iscsi->outqueue->cmdsn <= iscsi->maxcmdsn) { + if (iscsi->outqueue && iscsi_serial32_compare(iscsi->outqueue->cmdsn,iscsi->maxcmdsn) <= 0) { events |= POLLOUT; } return events; @@ -442,7 +442,7 @@ iscsi_write_to_socket(struct iscsi_context *iscsi) while (iscsi->outqueue) { ssize_t total; - if (iscsi->outqueue->cmdsn > iscsi->maxcmdsn) { + if (iscsi_serial32_compare(iscsi->outqueue->cmdsn,iscsi->maxcmdsn) > 0) { /* stop sending. maxcmdsn is reached */ return 0; } diff --git a/lib/task_mgmt.c b/lib/task_mgmt.c index 9585bd1..784fe6c 100644 --- a/lib/task_mgmt.c +++ b/lib/task_mgmt.c @@ -88,7 +88,7 @@ iscsi_process_task_mgmt_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu response = in->hdr[2]; maxcmdsn = scsi_get_uint32(&in->hdr[32]); - if (maxcmdsn > iscsi->maxcmdsn) { + if (iscsi_serial32_compare(maxcmdsn,iscsi->maxcmdsn) > 0) { iscsi->maxcmdsn = maxcmdsn; }