From nobody Wed Dec 22 18:43:36 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 5EFD219096F0; Wed, 22 Dec 2021 18:43:37 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JK2K5005Rz3vtb; Wed, 22 Dec 2021 18:43:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id D41CD223F7; Wed, 22 Dec 2021 18:43:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1BMIhaCf089840; Wed, 22 Dec 2021 18:43:36 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1BMIhaXm089839; Wed, 22 Dec 2021 18:43:36 GMT (envelope-from git) Date: Wed, 22 Dec 2021 18:43:36 GMT Message-Id: <202112221843.1BMIhaXm089839@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: John Baldwin Subject: git: fd99905b4591 - main - libiscsiutil: Fix a memory leak with negotiation keys. List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jhb X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: fd99905b4591a5e4df3dda32e4c67258aaf44517 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1640198617; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=M7VEs5430ehyTeJ7E/YPMlB4SoG2XVCTxN+6j1GRp1A=; b=Vs+otbKiKOUAp/wpTyr9RBJMMYW5KA3r+msc+9NH0XnCIQnTybn2lUS/wPIR2gQGgjvt8S BR6qe0ynMTqo9AOpm67sWg35RIMRYakLjasc7o8casEucxg+PNq1/2q43XbjxBhdAo7B0J BaO1m3Uoe19mptgEbD1gnZEK4l8j/mZdtIhiIeLa7wpSljflYiWXpVymjxawk0VYlGutTG 8Uct0LgBIs19l6FHTmG7FyGIiHjEgJNRLlRCR0BZLgjItZcyvWamGQTOm8WlcU9AiIBHab ibVILRA41O37oY/9gms2Wvfr9k/bukUVkGSN87oobtcy/rhvxiL0C9LvDNPUmA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1640198617; a=rsa-sha256; cv=none; b=fG7qvejtnxjPohiU0xbf6ZIt6TrbM7aFuIE6CseRDnwwZgAOPkWGK+CMChXlgZFfl3LzhR goigO0j6MUXwwCg8AcYwy1XcedaFQY3AxHoEvQaYICvEv+LNRaFNCLeV+UgbuN+NQC61cP ODA/DRY/wu/U/d94PVm5sYCRVVxpCSKcnze5zIAwemprCXKTWuYt9P+HvuSTaZ/Nr4jI2+ Y6exCnPlm5MESMzuv7tgTdevOvjd28PGzzlbPyZ/xNRE86C2l8Shbdvu9k0iZySN8+vWaH yLdx78W6/aio0p2pyx71y3Ba+LTW1K7GLWO0IjsJVYFv1ih5YtI6cwyr8Mgf1g== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=fd99905b4591a5e4df3dda32e4c67258aaf44517 commit fd99905b4591a5e4df3dda32e4c67258aaf44517 Author: John Baldwin AuthorDate: 2021-12-22 18:41:01 +0000 Commit: John Baldwin CommitDate: 2021-12-22 18:43:23 +0000 libiscsiutil: Fix a memory leak with negotiation keys. When keys are loaded from a received PDU, a copy of the received keys block is saved in the keys struct and the name and value pointers point into that saved block. Freeing the keys frees this block. However, when keys are added to a keys struct to build a set of keys later sent in a PDU, the keys data block pointer is not used and individual key names and values hold allocated strings. When the keys structure was freed, all of these individual key name and value strings were leaked. Instead, allocate copies of strings for names and values when parsing a set of keys from a received PDU and free all of the individual key name and value strings when deleting a set of keys. Reviewed by: mav Sponsored by: Chelsio Communications Differential Revision: https://reviews.freebsd.org/D33545 --- lib/libiscsiutil/keys.c | 30 +++++++++++++++++------------- lib/libiscsiutil/libiscsiutil.h | 2 -- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/libiscsiutil/keys.c b/lib/libiscsiutil/keys.c index 8011b0a25329..185a179906b9 100644 --- a/lib/libiscsiutil/keys.c +++ b/lib/libiscsiutil/keys.c @@ -51,7 +51,10 @@ void keys_delete(struct keys *keys) { - free(keys->keys_data); + for (int i = 0; i < KEYS_MAX; i++) { + free(keys->keys_names[i]); + free(keys->keys_values[i]); + } free(keys); } @@ -59,7 +62,7 @@ void keys_load(struct keys *keys, const struct pdu *pdu) { int i; - char *pair; + char *keys_data, *name, *pair, *value; size_t pair_len; if (pdu->pdu_data_len == 0) @@ -68,35 +71,36 @@ keys_load(struct keys *keys, const struct pdu *pdu) if (pdu->pdu_data[pdu->pdu_data_len - 1] != '\0') log_errx(1, "protocol error: key not NULL-terminated\n"); - assert(keys->keys_data == NULL); - keys->keys_data_len = pdu->pdu_data_len; - keys->keys_data = malloc(keys->keys_data_len); - if (keys->keys_data == NULL) + keys_data = malloc(pdu->pdu_data_len); + if (keys_data == NULL) log_err(1, "malloc"); - memcpy(keys->keys_data, pdu->pdu_data, keys->keys_data_len); + memcpy(keys_data, pdu->pdu_data, pdu->pdu_data_len); /* * XXX: Review this carefully. */ - pair = keys->keys_data; + pair = keys_data; for (i = 0;; i++) { if (i >= KEYS_MAX) log_errx(1, "too many keys received"); pair_len = strlen(pair); - keys->keys_values[i] = pair; - keys->keys_names[i] = strsep(&keys->keys_values[i], "="); - if (keys->keys_names[i] == NULL || keys->keys_values[i] == NULL) + value = pair; + name = strsep(&value, "="); + if (name == NULL || value == NULL) log_errx(1, "malformed keys"); + keys->keys_names[i] = checked_strdup(name); + keys->keys_values[i] = checked_strdup(value); log_debugx("key received: \"%s=%s\"", keys->keys_names[i], keys->keys_values[i]); pair += pair_len + 1; /* +1 to skip the terminating '\0'. */ - if (pair == keys->keys_data + keys->keys_data_len) + if (pair == keys_data + pdu->pdu_data_len) break; - assert(pair < keys->keys_data + keys->keys_data_len); + assert(pair < keys_data + pdu->pdu_data_len); } + free(keys_data); } void diff --git a/lib/libiscsiutil/libiscsiutil.h b/lib/libiscsiutil/libiscsiutil.h index 79c79872b2e6..20979626aa3c 100644 --- a/lib/libiscsiutil/libiscsiutil.h +++ b/lib/libiscsiutil/libiscsiutil.h @@ -75,8 +75,6 @@ struct connection_ops { struct keys { char *keys_names[KEYS_MAX]; char *keys_values[KEYS_MAX]; - char *keys_data; - size_t keys_data_len; }; #define CHAP_CHALLENGE_LEN 1024