From nobody Fri Apr 29 23:12:19 2022 X-Original-To: dev-commits-src-branches@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 592591ABC156; Fri, 29 Apr 2022 23:12:20 +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 4KqpD35kzlz3vwt; Fri, 29 Apr 2022 23:12:19 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1651273939; 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=+0FrRj3XVO5G7AJarXxNhmMa7ZAeEeyogCz5f9qV2RE=; b=ISyNIqjqf+XhfFzq7tAmttcdmDu8TX6Qr4o0uWhWfAoPtJJJi38lFJ6myq91F8hiXE/88L JwrbprXN9DL+k/K+meOt/erHlGJXL0Essa63Bv6l8qYUTlLr2I8t3RPKut+yYbfYARP0lF JBfEMvo/7pJCbvABH00RyJL8toIUM4HpoDJ7Z5z6dcKnkDfDuaY6N86OzhQUkvi4PMj6Lm HpADLfbc/enKwHC/8HkMG6wKnLF2istWCEF2Sy2iJVoFEG8H9uH1YTXLGAyYOqq06QBgmJ ny5AcyOqMkSefpwzm/JjKOQKG32K8BIcoXNjqo0Vse624cmdeXQP/xZV1F4pyg== 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 64FA515EE0; Fri, 29 Apr 2022 23:12:19 +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 23TNCJdJ044493; Fri, 29 Apr 2022 23:12:19 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 23TNCJeb044492; Fri, 29 Apr 2022 23:12:19 GMT (envelope-from git) Date: Fri, 29 Apr 2022 23:12:19 GMT Message-Id: <202204292312.23TNCJeb044492@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: John Baldwin Subject: git: a96064b669fd - stable/13 - libiscsiutil: Fix a memory leak with negotiation keys. List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@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/stable/13 X-Git-Reftype: branch X-Git-Commit: a96064b669fd57fdeb4120a116d9a6c4dfe0847c Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1651273939; 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=+0FrRj3XVO5G7AJarXxNhmMa7ZAeEeyogCz5f9qV2RE=; b=avOr7wHQJVJcIRbKo+PBEfIPUcv+SFSdxhSKtNT3FK+4Pa+nM31CCau/a5ymBtRXUZn3sc ONE6t2dYXvMCbS+as//EOfa1K+Agkz8jd2TWw74UwR5/G4/zhjhkMaFwEbW6jmj7Qmo4Zh xqZ2Njfe6/kC/0ugyC2JiwmhKD+ZMvVFUfujWslJg79VtCtTUcTCA0TzzY5vmXB2TQVJ9q wnSxEym5svZGKcY8SK6jl3og8klkFmGJy48SOo7eNGP1ehdZFK5Hp2o/IzqSqFucbFD/aR m6mpvBbPEEfdlhKP7evI1k9MlJ8PRAoxKuaioup/Yw8rqYrE9wn4RAOoQtL3dg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1651273939; a=rsa-sha256; cv=none; b=E+xdVA6E7hcUzjUmLTkh4mvyvXjS8xFterkkme2EmB+nwUDD93QYK7TQ6JV508GMCmXIS8 61Mx33APKKSKDIlSmI+i5EMsjxZnv0GD/c/ZMSJsW0xeYo6iaknizb4y70y7/s0+jSk4mz dCRvuHnz8lqTpeoPYiTXzXl2Lzl5FaYM5Kr2745XyPkcdL5lLcUXMMeZDDCD137I7CqCiQ /pc1ZIKAVEyGVSQZXQk4q9lwoEQan4DsH7OdDl7iLjD3WaSyBltXzy6oL/XyEcwpxw/uiw NAEMh8S++HLZ8Svcyeb086elKYyBMuNDdcoJtmtLJVKhtsRfW0sywNJQs00uxA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=a96064b669fd57fdeb4120a116d9a6c4dfe0847c commit a96064b669fd57fdeb4120a116d9a6c4dfe0847c Author: John Baldwin AuthorDate: 2021-12-22 18:41:01 +0000 Commit: John Baldwin 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