Date: Fri, 29 Apr 2022 23:12:19 GMT From: John Baldwin <jhb@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: a96064b669fd - stable/13 - libiscsiutil: Fix a memory leak with negotiation keys. Message-ID: <202204292312.23TNCJeb044492@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=a96064b669fd57fdeb4120a116d9a6c4dfe0847c commit a96064b669fd57fdeb4120a116d9a6c4dfe0847c Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2021-12-22 18:41:01 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-04-29 21:15:38 +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 (cherry picked from commit fd99905b4591a5e4df3dda32e4c67258aaf44517) --- 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 8c156877a689..9b165df87557 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202204292312.23TNCJeb044492>