Resolves: RHEL-78061 - 'sssd_kcm' leaks memory

0001-KCM-fix-memory-leak.patch

@@ -0,0 +1,113 @@
+From 50f703f25914254d2a545f52f504dfa5a6f65546 Mon Sep 17 00:00:00 2001
+From: Alexey Tikhonov <atikhono@redhat.com>
+Date: Tue, 4 Feb 2025 18:59:36 +0100
+Subject: [PATCH] KCM: fix memory leak
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+The copy of 'secret' argument - `secret_val.data` - was left hanging
+on `sss_sec_ctx`, effectively resulting in a memory leak.
+But this copy isn't actually required as this data isn't modified in
+below operations.
+Skipping alloc+memcpy+erase is also beneficial performance wise.
+
+:fixes:'sssd_kcm' memory leak was fixed.
+
+Reviewed-by: Alejandro López <allopez@redhat.com>
+Reviewed-by: Justin Stephenson <jstephen@redhat.com>
+(cherry picked from commit 7f1b7c9689827df92e8b2166423d4e80688dbacb)
+---
+ src/responder/kcm/secrets/secrets.c | 34 ++++++++++-------------------
+ 1 file changed, 12 insertions(+), 22 deletions(-)
+
+diff --git a/src/responder/kcm/secrets/secrets.c b/src/responder/kcm/secrets/secrets.c
+index 625a09f39..fe7410cb3 100644
+--- a/src/responder/kcm/secrets/secrets.c
++++ b/src/responder/kcm/secrets/secrets.c
+@@ -979,7 +979,7 @@ errno_t sss_sec_put(struct sss_sec_req *req,
+                     size_t secret_len)
+ {
+     struct ldb_message *msg;
+-    struct ldb_val secret_val = { .data = NULL };
++    const struct ldb_val secret_val = { .length = secret_len, .data = secret };
+     bool erase_msg = false;
+     int ret;
+ 
+@@ -1029,13 +1029,11 @@ errno_t sss_sec_put(struct sss_sec_req *req,
+         goto done;
+     }
+ 
+-    secret_val.length = secret_len;
+-    secret_val.data = talloc_memdup(req->sctx, secret, secret_len);
+-    if (!secret_val.data) {
+-        ret = ENOMEM;
+-        goto done;
+-    }
+-
++    /* `ldb_msg_add_value()` does NOT make a copy of secret_val::*data
++     * but rather copies a pointer under the hood.
++     * This is fine since no operations modifying this data are performed
++     * below and 'msg' is freed before function returns.
++     */
+     ret = ldb_msg_add_value(msg, SEC_ATTR_SECRET, &secret_val, NULL);
+     if (ret != EOK) {
+         DEBUG(SSSDBG_OP_FAILURE,
+@@ -1069,9 +1067,6 @@ errno_t sss_sec_put(struct sss_sec_req *req,
+ 
+     ret = EOK;
+ done:
+-    if (secret_val.data != NULL) {
+-        sss_erase_mem_securely(secret_val.data, secret_val.length);
+-    }
+     if (erase_msg) {
+         db_result_erase_message_securely(msg, SEC_ATTR_SECRET);
+     }
+@@ -1084,7 +1079,7 @@ errno_t sss_sec_update(struct sss_sec_req *req,
+                        size_t secret_len)
+ {
+     struct ldb_message *msg;
+-    struct ldb_val secret_val = { .data = NULL };
++    const struct ldb_val secret_val = { .length = secret_len, .data = secret };
+     bool erase_msg = false;
+     int ret;
+ 
+@@ -1134,13 +1129,6 @@ errno_t sss_sec_update(struct sss_sec_req *req,
+         goto done;
+     }
+ 
+-    secret_val.length = secret_len;
+-    secret_val.data = talloc_memdup(req->sctx, secret, secret_len);
+-    if (!secret_val.data) {
+-        ret = ENOMEM;
+-        goto done;
+-    }
+-
+     /* FIXME - should we have a lastUpdate timestamp? */
+     ret = ldb_msg_add_empty(msg, SEC_ATTR_SECRET, LDB_FLAG_MOD_REPLACE, NULL);
+     if (ret != LDB_SUCCESS) {
+@@ -1150,6 +1138,11 @@ errno_t sss_sec_update(struct sss_sec_req *req,
+         goto done;
+     }
+ 
++    /* `ldb_msg_add_value()` does NOT make a copy of secret_val::*data
++     * but rather copies a pointer under the hood.
++     * This is fine since no operations modifying this data are performed
++     * below and 'msg' is freed before function returns.
++     */
+     ret = ldb_msg_add_value(msg, SEC_ATTR_SECRET, &secret_val, NULL);
+     if (ret != LDB_SUCCESS) {
+         DEBUG(SSSDBG_MINOR_FAILURE,
+@@ -1174,9 +1167,6 @@ errno_t sss_sec_update(struct sss_sec_req *req,
+ 
+     ret = EOK;
+ done:
+-    if (secret_val.data != NULL) {
+-        sss_erase_mem_securely(secret_val.data, secret_val.length);
+-    }
+     if (erase_msg) {
+         db_result_erase_message_securely(msg, SEC_ATTR_SECRET);
+     }
+-- 
+2.47.0
+

sssd.spec

@@ -57,7 +57,7 @@
 
 Name: sssd
 Version: 2.10.2
-Release: 1%{?dist}
+Release: 2%{?dist}
 Summary: System Security Services Daemon
 License: GPL-3.0-or-later
 URL: https://github.com/SSSD/sssd/
@@ -65,7 +65,7 @@ Source0: https://github.com/SSSD/sssd/releases/download/2.10.2/sssd-2.10.2.tar.g
 Source1: sssd.sysusers
 
 ### Patches ###
-# Patch0001:
+Patch0001: 0001-KCM-fix-memory-leak.patch
 
 ### Dependencies ###
 
@@ -1116,6 +1116,9 @@ fi
 %systemd_postun_with_restart sssd.service
 
 %changelog
+* Mon Feb 10 2025 Alexey Tikhonov <atikhono@redhat.com> - 2.10.2-2
+- Resolves: RHEL-78061 - 'sssd_kcm' leaks memory
+
 * Wed Jan 29 2025 Alexey Tikhonov <atikhono@redhat.com> - 2.10.2-1
 - Resolves: RHEL-62725 - Rebase SSSD for RHEL 10.0