Skip to content

Commit

Permalink
KCM: fix memory leak
Browse files Browse the repository at this point in the history
`ldb_msg_add_value()` makes a copy under the hood, so there is no need
to make intermediate copy of `secret` argument.
This copy - `secret_val.data` - was left hanging on `sss_sec_ctx`, effectively
resulting in a memory leak.

:fixes:'sssd_kcm' memory leak was fixed.
  • Loading branch information
alexey-tikhonov committed Feb 4, 2025
1 parent 196ad92 commit 5d3f318
Showing 1 changed file with 2 additions and 22 deletions.
24 changes: 2 additions & 22 deletions src/responder/kcm/secrets/secrets.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -1029,13 +1029,6 @@ 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;
}

ret = ldb_msg_add_value(msg, SEC_ATTR_SECRET, &secret_val, NULL);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
Expand Down Expand Up @@ -1069,9 +1062,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);
}
Expand All @@ -1084,7 +1074,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;

Expand Down Expand Up @@ -1134,13 +1124,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) {
Expand Down Expand Up @@ -1174,9 +1157,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);
}
Expand Down

0 comments on commit 5d3f318

Please sign in to comment.