git: fd99905b4591 - main - libiscsiutil: Fix a memory leak with negotiation keys.

The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=fd99905b4591a5e4df3dda32e4c67258aaf44517

commit fd99905b4591a5e4df3dda32e4c67258aaf44517
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2021-12-22 18:41:01 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
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