summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLukas Slebodnik <lslebodn@redhat.com>2014-07-09 19:03:30 +0200
committerJakub Hrozek <jhrozek@redhat.com>2014-07-23 21:10:27 +0200
commit423acb81f83bc8d2954746f370c640fb426b773a (patch)
tree67aab3059948c9300f370328ec9595a3e6d7ed3d
parent7b8b213f8babc718d977b5ef5b260e62f6300388 (diff)
downloadsssd-423acb81f83bc8d2954746f370c640fb426b773a.tar.gz
sssd-423acb81f83bc8d2954746f370c640fb426b773a.tar.xz
sssd-423acb81f83bc8d2954746f370c640fb426b773a.zip
sss_client: Fix memory leak in nss_mc_{group,passwd}
Memory leak can happen with long living clients where there are records with colliding hashes; usually LDAP servers with many users or groups. Function sss_nss_mc_get_record allocates memory that is stored into "rec", with next iteration variable rec is overriden with new record and old one is lost and cannot be freed. Example code flow: src/sss_client/nss_mc_group.c:133: alloc_arg: "sss_nss_mc_get_record" allocates memory that is stored into "rec". src/sss_client/nss_mc_common.c:216:13: alloc_fn: Storage is returned from allocation function "malloc". src/sss_client/nss_mc_common.c:216:13: var_assign: Assigning: "copy_rec" = "malloc(rec_len)". src/sss_client/nss_mc_common.c:225:9: noescape: Resource "copy_rec" is not freed or pointed-to in function "memcpy". [Note: The source code implementation of the function has been overridden by a builtin model.] src/sss_client/nss_mc_common.c:239:5: var_assign: Assigning: "*_rec" = "copy_rec". src/sss_client/nss_mc_group.c:163: noescape: Resource "rec" is not freed or pointed-to in "sss_nss_mc_next_slot_with_hash". src/sss_client/nss_mc_common.c:294:60: noescape: "sss_nss_mc_next_slot_with_hash(struct sss_mc_rec *, uint32_t)" does not free or save its pointer parameter "rec". src/sss_client/nss_mc_group.c:133: overwrite_var: Overwriting "rec" in call to "sss_nss_mc_get_record" leaks the storage that "rec" points to. src/sss_client/nss_mc_common.c:239:5: write_notnull_to_parm: Assigning: "*_rec" = "copy_rec". Reviewed-by: Michal Židek <mzidek@redhat.com> Reviewed-by: Sumit Bose <sbose@redhat.com> (cherry picked from commit 9d876108620931e0941a115adf60bfd8d67459d9)
-rw-r--r--src/sss_client/nss_mc_group.c8
-rw-r--r--src/sss_client/nss_mc_passwd.c8
2 files changed, 16 insertions, 0 deletions
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index 4e3d9fb0d..8bb6f33ee 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -122,6 +122,10 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
while (MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
+ /* free record from previous iteration */
+ free(rec);
+ rec = NULL;
+
ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -197,6 +201,10 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
while (MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
+ /* free record from previous iteration */
+ free(rec);
+ rec = NULL;
+
ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
if (ret) {
goto done;
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index a0a8d87f7..b952b3658 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -123,6 +123,10 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
while (MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
+ /* free record from previous iteration */
+ free(rec);
+ rec = NULL;
+
ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
if (ret) {
goto done;
@@ -199,6 +203,10 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
while (MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
+ /* free record from previous iteration */
+ free(rec);
+ rec = NULL;
+
ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
if (ret) {
goto done;