>From ab9023eee5c9d545dd6a9e4af5154b79a73326ef Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 9 Jul 2014 19:03:30 +0200 Subject: [PATCH 2/3] 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". --- src/sss_client/nss_mc_group.c | 8 ++++++++ src/sss_client/nss_mc_passwd.c | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c index 5af55468f1a6fb2847844f2131cad6e2f83588b4..268b40ef02f2a621c4f61755ce4dfe2c3786bfa6 100644 --- a/src/sss_client/nss_mc_group.c +++ b/src/sss_client/nss_mc_group.c @@ -130,6 +130,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; @@ -205,6 +209,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 95b8a0407bc8650d6354b13823fbb486b8c64839..fa19afc3c0e468430183ed3f13b80e086251ee01 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; -- 1.9.3