From 83ac22abbb3d08b03f5ae21e04cf3495065ca766 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Sat, 27 Oct 2012 16:18:50 +0200 Subject: [PATCH 01/13] Fix memleaks in iscsi-ls, iscsi-inq & iscsi-readcapacity16 --- src/iscsi-inq.c | 5 ++++- src/iscsi-ls.c | 11 +++++++---- src/iscsi-readcapacity16.c | 5 ++++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/iscsi-inq.c b/src/iscsi-inq.c index 87b6d35..031e4e3 100644 --- a/src/iscsi-inq.c +++ b/src/iscsi-inq.c @@ -216,7 +216,7 @@ int main(int argc, const char *argv[]) struct iscsi_context *iscsi; const char **extra_argv; int extra_argc = 0; - const char *url = NULL; + char *url = NULL; struct iscsi_url *iscsi_url = NULL; int evpd = 0, pagecode = 0; int show_help = 0, show_usage = 0, debug = 0; @@ -275,6 +275,9 @@ int main(int argc, const char *argv[]) exit(10); } iscsi_url = iscsi_parse_full_url(iscsi, url); + + if (url) free(url); + if (iscsi_url == NULL) { fprintf(stderr, "Failed to parse URL: %s\n", iscsi_get_error(iscsi)); diff --git a/src/iscsi-ls.c b/src/iscsi-ls.c index 6951a57..b8c4b7c 100644 --- a/src/iscsi-ls.c +++ b/src/iscsi-ls.c @@ -312,7 +312,7 @@ int main(int argc, const char *argv[]) struct client_state state; const char **extra_argv; int extra_argc = 0; - const char *url = NULL; + char *url = NULL; poptContext pc; int res; int show_help = 0, show_usage = 0, debug = 0; @@ -367,11 +367,14 @@ int main(int argc, const char *argv[]) exit(10); } - if (debug > 0) { - iscsi_set_debug(iscsi, debug); - } + if (debug > 0) { + iscsi_set_debug(iscsi, debug); + } iscsi_url = iscsi_parse_portal_url(iscsi, url); + + if (url) free(url); + if (iscsi_url == NULL) { fprintf(stderr, "Failed to parse URL: %s\n", iscsi_get_error(iscsi)); diff --git a/src/iscsi-readcapacity16.c b/src/iscsi-readcapacity16.c index 5f76de0..bbbc38f 100644 --- a/src/iscsi-readcapacity16.c +++ b/src/iscsi-readcapacity16.c @@ -58,7 +58,7 @@ int main(int argc, const char *argv[]) struct iscsi_context *iscsi; const char **extra_argv; int extra_argc = 0; - const char *url = NULL; + char *url = NULL; struct iscsi_url *iscsi_url = NULL; int show_help = 0, show_usage = 0, debug = 0, size_only=0; int res; @@ -117,6 +117,9 @@ int main(int argc, const char *argv[]) exit(10); } iscsi_url = iscsi_parse_full_url(iscsi, url); + + if (url) free(url); + if (iscsi_url == NULL) { fprintf(stderr, "Failed to parse URL: %s\n", iscsi_get_error(iscsi)); From 0906109d8a091a5ad1102ab05f587592a6cbfcd3 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Sat, 27 Oct 2012 16:31:56 +0200 Subject: [PATCH 02/13] CONNECT fix mem leak of connection_task object --- lib/connect.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/connect.c b/lib/connect.c index 7550162..96993f1 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -63,6 +63,7 @@ iscsi_testunitready_cb(struct iscsi_context *iscsi, int status, ct->private_data); } scsi_free_scsi_task(task); + free(ct); return; } } @@ -79,6 +80,7 @@ iscsi_testunitready_cb(struct iscsi_context *iscsi, int status, ct->cb(iscsi, status?SCSI_STATUS_ERROR:SCSI_STATUS_GOOD, NULL, ct->private_data); scsi_free_scsi_task(task); + free(ct); } static void @@ -90,6 +92,7 @@ iscsi_login_cb(struct iscsi_context *iscsi, int status, void *command_data _U_, if (status == SCSI_STATUS_REDIRECT && iscsi->target_address) { iscsi_disconnect(iscsi); if (iscsi_connect_async(iscsi, iscsi->target_address, iscsi_connect_cb, iscsi->connect_data) != 0) { + free(ct); return; } return; @@ -97,6 +100,7 @@ iscsi_login_cb(struct iscsi_context *iscsi, int status, void *command_data _U_, if (status != 0) { ct->cb(iscsi, SCSI_STATUS_ERROR, NULL, ct->private_data); + free(ct); return; } @@ -117,12 +121,14 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data _U_ iscsi_set_error(iscsi, "Failed to connect to iSCSI socket. " "%s", iscsi_get_error(iscsi)); ct->cb(iscsi, SCSI_STATUS_ERROR, NULL, ct->private_data); + free(ct); return; } if (iscsi_login_async(iscsi, iscsi_login_cb, ct) != 0) { iscsi_set_error(iscsi, "iscsi_login_async failed."); ct->cb(iscsi, SCSI_STATUS_ERROR, NULL, ct->private_data); + free(ct); } } From 923b9a4fb2ea0821d6ef9e00cf8f922914257764 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Sat, 27 Oct 2012 17:23:40 +0200 Subject: [PATCH 03/13] ISCSI-CONTEXT change dynamic string allocations to statics --- include/iscsi-private.h | 26 +++++++------ include/iscsi.h | 2 +- lib/connect.c | 22 ++--------- lib/init.c | 84 +++++------------------------------------ lib/login.c | 24 +++--------- lib/socket.c | 5 +-- 6 files changed, 35 insertions(+), 128 deletions(-) diff --git a/include/iscsi-private.h b/include/iscsi-private.h index f523b6e..490876a 100644 --- a/include/iscsi-private.h +++ b/include/iscsi-private.h @@ -61,15 +61,21 @@ enum iscsi_immediate_data { ISCSI_IMMEDIATE_DATA_YES = 1 }; -struct iscsi_context { - const char *initiator_name; - const char *target_name; - const char *target_address; /* If a redirect */ - const char *connected_portal; - const char *alias; +#define MAX_STRING_SIZE (255) - const char *user; - const char *passwd; +struct iscsi_context { + char initiator_name[MAX_STRING_SIZE+1]; + char target_name[MAX_STRING_SIZE+1]; + char target_address[MAX_STRING_SIZE+1]; /* If a redirect */ + char connected_portal[MAX_STRING_SIZE+1]; + char portal[MAX_STRING_SIZE+1]; + char alias[MAX_STRING_SIZE+1]; + + char user[MAX_STRING_SIZE+1]; + char passwd[MAX_STRING_SIZE+1]; + char chap_c[MAX_STRING_SIZE+1]; + + char error_string[MAX_STRING_SIZE+1]; enum iscsi_session_type session_type; unsigned char isid[6]; @@ -80,8 +86,6 @@ struct iscsi_context { enum iscsi_header_digest want_header_digest; enum iscsi_header_digest header_digest; - char *error_string; - int fd; int is_connected; @@ -103,7 +107,6 @@ struct iscsi_context { int chap_a; int chap_i; - char *chap_c; iscsi_command_cb socket_status_cb; void *connect_data; @@ -124,7 +127,6 @@ struct iscsi_context { enum iscsi_immediate_data use_immediate_data; int lun; - const char *portal; int no_auto_reconnect; int reconnect_deferred; int debug; diff --git a/include/iscsi.h b/include/iscsi.h index 8453e5d..bdba224 100644 --- a/include/iscsi.h +++ b/include/iscsi.h @@ -971,7 +971,7 @@ iscsi_scsi_cancel_all_tasks(struct iscsi_context *iscsi); if ((iscsi)->debug >= level) { \ fprintf(stderr,"libiscsi: "); \ fprintf(stderr, (fmt), ##args); \ - if (iscsi->target_name) { \ + if (iscsi->target_name[0]) { \ fprintf(stderr," [%s]",iscsi->target_name); \ } \ fprintf(stderr,"\n"); \ diff --git a/lib/connect.c b/lib/connect.c index 96993f1..dc70565 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -89,7 +89,7 @@ iscsi_login_cb(struct iscsi_context *iscsi, int status, void *command_data _U_, { struct connect_task *ct = private_data; - if (status == SCSI_STATUS_REDIRECT && iscsi->target_address) { + if (status == SCSI_STATUS_REDIRECT && iscsi->target_address[0]) { iscsi_disconnect(iscsi); if (iscsi_connect_async(iscsi, iscsi->target_address, iscsi_connect_cb, iscsi->connect_data) != 0) { free(ct); @@ -140,7 +140,7 @@ iscsi_full_connect_async(struct iscsi_context *iscsi, const char *portal, struct connect_task *ct; iscsi->lun = lun; - iscsi->portal = strdup(portal); + strncpy(iscsi->portal,portal,MAX_STRING_SIZE); ct = malloc(sizeof(struct connect_task)); if (ct == NULL) { @@ -227,7 +227,7 @@ try_again: iscsi_set_header_digest(iscsi, old_iscsi->want_header_digest); - if (old_iscsi->user != NULL) { + if (old_iscsi->user[0]) { iscsi_set_initiator_username_pwd(iscsi, old_iscsi->user, old_iscsi->passwd); } @@ -235,7 +235,7 @@ try_again: iscsi->lun = old_iscsi->lun; - iscsi->portal = strdup(old_iscsi->portal); + strncpy(iscsi->portal,old_iscsi->portal,MAX_STRING_SIZE); iscsi->debug = old_iscsi->debug; @@ -317,26 +317,12 @@ try_again: goto try_again; } - - free(discard_const(old_iscsi->initiator_name)); - free(discard_const(old_iscsi->target_name)); - free(discard_const(old_iscsi->target_address)); - free(discard_const(old_iscsi->alias)); - free(discard_const(old_iscsi->portal)); if (old_iscsi->incoming != NULL) { iscsi_free_iscsi_in_pdu(old_iscsi->incoming); } if (old_iscsi->inqueue != NULL) { iscsi_free_iscsi_inqueue(old_iscsi->inqueue); } - free(old_iscsi->error_string); - free(discard_const(old_iscsi->user)); - free(discard_const(old_iscsi->passwd)); - free(discard_const(old_iscsi->chap_c)); - - if (old_iscsi->connected_portal != NULL) { - free(discard_const(old_iscsi->connected_portal)); - } close(iscsi->fd); iscsi->fd = old_iscsi->fd; diff --git a/lib/init.c b/lib/init.c index 5bd6d0c..c62bbae 100644 --- a/lib/init.c +++ b/lib/init.c @@ -45,8 +45,8 @@ iscsi_create_context(const char *initiator_name) memset(iscsi, 0, sizeof(struct iscsi_context)); - iscsi->initiator_name = strdup(initiator_name); - if (iscsi->initiator_name == NULL) { + strncpy(iscsi->initiator_name,initiator_name,MAX_STRING_SIZE); + if (!iscsi->initiator_name[0]) { free(iscsi); return NULL; } @@ -169,14 +169,7 @@ iscsi_set_alias(struct iscsi_context *iscsi, const char *alias) return -1; } - free(discard_const(iscsi->alias)); - - iscsi->alias = strdup(alias); - if (iscsi->alias == NULL) { - iscsi_set_error(iscsi, "Failed to allocate alias name"); - return -1; - } - + strncpy(iscsi->alias,alias,MAX_STRING_SIZE); return 0; } @@ -189,13 +182,7 @@ iscsi_set_targetname(struct iscsi_context *iscsi, const char *target_name) return -1; } - free(discard_const(iscsi->target_name)); - - iscsi->target_name = strdup(target_name); - if (iscsi->target_name == NULL) { - iscsi_set_error(iscsi, "Failed to allocate target name"); - return -1; - } + strncpy(iscsi->target_name,target_name,MAX_STRING_SIZE); return 0; } @@ -238,21 +225,6 @@ iscsi_destroy_context(struct iscsi_context *iscsi) iscsi_free_pdu(iscsi, pdu); } - free(discard_const(iscsi->initiator_name)); - iscsi->initiator_name = NULL; - - free(discard_const(iscsi->target_name)); - iscsi->target_name = NULL; - - free(discard_const(iscsi->target_address)); - iscsi->target_address = NULL; - - free(discard_const(iscsi->alias)); - iscsi->alias = NULL; - - free(discard_const(iscsi->portal)); - iscsi->portal = NULL; - if (iscsi->incoming != NULL) { iscsi_free_iscsi_in_pdu(iscsi->incoming); } @@ -260,23 +232,6 @@ iscsi_destroy_context(struct iscsi_context *iscsi) iscsi_free_iscsi_inqueue(iscsi->inqueue); } - free(iscsi->error_string); - iscsi->error_string = NULL; - - free(discard_const(iscsi->user)); - iscsi->user = NULL; - - free(discard_const(iscsi->passwd)); - iscsi->passwd = NULL; - - free(discard_const(iscsi->chap_c)); - iscsi->chap_c = NULL; - - if (iscsi->connected_portal != NULL) { - free(discard_const(iscsi->connected_portal)); - iscsi->connected_portal = NULL; - } - iscsi->connect_data = NULL; free(iscsi); @@ -288,23 +243,14 @@ void iscsi_set_error(struct iscsi_context *iscsi, const char *error_string, ...) { va_list ap; - char *str; va_start(ap, error_string); - str = malloc(1024); - if (vsnprintf(str, 1024, error_string, ap) < 0) { - /* not much we can do here */ - free(str); - str = NULL; + if (vsnprintf(iscsi->error_string, MAX_STRING_SIZE, error_string, ap) < 0) { + strncpy(iscsi->error_string,"could not format error string!",MAX_STRING_SIZE); } - - free(iscsi->error_string); - - iscsi->error_string = str; - va_end(ap); - DPRINTF(iscsi,1,"%s",str); + DPRINTF(iscsi,1,"%s",iscsi->error_string); } void @@ -589,19 +535,7 @@ int iscsi_set_initiator_username_pwd(struct iscsi_context *iscsi, const char *user, const char *passwd) { - free(discard_const(iscsi->user)); - iscsi->user = strdup(user); - if (iscsi->user == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: failed to strdup username"); - return -1; - } - - free(discard_const(iscsi->passwd)); - iscsi->passwd = strdup(passwd); - if (iscsi->passwd == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: failed to strdup password"); - return -1; - } - + strncpy(iscsi->user,user,MAX_STRING_SIZE); + strncpy(iscsi->passwd,passwd,MAX_STRING_SIZE); return 0; } diff --git a/lib/login.c b/lib/login.c index 8e57839..5da4d21 100644 --- a/lib/login.c +++ b/lib/login.c @@ -106,7 +106,7 @@ iscsi_login_add_targetname(struct iscsi_context *iscsi, struct iscsi_pdu *pdu) return 0; } - if (iscsi->target_name == NULL) { + if (!iscsi->target_name[0]) { iscsi_set_error(iscsi, "Trying normal connect but " "target name not set."); return -1; @@ -663,7 +663,7 @@ iscsi_login_add_chap_response(struct iscsi_context *iscsi, struct iscsi_pdu *pdu return 0; } - if (iscsi->chap_c == NULL) { + if (!iscsi->chap_c[0]) { iscsi_set_error(iscsi, "No CHAP challenge found"); return -1; } @@ -748,7 +748,7 @@ iscsi_login_async(struct iscsi_context *iscsi, iscsi_command_cb cb, /* login request */ iscsi_pdu_set_immediate(pdu); - if (iscsi->user == NULL) { + if (!iscsi->user[0]) { iscsi->current_phase = ISCSI_PDU_LOGIN_CSG_OPNEG; } @@ -1008,15 +1008,7 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, /* parse the strings */ if (!strncmp(ptr, "TargetAddress=", 14)) { - free(discard_const(iscsi->target_address)); - iscsi->target_address = strdup(ptr+14); - if (iscsi->target_address == NULL) { - iscsi_set_error(iscsi, "Failed to allocate" - " target address"); - pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, - pdu->private_data); - return -1; - } + strncpy(iscsi->target_address,ptr+14,MAX_STRING_SIZE); } if (!strncmp(ptr, "HeaderDigest=", 13)) { @@ -1074,13 +1066,7 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, } if (!strncmp(ptr, "CHAP_C=0x", 9)) { - free(iscsi->chap_c); - iscsi->chap_c = strdup(ptr+9); - if (iscsi->chap_c == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup CHAP challenge."); - pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, pdu->private_data); - return -1; - } + strncpy(iscsi->chap_c,ptr+9,MAX_STRING_SIZE); iscsi->secneg_phase = ISCSI_LOGIN_SECNEG_PHASE_SEND_RESPONSE; } diff --git a/lib/socket.c b/lib/socket.c index 1376527..ce838cc 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -228,8 +228,7 @@ iscsi_connect_async(struct iscsi_context *iscsi, const char *portal, freeaddrinfo(ai); - if (iscsi->connected_portal) free(discard_const(iscsi->connected_portal)); - iscsi->connected_portal=strdup(portal); + strncpy(iscsi->connected_portal,portal,MAX_STRING_SIZE); return 0; } @@ -245,7 +244,7 @@ iscsi_disconnect(struct iscsi_context *iscsi) close(iscsi->fd); - if (iscsi->connected_portal) + if (iscsi->connected_portal[0]) DPRINTF(iscsi,2,"disconnected from portal %s",iscsi->connected_portal); iscsi->fd = -1; From 2e30d7aafbe0e2cddc4440a11ce7414ad90cd8d5 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 29 Oct 2012 21:37:39 +0100 Subject: [PATCH 04/13] CONNECT correctly free ct in case first testunitready fails --- lib/connect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connect.c b/lib/connect.c index dc70565..4fce89c 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -61,9 +61,9 @@ iscsi_testunitready_cb(struct iscsi_context *iscsi, int status, "failed."); ct->cb(iscsi, SCSI_STATUS_ERROR, NULL, ct->private_data); + free(ct); } scsi_free_scsi_task(task); - free(ct); return; } } From d989474a3604ba4bf48e55e0a9abdd9f9da5d21d Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 29 Oct 2012 22:07:58 +0100 Subject: [PATCH 05/13] ISCSI-TEST free strings malloc'ed by libpopt --- test-tool/iscsi-test.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test-tool/iscsi-test.c b/test-tool/iscsi-test.c index cad27bd..c34bc1e 100644 --- a/test-tool/iscsi-test.c +++ b/test-tool/iscsi-test.c @@ -436,6 +436,8 @@ int main(int argc, const char *argv[]) if (url == NULL) { fprintf(stderr, "You must specify the URL\n"); print_usage(); + free(skipname); + free(testname); exit(10); } @@ -473,6 +475,10 @@ int main(int argc, const char *argv[]) printf("\n"); } + free(skipname); + free(testname); + free(url); + return num_failed ? num_failed : num_skipped ? 77 : 0; } From 774ede1f46349ce94cb1d89380247d7d6330be40 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 30 Oct 2012 11:41:51 +0100 Subject: [PATCH 06/13] ISCSI_URL change strings from dynamic to static --- include/iscsi-private.h | 2 - include/iscsi.h | 12 +++--- lib/init.c | 94 ++++++----------------------------------- test-tool/iscsi-test.c | 6 +-- 4 files changed, 21 insertions(+), 93 deletions(-) diff --git a/include/iscsi-private.h b/include/iscsi-private.h index 490876a..5c9b15a 100644 --- a/include/iscsi-private.h +++ b/include/iscsi-private.h @@ -61,8 +61,6 @@ enum iscsi_immediate_data { ISCSI_IMMEDIATE_DATA_YES = 1 }; -#define MAX_STRING_SIZE (255) - struct iscsi_context { char initiator_name[MAX_STRING_SIZE+1]; char target_name[MAX_STRING_SIZE+1]; diff --git a/include/iscsi.h b/include/iscsi.h index bdba224..13cc705 100644 --- a/include/iscsi.h +++ b/include/iscsi.h @@ -32,6 +32,8 @@ extern "C" { struct iscsi_context; struct sockaddr; +#define MAX_STRING_SIZE (255) + /* * Syntax for normal and portal/discovery URLs. */ @@ -69,13 +71,11 @@ EXTERN int iscsi_queue_length(struct iscsi_context *iscsi); */ int iscsi_set_tcp_keepalive(struct iscsi_context *iscsi, int idle, int count, int interval); - - struct iscsi_url { - const char *portal; - const char *target; - const char *user; - const char *passwd; + char portal[MAX_STRING_SIZE+1]; + char target[MAX_STRING_SIZE+1]; + char user[MAX_STRING_SIZE+1]; + char passwd[MAX_STRING_SIZE+1]; int lun; }; diff --git a/lib/init.c b/lib/init.c index c62bbae..278e20d 100644 --- a/lib/init.c +++ b/lib/init.c @@ -302,7 +302,7 @@ struct iscsi_url * iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) { struct iscsi_url *iscsi_url; - char *str; + char str[MAX_STRING_SIZE+1]; char *portal; char *user = NULL; char *passwd = NULL; @@ -319,11 +319,7 @@ iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) return NULL; } - str = strdup(url + 8); - if (str == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup url %s", url); - return NULL; - } + strncpy(str,url + 8,MAX_STRING_SIZE); portal = str; user = getenv("LIBISCSI_CHAP_USERNAME"); @@ -353,7 +349,6 @@ iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) "form: %s", url, ISCSI_URL_SYNTAX); - free(str); return NULL; } *target++ = 0; @@ -364,7 +359,6 @@ iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) "iSCSI URL must be of the form: %s", url, ISCSI_URL_SYNTAX); - free(str); return NULL; } @@ -374,7 +368,6 @@ iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) "iSCSI URL must be of the form: %s", url, ISCSI_URL_SYNTAX); - free(str); return NULL; } *lun++ = 0; @@ -385,54 +378,25 @@ iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) "iSCSI URL must be of the form: %s", url, ISCSI_URL_SYNTAX); - free(str); return NULL; } iscsi_url = malloc(sizeof(struct iscsi_url)); if (iscsi_url == NULL) { iscsi_set_error(iscsi, "Out-of-memory: Failed to allocate iscsi_url structure"); - free(str); return NULL; } memset(iscsi_url, 0, sizeof(struct iscsi_url)); - iscsi_url->portal = strdup(portal); - if (iscsi_url->portal == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup portal string"); - iscsi_destroy_url(iscsi_url); - free(str); - return NULL; - } - - iscsi_url->target = strdup(target); - if (iscsi_url->target == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup target string"); - iscsi_destroy_url(iscsi_url); - free(str); - return NULL; - } + strncpy(iscsi_url->portal,portal,MAX_STRING_SIZE); + strncpy(iscsi_url->target,target,MAX_STRING_SIZE); if (user != NULL && passwd != NULL) { - iscsi_url->user = strdup(user); - if (iscsi_url->user == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup username string"); - iscsi_destroy_url(iscsi_url); - free(str); - return NULL; - } - - iscsi_url->passwd = strdup(passwd); - if (iscsi_url->passwd == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup password string"); - iscsi_destroy_url(iscsi_url); - free(str); - return NULL; - } + strncpy(iscsi_url->user,user,MAX_STRING_SIZE); + strncpy(iscsi_url->user,passwd,MAX_STRING_SIZE); } iscsi_url->lun = l; - free(str); return iscsi_url; } @@ -440,7 +404,7 @@ struct iscsi_url * iscsi_parse_portal_url(struct iscsi_context *iscsi, const char *url) { struct iscsi_url *iscsi_url; - char *str; + char str[MAX_STRING_SIZE+1]; char *portal; char *user = NULL; char *passwd = NULL; @@ -454,11 +418,7 @@ iscsi_parse_portal_url(struct iscsi_context *iscsi, const char *url) return NULL; } - str = strdup(url + 8); - if (str == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup url %s", url); - return NULL; - } + strncpy(str,url + 8,MAX_STRING_SIZE); portal = str; user = getenv("LIBISCSI_CHAP_USERNAME"); @@ -477,56 +437,26 @@ iscsi_parse_portal_url(struct iscsi_context *iscsi, const char *url) } } - iscsi_url = malloc(sizeof(struct iscsi_url)); if (iscsi_url == NULL) { iscsi_set_error(iscsi, "Out-of-memory: Failed to allocate iscsi_url structure"); - free(str); return NULL; } memset(iscsi_url, 0, sizeof(struct iscsi_url)); - iscsi_url->portal = strdup(portal); - if (iscsi_url->portal == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup portal string"); - iscsi_destroy_url(iscsi_url); - free(str); - return NULL; - } + strncpy(iscsi_url->portal,portal,MAX_STRING_SIZE); if (user != NULL && passwd != NULL) { - iscsi_url->user = strdup(user); - if (iscsi_url->user == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup username string"); - iscsi_destroy_url(iscsi_url); - free(str); - return NULL; - } - - iscsi_url->passwd = strdup(passwd); - if (iscsi_url->passwd == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to strdup password string"); - iscsi_destroy_url(iscsi_url); - free(str); - return NULL; - } + strncpy(iscsi_url->user,user,MAX_STRING_SIZE); + strncpy(iscsi_url->user,passwd,MAX_STRING_SIZE); } - - free(str); + return iscsi_url; } void iscsi_destroy_url(struct iscsi_url *iscsi_url) { - if (iscsi_url == NULL) { - return; - } - - free(discard_const(iscsi_url->portal)); - free(discard_const(iscsi_url->target)); - free(discard_const(iscsi_url->user)); - free(discard_const(iscsi_url->passwd)); free(iscsi_url); } diff --git a/test-tool/iscsi-test.c b/test-tool/iscsi-test.c index c34bc1e..117773a 100644 --- a/test-tool/iscsi-test.c +++ b/test-tool/iscsi-test.c @@ -475,9 +475,9 @@ int main(int argc, const char *argv[]) printf("\n"); } - free(skipname); - free(testname); - free(url); + free(skipname); + free(testname); + free(url); return num_failed ? num_failed : num_skipped ? 77 : 0; } From 9f82d0bf8303cb5134cf51b0f8cda2eeb112fb22 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 30 Oct 2012 11:47:12 +0100 Subject: [PATCH 07/13] INIT allow a trailing / in iscsi_parse_portal_url() --- lib/init.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/init.c b/lib/init.c index 278e20d..a534c3b 100644 --- a/lib/init.c +++ b/lib/init.c @@ -388,6 +388,8 @@ iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) } memset(iscsi_url, 0, sizeof(struct iscsi_url)); + + strncpy(iscsi_url->portal,portal,MAX_STRING_SIZE); strncpy(iscsi_url->target,target,MAX_STRING_SIZE); @@ -437,6 +439,9 @@ iscsi_parse_portal_url(struct iscsi_context *iscsi, const char *url) } } + tmp=strchr(portal,'/'); + if (tmp) *tmp=0; + iscsi_url = malloc(sizeof(struct iscsi_url)); if (iscsi_url == NULL) { iscsi_set_error(iscsi, "Out-of-memory: Failed to allocate iscsi_url structure"); From ca6f28437ac0c07a24df06daf33b4e50ffd7ed1d Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 30 Oct 2012 11:56:12 +0100 Subject: [PATCH 08/13] INIT remove redundant url parsing code --- lib/init.c | 120 ++++++++++++++++++----------------------------------- 1 file changed, 41 insertions(+), 79 deletions(-) diff --git a/lib/init.c b/lib/init.c index a534c3b..dedcc42 100644 --- a/lib/init.c +++ b/lib/init.c @@ -299,7 +299,7 @@ iscsi_is_logged_in(struct iscsi_context *iscsi) } struct iscsi_url * -iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) +iscsi_parse_url(struct iscsi_context *iscsi, const char *url, int full) { struct iscsi_url *iscsi_url; char str[MAX_STRING_SIZE+1]; @@ -339,48 +339,54 @@ iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) *tmp++ = 0; passwd = tmp; } - } - target = strchr(portal, '/'); - if (target == NULL) { - iscsi_set_error(iscsi, "Invalid URL %s\nCould not parse " + if (full) { + target = strchr(portal, '/'); + if (target == NULL) { + iscsi_set_error(iscsi, "Invalid URL %s\nCould not parse " "''\niSCSI URL must be of the " "form: %s", url, ISCSI_URL_SYNTAX); - return NULL; - } - *target++ = 0; + return NULL; + } + *target++ = 0; - if (*target == 0) { - iscsi_set_error(iscsi, "Invalid URL %s\nCould not parse " + if (*target == 0) { + iscsi_set_error(iscsi, "Invalid URL %s\nCould not parse " "\n" "iSCSI URL must be of the form: %s", url, ISCSI_URL_SYNTAX); - return NULL; - } + return NULL; + } - lun = strchr(target, '/'); - if (lun == NULL) { - iscsi_set_error(iscsi, "Invalid URL %s\nCould not parse \n" + lun = strchr(target, '/'); + if (lun == NULL) { + iscsi_set_error(iscsi, "Invalid URL %s\nCould not parse \n" "iSCSI URL must be of the form: %s", url, ISCSI_URL_SYNTAX); - return NULL; - } - *lun++ = 0; + return NULL; + } + *lun++ = 0; - l = strtol(lun, &tmp, 10); - if (*lun == 0 || *tmp != 0) { - iscsi_set_error(iscsi, "Invalid URL %s\nCould not parse \n" + l = strtol(lun, &tmp, 10); + if (*lun == 0 || *tmp != 0) { + iscsi_set_error(iscsi, "Invalid URL %s\nCould not parse \n" "iSCSI URL must be of the form: %s", url, ISCSI_URL_SYNTAX); - return NULL; + return NULL; + } } - + else + { + tmp=strchr(portal,'/'); + if (tmp) *tmp=0; + } + iscsi_url = malloc(sizeof(struct iscsi_url)); if (iscsi_url == NULL) { iscsi_set_error(iscsi, "Out-of-memory: Failed to allocate iscsi_url structure"); @@ -388,75 +394,31 @@ iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) } memset(iscsi_url, 0, sizeof(struct iscsi_url)); - - strncpy(iscsi_url->portal,portal,MAX_STRING_SIZE); - strncpy(iscsi_url->target,target,MAX_STRING_SIZE); if (user != NULL && passwd != NULL) { strncpy(iscsi_url->user,user,MAX_STRING_SIZE); strncpy(iscsi_url->user,passwd,MAX_STRING_SIZE); } - iscsi_url->lun = l; + if (full) { + strncpy(iscsi_url->target,target,MAX_STRING_SIZE); + iscsi_url->lun = l; + } + return iscsi_url; } +struct iscsi_url * +iscsi_parse_full_url(struct iscsi_context *iscsi, const char *url) +{ + return iscsi_parse_url(iscsi,url,1); +} + struct iscsi_url * iscsi_parse_portal_url(struct iscsi_context *iscsi, const char *url) { - struct iscsi_url *iscsi_url; - char str[MAX_STRING_SIZE+1]; - char *portal; - char *user = NULL; - char *passwd = NULL; - char *tmp; - - if (strncmp(url, "iscsi://", 8)) { - iscsi_set_error(iscsi, "Invalid URL %s\niSCSI Portal URL must be of " - "the form: %s", - url, - ISCSI_PORTAL_URL_SYNTAX); - return NULL; - } - - strncpy(str,url + 8,MAX_STRING_SIZE); - portal = str; - - user = getenv("LIBISCSI_CHAP_USERNAME"); - passwd = getenv("LIBISCSI_CHAP_PASSWORD"); - - tmp = strchr(portal, '@'); - if (tmp != NULL) { - user = portal; - *tmp++ = 0; - portal = tmp; - - tmp = strchr(user, '%'); - if (tmp != NULL) { - *tmp++ = 0; - passwd = tmp; - } - } - - tmp=strchr(portal,'/'); - if (tmp) *tmp=0; - - iscsi_url = malloc(sizeof(struct iscsi_url)); - if (iscsi_url == NULL) { - iscsi_set_error(iscsi, "Out-of-memory: Failed to allocate iscsi_url structure"); - return NULL; - } - memset(iscsi_url, 0, sizeof(struct iscsi_url)); - - strncpy(iscsi_url->portal,portal,MAX_STRING_SIZE); - - if (user != NULL && passwd != NULL) { - strncpy(iscsi_url->user,user,MAX_STRING_SIZE); - strncpy(iscsi_url->user,passwd,MAX_STRING_SIZE); - } - - return iscsi_url; + return iscsi_parse_url(iscsi,url,0); } void From ef3b6fe9111c074db957321e654495a8044b6ca0 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 30 Oct 2012 12:02:09 +0100 Subject: [PATCH 09/13] INIT fix url parsing error message --- lib/init.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/init.c b/lib/init.c index dedcc42..b80456c 100644 --- a/lib/init.c +++ b/lib/init.c @@ -312,10 +312,12 @@ iscsi_parse_url(struct iscsi_context *iscsi, const char *url, int full) int l; if (strncmp(url, "iscsi://", 8)) { + if (full) { iscsi_set_error(iscsi, "Invalid URL %s\niSCSI URL must be of " - "the form: %s", - url, - ISCSI_URL_SYNTAX); + "the form: %s",url,ISCSI_URL_SYNTAX); } + else { + iscsi_set_error(iscsi, "Invalid URL %s\niSCSI Portal URL must be of " + "the form: %s",url,ISCSI_PORTAL_URL_SYNTAX); } return NULL; } From 820410526e853b5db715bd57ba4e23326426c085 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 30 Oct 2012 12:05:25 +0100 Subject: [PATCH 10/13] INIT fix missing export of iscsi_set_tcp_syncnt() --- include/iscsi.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/iscsi.h b/include/iscsi.h index 13cc705..0ab4b84 100644 --- a/include/iscsi.h +++ b/include/iscsi.h @@ -1019,6 +1019,12 @@ iscsi_set_tcp_keepcnt(struct iscsi_context *iscsi, int value); EXTERN void iscsi_set_tcp_keepintvl(struct iscsi_context *iscsi, int value); +/* + * This function is to set the TCP_SYNCNT option. It has to be called after iscsi + * context creation. + */ +EXTERN void +iscsi_set_tcp_syncnt(struct iscsi_context *iscsi, int value); #ifdef __cplusplus } From a0fb2d179d47ec528c102035b49ec7279aad9483 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 30 Oct 2012 16:38:09 +0100 Subject: [PATCH 11/13] ERROR fix error string creation At some points in the code the error string includes itself. This generates self-repeating error messages. --- lib/init.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/init.c b/lib/init.c index b80456c..b01cba1 100644 --- a/lib/init.c +++ b/lib/init.c @@ -243,13 +243,15 @@ void iscsi_set_error(struct iscsi_context *iscsi, const char *error_string, ...) { va_list ap; + char errstr[MAX_STRING_SIZE+1] = {0}; va_start(ap, error_string); - if (vsnprintf(iscsi->error_string, MAX_STRING_SIZE, error_string, ap) < 0) { - strncpy(iscsi->error_string,"could not format error string!",MAX_STRING_SIZE); + if (vsnprintf(errstr, MAX_STRING_SIZE, error_string, ap) < 0) { + strncpy(errstr,"could not format error string!",MAX_STRING_SIZE); } va_end(ap); + strncpy(iscsi->error_string,errstr,MAX_STRING_SIZE); DPRINTF(iscsi,1,"%s",iscsi->error_string); } From a9257d52a7577b607e237e209b9868c5ce78a927 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 30 Oct 2012 17:22:39 +0100 Subject: [PATCH 12/13] CONNECT only read/write from sockets when connection is established qemu-kvm/qemu-img starts in+out polls from the socket before the connection is established. This leads to a hang if the connection cant be established (i.e. the target is down when qemu-kvm is started). before: LIBISCSI_DEBUG=2 qemu-img convert -f iscsi -O raw iscsi://127.0.0.1/iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac/0 /dev/null libiscsi: connecting to portal 127.0.0.1 [iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac] libiscsi: read from socket failed, errno:111 [iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac] libiscsi: connection to 127.0.0.1 established [iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac] ->success!! after: LIBISCSI_DEBUG=1 qemu-img convert -f iscsi -O raw iscsi://127.0.0.1/iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac/0 /dev/null libiscsi: iscsi_service: socket error Connection refused(111) while connecting. [iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac] libiscsi: Failed to connect to iSCSI socket. iscsi_service: socket error Connection refused(111) while connecting. [iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac] qemu-img: iSCSI: Failed to connect to LUN : Failed to connect to iSCSI socket. iscsi_service: socket error Connection refused(111) while connecting. qemu-img: Could not open 'iscsi://127.0.0.1/iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac/0': Invalid argument qemu-img: Could not open 'iscsi://127.0.0.1/iqn.2004-04.com.qnap:ts-809u:iscsi.lieven20.c53dac/0' --- lib/socket.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/socket.c b/lib/socket.c index ce838cc..4c4da61 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -508,12 +508,12 @@ iscsi_service(struct iscsi_context *iscsi, int revents) return 0; } - if (revents & POLLOUT && iscsi->outqueue != NULL) { + if (iscsi->is_connected && revents & POLLOUT && iscsi->outqueue != NULL) { if (iscsi_write_to_socket(iscsi) != 0) { return iscsi_service_reconnect_if_loggedin(iscsi); } } - if (revents & POLLIN) { + if (iscsi->is_connected && revents & POLLIN) { if (iscsi_read_from_socket(iscsi) != 0) { return iscsi_service_reconnect_if_loggedin(iscsi); } From 1d348de71f4b180d23473480e1fdcc9c99982ce1 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 30 Oct 2012 21:00:46 +0100 Subject: [PATCH 13/13] INIT fix typo in iscsi_parse_url() --- lib/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/init.c b/lib/init.c index b01cba1..533b654 100644 --- a/lib/init.c +++ b/lib/init.c @@ -402,7 +402,7 @@ iscsi_parse_url(struct iscsi_context *iscsi, const char *url, int full) if (user != NULL && passwd != NULL) { strncpy(iscsi_url->user,user,MAX_STRING_SIZE); - strncpy(iscsi_url->user,passwd,MAX_STRING_SIZE); + strncpy(iscsi_url->passwd,passwd,MAX_STRING_SIZE); } if (full) {