From 3907139e341ab8c73ab46ce085e7e75893b3902a Mon Sep 17 00:00:00 2001 From: Dariusz Stojaczyk Date: Thu, 9 Mar 2017 15:23:00 +0100 Subject: [PATCH] iscsi: fixed crash on invalid add_portal_group call Fixed double free in spdk_rpc_add_portal_group() spdk_iscsi_portal_create() now takes string arguments as const char* and makes internal copies of them. This patch also fixes potential memory leak when id == NULL Change-Id: I4d0efb101471fb2368ceb8ceecb0e40614e3585d Signed-off-by: Dariusz Stojaczyk --- lib/iscsi/iscsi_rpc.c | 39 ++++++++++++++++++------------------- lib/iscsi/portal_grp.c | 44 ++++++++++++++++++------------------------ lib/iscsi/portal_grp.h | 2 +- 3 files changed, 39 insertions(+), 46 deletions(-) diff --git a/lib/iscsi/iscsi_rpc.c b/lib/iscsi/iscsi_rpc.c index 4c40cf3fc..49029073a 100644 --- a/lib/iscsi/iscsi_rpc.c +++ b/lib/iscsi/iscsi_rpc.c @@ -742,12 +742,13 @@ spdk_rpc_add_portal_group(struct spdk_jsonrpc_server_conn *conn, struct spdk_iscsi_portal *portal_list[MAX_PORTAL] = {}; struct spdk_json_write_ctx *w; size_t i; + int rc = -1; if (spdk_json_decode_object(params, rpc_portal_group_decoders, SPDK_COUNTOF(rpc_portal_group_decoders), &req)) { SPDK_ERRLOG("spdk_json_decode_object failed\n"); - goto invalid; + goto out; } for (i = 0; i < req.portal_list.num_portals; i++) { @@ -755,34 +756,32 @@ spdk_rpc_add_portal_group(struct spdk_jsonrpc_server_conn *conn, req.portal_list.portals[i].port, 0); if (portal_list[i] == NULL) { SPDK_ERRLOG("portal_list allocation failed\n"); - goto invalid; + goto out; } } - if (spdk_iscsi_portal_grp_create_from_portal_list(req.tag, portal_list, - req.portal_list.num_portals)) { + rc = spdk_iscsi_portal_grp_create_from_portal_list(req.tag, portal_list, + req.portal_list.num_portals); + + if (rc < 0) { SPDK_ERRLOG("create_from_portal_list failed\n"); - goto invalid; } - /* - * Leave the host and port strings allocated here and don't call free_rpc_portal_group(), - * because the pointers are inserted directly into the portal group list. - */ - - if (id == NULL) { - return; +out: + if (rc > 0) { + if (id != NULL) { + w = spdk_jsonrpc_begin_result(conn, id); + spdk_json_write_bool(w, true); + spdk_jsonrpc_end_result(conn, w); + } + } else { + spdk_jsonrpc_send_error_response(conn, id, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters"); } - w = spdk_jsonrpc_begin_result(conn, id); - spdk_json_write_bool(w, true); - spdk_jsonrpc_end_result(conn, w); - return; - -invalid: - spdk_jsonrpc_send_error_response(conn, id, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters"); for (i = 0; i < req.portal_list.num_portals; i++) { - spdk_iscsi_portal_destroy(portal_list[i]); + if (portal_list[i] != NULL) { + spdk_iscsi_portal_destroy(portal_list[i]); + } } free_rpc_portal_group(&req); } diff --git a/lib/iscsi/portal_grp.c b/lib/iscsi/portal_grp.c index b43a9c846..12298c4f0 100644 --- a/lib/iscsi/portal_grp.c +++ b/lib/iscsi/portal_grp.c @@ -65,7 +65,7 @@ spdk_iscsi_portal_grp_open(struct spdk_iscsi_portal_grp *pg); /* Assumes caller allocated host and port strings on the heap */ struct spdk_iscsi_portal * -spdk_iscsi_portal_create(char *host, char *port, uint64_t cpumask) +spdk_iscsi_portal_create(const char *host, const char *port, uint64_t cpumask) { struct spdk_iscsi_portal *p = NULL; @@ -77,8 +77,8 @@ spdk_iscsi_portal_create(char *host, char *port, uint64_t cpumask) SPDK_ERRLOG("portal malloc error (%s, %s)\n", host, port); return NULL; } - p->host = host; - p->port = port; + p->host = strdup(host); + p->port = strdup(port); p->cpumask = cpumask; p->sock = -1; p->group = NULL; /* set at a later time by caller */ @@ -106,7 +106,7 @@ spdk_iscsi_portal_create_from_configline(const char *portalstring, const char *cpumask_str; uint64_t cpumask = 0; - int n, len; + int n, len, rc = -1; const char *p, *q; if (portalstring == NULL) { @@ -240,14 +240,12 @@ spdk_iscsi_portal_create_from_configline(const char *portalstring, } } - return 0; - + rc = 0; error_out: - if (host != NULL) - free(host); - if (port != NULL) - free(port); - return -1; + free(host); + free(port); + + return rc; } struct spdk_iscsi_portal_grp * @@ -316,13 +314,13 @@ spdk_iscsi_portal_grp_create_from_portal_list(int tag, if (num_portals > MAX_PORTAL) { SPDK_ERRLOG("%d > MAX_PORTAL\n", num_portals); - goto error_out; + return -1; } pg = spdk_iscsi_portal_grp_create(tag); if (!pg) { SPDK_ERRLOG("portal group creation error (%d)\n", tag); - goto error_out; + return -1; } for (i = 0; i < num_portals; i++) { @@ -336,28 +334,24 @@ spdk_iscsi_portal_grp_create_from_portal_list(int tag, sock = spdk_sock_listen(p->host, port); if (sock < 0) { SPDK_ERRLOG("listen error %.64s:%d\n", p->host, port); - spdk_iscsi_portal_destroy(p); count++; continue; } p->sock = sock; spdk_iscsi_portal_grp_add_portal(pg, p); + portal_list[i] = NULL; } - /* if listening is failed on all the ports, - * then do not register the portal group. */ if (count == num_portals) { + /* if listening is failed on all the ports, + * then do not register the portal group. */ spdk_iscsi_portal_grp_destroy(pg); - goto error_out; + return -1; + } else { + /* Add portal group to the end of the pg list */ + spdk_iscsi_portal_grp_register(pg); + return num_portals - count; } - - /* Add portal group to the end of the pg list */ - spdk_iscsi_portal_grp_register(pg); - - return 0; - -error_out: - return -1; } int diff --git a/lib/iscsi/portal_grp.h b/lib/iscsi/portal_grp.h index 3423cd54f..adb18da50 100644 --- a/lib/iscsi/portal_grp.h +++ b/lib/iscsi/portal_grp.h @@ -56,7 +56,7 @@ struct spdk_iscsi_portal_grp { /* SPDK iSCSI Portal Group management API */ -struct spdk_iscsi_portal *spdk_iscsi_portal_create(char *host, char *port, +struct spdk_iscsi_portal *spdk_iscsi_portal_create(const char *host, const char *port, uint64_t cpumask); void spdk_iscsi_portal_destroy(struct spdk_iscsi_portal *p);