[PATCH] NSS: Fix memory leak netgroup

[Pavel Reichl]

Hello,

attached patch proposes solution for leaking memory when non-existing netgroup is looked up.

1st patch is just for testing - just call 'pkill -SIGUSR1 sssd_nss' and talloc report will be generated in /tmp/sssd_nss_talloc_report_full.

For details about the bug please see commit message in 2nd patch.

User who reported the bug confirmed that so far it seems that memory leak has been fixed and he didn't report any side effects.

Thanks!

[Pavel Reichl]

Bump.

[Jakub Hrozek]

On Wed, Nov 18, 2015 at 03:06:03PM +0100, Pavel Reichl wrote:
> Hello,
>
> attached patch proposes solution for leaking memory when non-existing netgroup is looked up.
>
> 1st patch is just for testing - just call 'pkill -SIGUSR1 sssd_nss' and talloc report will be generated in /tmp/sssd_nss_talloc_report_full.
>
> For details about the bug please see commit message in 2nd patch.
>
> User who reported the bug confirmed that so far it seems that memory leak has been fixed and he didn't report any side effects.
>
> Thanks!
> From abbf720b832beb6d04f909f598e7114a19f72a03 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl 
> Date: Wed, 30 Sep 2015 07:54:31 -0400
> Subject: [PATCH 1/2] talloc_report for nss responder
>
> src/responder/nss/nsssrv.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
> index d8eff7968c4929663412aa56d08414689b921a22..79ca9ba89d49a5fd4ee3389e0f881b1a01f9c7a9 100644
> --- a/src/responder/nss/nsssrv.c
> +++ b/src/responder/nss/nsssrv.c
> @@ -403,6 +403,20 @@ static void nss_dp_reconnect_init(struct sbus_connection *conn,
>      /* nss_shutdown(rctx); */
>  }
>
> +static void signal_nss_talloc_report(struct tevent_context *ev,
>                                 struct tevent_signal *se,
>                                 int signum,
>                                 int count,
>                                 void *siginfo,
>                                 void *private_data)
>
> +{
> FILE *f = fopen("/tmp/sssd_nss_talloc_report_full", "w");
> if (f != NULL) {
>    talloc_report_full(NULL, f);
>    fclose(f);
> }
>
> +}
>
> int nss_process_init(TALLOC_CTX *mem_ctx,
>                       struct tevent_context *ev,
>                       struct confdb_ctx *cdb)
> @@ -417,6 +431,8 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
>      int hret;
>      int fd_limit;
>
> talloc_enable_leak_report_full();
>
> nss_cmds = get_nss_cmds();
>
> ret = sss_process_init(mem_ctx, ev, cdb,
>
> @@ -558,6 +574,16 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
>          goto fail;
>      }
>
> /* Handle SIGUSR1 to force offline behavior */
>
> BlockSignals(false, SIGUSR1);
>
> struct tevent_signal *tes;
>
> tes = tevent_add_signal(rctx->ev, rctx, SIGUSR1, 0,
>                        signal_nss_talloc_report, rctx);
>
> if (tes == NULL) {
>    ret = EIO;
>    goto fail;
>
> }
>
> DEBUG(SSSDBG_TRACE_FUNC, "NSS Initialization complete\n");
>
> return EOK;
>
> -- 
> 2.4.3
> From 0c05cef940b8a8650537056db4ade09b0b6a6fa6 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl 
> Date: Tue, 10 Nov 2015 10:56:56 -0500
> Subject: [PATCH 2/2] NSS: Fix memory leak netgroup
>
> If netgroup cannot be found in setnetgrent_retry() function
> set_netgroup_entry() is called which steals getent_ctx directly to
> nss_ctx->netgroups.
> Subsequently function lookup_netgr_step() is called that (in case of
> nenexisting group) will call create_negcache_netgr() which creates
> a new dummy object to serve as negative cache. While doing so it calls
> again set_netgroup_entry() for the same netgroup and it calls
> hash_enter.
>
> hash_enter will remove previously hashed entry for netgroup (created in
> setnetgrent_retry()) from hash table but it won't be freed and thus it
> leaks.
>
> This patch sets netgroup lifetime for netgroups allocated in
> setnetgrent_retry().

We already set the netgroup lifetime in both the found and notfound case
in lookup_netgr_step(). If I understand the code correctly, the issue is
that we create a new negative result in create_negcache_netgr(),
overwriting the one that we created in setnetgrent_retry(), correct?

Wouldn't it then be better to see if another same object is already in
the hashtable and free it before replacing?

[Pavel Reichl]

On 11/27/2015 10:06 AM, Jakub Hrozek wrote:
> On Wed, Nov 18, 2015 at 03:06:03PM +0100, Pavel Reichl wrote:
> > Hello,
> >
> > attached patch proposes solution for leaking memory when non-existing netgroup is looked up.
> >
> > 1st patch is just for testing - just call 'pkill -SIGUSR1 sssd_nss' and talloc report will be generated in /tmp/sssd_nss_talloc_report_full.
> >
> > For details about the bug please see commit message in 2nd patch.
> >
> > User who reported the bug confirmed that so far it seems that memory leak has been fixed and he didn't report any side effects.
> >
> > Thanks!
> > From abbf720b832beb6d04f909f598e7114a19f72a03 Mon Sep 17 00:00:00 2001
> > From: Pavel Reichl 
> > Date: Wed, 30 Sep 2015 07:54:31 -0400
> > Subject: [PATCH 1/2] talloc_report for nss responder
> >
> > src/responder/nss/nsssrv.c | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> >
> > diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
> > index d8eff7968c4929663412aa56d08414689b921a22..79ca9ba89d49a5fd4ee3389e0f881b1a01f9c7a9 100644
> > --- a/src/responder/nss/nsssrv.c
> > +++ b/src/responder/nss/nsssrv.c
> > @@ -403,6 +403,20 @@ static void nss_dp_reconnect_init(struct sbus_connection *conn,
> >       /* nss_shutdown(rctx); */
> >   }
> >
> > +static void signal_nss_talloc_report(struct tevent_context *ev,
> >                                 struct tevent_signal *se,
> >                                 int signum,
> >                                 int count,
> >                                 void *siginfo,
> >                                 void *private_data)
> >
> > +{
> > FILE *f = fopen("/tmp/sssd_nss_talloc_report_full", "w");
> > if (f != NULL) {
> >    talloc_report_full(NULL, f);
> >    fclose(f);
> > }
> >
> > +}
> > int nss_process_init(TALLOC_CTX *mem_ctx,
> >                      struct tevent_context *ev,
> >                      struct confdb_ctx *cdb)
> >
> > @@ -417,6 +431,8 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
> >       int hret;
> >       int fd_limit;
> >
> > talloc_enable_leak_report_full();
> > nss_cmds = get_nss_cmds();
> >
> > ret = sss_process_init(mem_ctx, ev, cdb,
> >
> > @@ -558,6 +574,16 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
> >           goto fail;
> >       }
> >
> > /* Handle SIGUSR1 to force offline behavior */
> >
> > BlockSignals(false, SIGUSR1);
> >
> > struct tevent_signal *tes;
> >
> > tes = tevent_add_signal(rctx->ev, rctx, SIGUSR1, 0,
> >                        signal_nss_talloc_report, rctx);
> >
> > if (tes == NULL) {
> >    ret = EIO;
> >    goto fail;
> >
> > }
> > DEBUG(SSSDBG_TRACE_FUNC, "NSS Initialization complete\n");
> >
> > return EOK;
> >
> > --
> > 2.4.3
> > From 0c05cef940b8a8650537056db4ade09b0b6a6fa6 Mon Sep 17 00:00:00 2001
> > From: Pavel Reichl 
> > Date: Tue, 10 Nov 2015 10:56:56 -0500
> > Subject: [PATCH 2/2] NSS: Fix memory leak netgroup
> >
> > If netgroup cannot be found in setnetgrent_retry() function
> > set_netgroup_entry() is called which steals getent_ctx directly to
> > nss_ctx->netgroups.
> > Subsequently function lookup_netgr_step() is called that (in case of
> > nenexisting group) will call create_negcache_netgr() which creates
> > a new dummy object to serve as negative cache. While doing so it calls
> > again set_netgroup_entry() for the same netgroup and it calls
> > hash_enter.
> >
> > hash_enter will remove previously hashed entry for netgroup (created in
> > setnetgrent_retry()) from hash table but it won't be freed and thus it
> > leaks.
> >
> > This patch sets netgroup lifetime for netgroups allocated in
> > setnetgrent_retry().
>
> We already set the netgroup lifetime in both the found and notfound case
> in lookup_netgr_step(). If I understand the code correctly, the issue is
> that we create a new negative result in create_negcache_netgr(),
> overwriting the one that we created in setnetgrent_retry(), correct?

Agree.
> Wouldn't it then be better to see if another same object is already in
> the hashtable and free it before replacing?

I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.

I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing. Code got even messier :-(

[Jakub Hrozek]

On Fri, Nov 27, 2015 at 03:54:23PM +0100, Pavel Reichl wrote:
> On 11/27/2015 10:06 AM, Jakub Hrozek wrote:
> > On Wed, Nov 18, 2015 at 03:06:03PM +0100, Pavel Reichl wrote:
> > > Hello,
> > >
> > > attached patch proposes solution for leaking memory when non-existing netgroup is looked up.
> > >
> > > 1st patch is just for testing - just call 'pkill -SIGUSR1 sssd_nss' and talloc report will be generated in /tmp/sssd_nss_talloc_report_full.
> > >
> > > For details about the bug please see commit message in 2nd patch.
> > >
> > > User who reported the bug confirmed that so far it seems that memory leak has been fixed and he didn't report any side effects.
> > >
> > > Thanks!
> > > From abbf720b832beb6d04f909f598e7114a19f72a03 Mon Sep 17 00:00:00 2001
> > > From: Pavel Reichl 
> > > Date: Wed, 30 Sep 2015 07:54:31 -0400
> > > Subject: [PATCH 1/2] talloc_report for nss responder
> > >
> > > src/responder/nss/nsssrv.c | 26 ++++++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > >
> > > diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
> > > index d8eff7968c4929663412aa56d08414689b921a22..79ca9ba89d49a5fd4ee3389e0f881b1a01f9c7a9 100644
> > > --- a/src/responder/nss/nsssrv.c
> > > +++ b/src/responder/nss/nsssrv.c
> > > @@ -403,6 +403,20 @@ static void nss_dp_reconnect_init(struct sbus_connection *conn,
> > >      /* nss_shutdown(rctx); */
> > >  }
> > >
> > > +static void signal_nss_talloc_report(struct tevent_context *ev,
> > >                                 struct tevent_signal *se,
> > >                                 int signum,
> > >                                 int count,
> > >                                 void *siginfo,
> > >                                 void *private_data)
> > >
> > > +{
> > > FILE *f = fopen("/tmp/sssd_nss_talloc_report_full", "w");
> > > if (f != NULL) {
> > >    talloc_report_full(NULL, f);
> > >    fclose(f);
> > > }
> > >
> > > +}
> > >
> > > int nss_process_init(TALLOC_CTX *mem_ctx,
> > >                       struct tevent_context *ev,
> > >                       struct confdb_ctx *cdb)
> > > @@ -417,6 +431,8 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
> > >      int hret;
> > >      int fd_limit;
> > >
> > > talloc_enable_leak_report_full();
> > >
> > > nss_cmds = get_nss_cmds();
> > >
> > > ret = sss_process_init(mem_ctx, ev, cdb,
> > >
> > > @@ -558,6 +574,16 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
> > >          goto fail;
> > >      }
> > >
> > > /* Handle SIGUSR1 to force offline behavior */
> > >
> > > BlockSignals(false, SIGUSR1);
> > >
> > > struct tevent_signal *tes;
> > >
> > > tes = tevent_add_signal(rctx->ev, rctx, SIGUSR1, 0,
> > >                        signal_nss_talloc_report, rctx);
> > >
> > > if (tes == NULL) {
> > >    ret = EIO;
> > >    goto fail;
> > >
> > > }
> > >
> > > DEBUG(SSSDBG_TRACE_FUNC, "NSS Initialization complete\n");
> > >
> > > return EOK;
> > >
> > > --
> > > 2.4.3
> > > From 0c05cef940b8a8650537056db4ade09b0b6a6fa6 Mon Sep 17 00:00:00 2001
> > > From: Pavel Reichl 
> > > Date: Tue, 10 Nov 2015 10:56:56 -0500
> > > Subject: [PATCH 2/2] NSS: Fix memory leak netgroup
> > >
> > > If netgroup cannot be found in setnetgrent_retry() function
> > > set_netgroup_entry() is called which steals getent_ctx directly to
> > > nss_ctx->netgroups.
> > > Subsequently function lookup_netgr_step() is called that (in case of
> > > nenexisting group) will call create_negcache_netgr() which creates
> > > a new dummy object to serve as negative cache. While doing so it calls
> > > again set_netgroup_entry() for the same netgroup and it calls
> > > hash_enter.
> > >
> > > hash_enter will remove previously hashed entry for netgroup (created in
> > > setnetgrent_retry()) from hash table but it won't be freed and thus it
> > > leaks.
> > >
> > > This patch sets netgroup lifetime for netgroups allocated in
> > > setnetgrent_retry().
> >
> > We already set the netgroup lifetime in both the found and notfound case
> > in lookup_netgr_step(). If I understand the code correctly, the issue is
> > that we create a new negative result in create_negcache_netgr(),
> > overwriting the one that we created in setnetgrent_retry(), correct?
>
> Agree.
> > Wouldn't it then be better to see if another same object is already in
> > the hashtable and free it before replacing?
>
> I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
>
> I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> Code got even messier :-(

I think the code would look nice if it was placed in the else branch of
create_negcache_netgr() :-)

I also don't think we need the tmp_ctx change..

[Pavel Reichl]

On 11/30/2015 06:02 PM, Jakub Hrozek wrote:
> > > Wouldn't it then be better to see if another same object is already in
> > > the hashtable and free it before replacing?
> >
> > I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
> >
> > I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> > Code got even messier :-(
>
> I think the code would look nice if it was placed in the else branch of
> create_negcache_netgr() :-)

Done, thanks for hint.
> I also don't think we need the tmp_ctx change..

I dare to disagree here. With prev. versions of patch I was experiencing segfaults. IIRC step_context is being allocated on context of the netgroup that is freed. I also think that it's not a good idea to allocate local data on context that does not 
hold any reference to that data. But it might be matter of personal taste.
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]

[Jakub Hrozek]

On Mon, Nov 30, 2015 at 06:45:00PM +0100, Pavel Reichl wrote:
> On 11/30/2015 06:02 PM, Jakub Hrozek wrote:
> > > > Wouldn't it then be better to see if another same object is already in
> > > > the hashtable and free it before replacing?
> > >
> > > I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
> > >
> > > I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> > > Code got even messier :-(
> >
> > I think the code would look nice if it was placed in the else branch of
> > create_negcache_netgr() :-)
>
> Done, thanks for hint.
> > I also don't think we need the tmp_ctx change..
>
> I dare to disagree here. With prev. versions of patch I was experiencing
> segfaults. IIRC step_context is being allocated on context of the netgroup
> that is freed. I also think that it's not a good idea to allocate local data
> on context that does not hold any reference to that data. But it might be
> matter of personal taste.

What kind of segfaults, what was the backtrace? I think it might be good to
add talloc_zfree() instead of talloc_free() or explicitly NULL the pointer,
that should solve the issue without adding a new context...

Since it's per-domain, I wonder if step_ctx->dctx might be better
candidate than step_ctx either way.

Some more comments inline.
> From 8f2d4cdac0f5bee1847d7b13e904e84e7d706841 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl 
> Date: Fri, 27 Nov 2015 07:53:00 -0500
> Subject: [PATCH] NSS: Fix memory leak netgroup
>
> If netgroup cannot be found in setnetgrent_retry() function
> set_netgroup_entry() is called which steals getent_ctx directly to
> nss_ctx->netgroups.
> Subsequently function lookup_netgr_step() is called that (in case of
> nenexisting group) will call create_negcache_netgr() which creates
> a new dummy object to serve as negative cache. While doing so it calls
> again set_netgroup_entry() for the same netgroup and it calls
> hash_enter.
>
> hash_enter will remove previously hashed entry for netgroup (created in
> setnetgrent_retry()) from hash table but it won't be freed and thus it
> leaks. This patch marks such netgroup and freed it after the call of
> create_negcache_netgr().
>
> Resolves:
> https://fedorahosted.org/sssd/ticket/2865
>
> src/responder/nss/nsssrv_netgroup.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/src/responder/nss/nsssrv_netgroup.c b/src/responder/nss/nsssrv_netgroup.c
> index 9a78c1119c2f4e06e43ebec29ace775adc997e08..27ae1474a70858efb8040b864dd0e21a8bb674de 100644
> --- a/src/responder/nss/nsssrv_netgroup.c
> +++ b/src/responder/nss/nsssrv_netgroup.c
> @@ -434,6 +434,7 @@ static errno_t create_negcache_netgr(struct setent_step_ctx *step_ctx)
>  {
>      errno_t ret;
>      struct getent_ctx *netgr;
>
> struct getent_ctx *netgr_to_be_freed = NULL;
>
> netgr = talloc_zero(step_ctx->nctx, struct getent_ctx);
> if (netgr == NULL) {
>
> @@ -441,6 +442,11 @@ static errno_t create_negcache_netgr(struct setent_step_ctx *step_ctx)
>          ret = ENOMEM;
>          goto done;
>      } else {
>    /* Is there already netgroup with such name? */
>    ret = get_netgroup_entry(step_ctx->nctx, step_ctx->name,
>                         &netgr_to_be_freed);

Bad indenting
>    if (ret != EOK) netgr_to_be_freed = NULL;
>    netgr->ready = true;
>    netgr->found = false;
>    netgr->entries = NULL;
>
> @@ -461,6 +467,9 @@ static errno_t create_negcache_netgr(struct setent_step_ctx *step_ctx)
>      }
>
> done:
> /* Free netgroup after step_ctx is not needed. */
> if (netgr_to_be_freed) talloc_zfree(netgr_to_be_freed);

In new code we should prefer { } even for one-line statements.
> if (ret != EOK) {
>        talloc_free(netgr);
>    }

[Pavel Reichl]

On 12/03/2015 02:06 PM, Jakub Hrozek wrote:
> On Mon, Nov 30, 2015 at 06:45:00PM +0100, Pavel Reichl wrote:
> > On 11/30/2015 06:02 PM, Jakub Hrozek wrote:
> > > > > Wouldn't it then be better to see if another same object is already in
> > > > > the hashtable and free it before replacing?
> > > >
> > > > I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
> > > >
> > > > I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> > > > Code got even messier :-(
> > >
> > > I think the code would look nice if it was placed in the else branch of
> > > create_negcache_netgr() :-)
> >
> > Done, thanks for hint.
> > > I also don't think we need the tmp_ctx change..
> >
> > I dare to disagree here. With prev. versions of patch I was experiencing
> > segfaults. IIRC step_context is being allocated on context of the netgroup
> > that is freed. I also think that it's not a good idea to allocate local data
> > on context that does not hold any reference to that data. But it might be
> > matter of personal taste.
>
> What kind of segfaults, what was the backtrace? I think it might be good to
> add talloc_zfree() instead of talloc_free() or explicitly NULL the pointer,
> that should solve the issue without adding a new context...
>
> Since it's per-domain, I wonder if step_ctx->dctx might be better
> candidate than step_ctx either way.

Hello Jakub, I amended the patch as you proposed. You can now experience the segfault yourself - just query a non-existing netgroup.
I attached the backtrace as well.

I can investiage and use talloc reporting if you wish. But still using tmp_ctx seems as way of the least effort...

[Jakub Hrozek]

On Thu, Dec 03, 2015 at 03:29:22PM +0100, Pavel Reichl wrote:
> On 12/03/2015 02:06 PM, Jakub Hrozek wrote:
> > On Mon, Nov 30, 2015 at 06:45:00PM +0100, Pavel Reichl wrote:
> > > On 11/30/2015 06:02 PM, Jakub Hrozek wrote:
> > > > > > Wouldn't it then be better to see if another same object is already in
> > > > > > the hashtable and free it before replacing?
> > > > >
> > > > > I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
> > > > >
> > > > > I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> > > > > Code got even messier :-(
> > > >
> > > > I think the code would look nice if it was placed in the else branch of
> > > > create_negcache_netgr() :-)
> > >
> > > Done, thanks for hint.
> > > > I also don't think we need the tmp_ctx change..
> > >
> > > I dare to disagree here. With prev. versions of patch I was experiencing
> > > segfaults. IIRC step_context is being allocated on context of the netgroup
> > > that is freed. I also think that it's not a good idea to allocate local data
> > > on context that does not hold any reference to that data. But it might be
> > > matter of personal taste.
> >
> > What kind of segfaults, what was the backtrace? I think it might be good to
> > add talloc_zfree() instead of talloc_free() or explicitly NULL the pointer,
> > that should solve the issue without adding a new context...
> >
> > Since it's per-domain, I wonder if step_ctx->dctx might be better
> > candidate than step_ctx either way.
>
> Hello Jakub, I amended the patch as you proposed. You can now experience the segfault yourself - just query a non-existing netgroup.
> I attached the backtrace as well.
>
> I can investiage and use talloc reporting if you wish. But still using tmp_ctx seems as way of the least effort...

I think you are right, the step_ctx hierarchy is tricky, so the
temporary context looks like the easiest solution. Please do one more
change in the patch:
> From 0160ee92c4d9a9542abfd3e918686526a52e367d Mon Sep 17 00:00:00 2001
> From: Pavel Reichl 
> Date: Fri, 27 Nov 2015 07:53:00 -0500
> Subject: [PATCH] NSS: Fix memory leak netgroup

[...]
> @@ -461,6 +469,11 @@ static errno_t create_negcache_netgr(struct setent_step_ctx *step_ctx)
>      }
>
> done:
> /* Free netgroup after step_ctx is not needed. */
> if (netgr_to_be_freed) {
>    talloc_zfree(netgr_to_be_freed);
> }

Since talloc_free(NULL) is a noop, we should drop the if completely.

[Pavel Reichl]

On 12/04/2015 03:37 PM, Jakub Hrozek wrote:
> On Thu, Dec 03, 2015 at 03:29:22PM +0100, Pavel Reichl wrote:
> > On 12/03/2015 02:06 PM, Jakub Hrozek wrote:
> > > On Mon, Nov 30, 2015 at 06:45:00PM +0100, Pavel Reichl wrote:
> > > > On 11/30/2015 06:02 PM, Jakub Hrozek wrote:
> > > > > > >
> > > > > > > Wouldn't it then be better to see if another same object is already in
> > > > > > > the hashtable and free it before replacing?
> > > > > >
> > > > > > I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
> > > > > >
> > > > > > I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> > > > > > Code got even messier :-(
> > > > >
> > > > > I think the code would look nice if it was placed in the else branch of
> > > > > create_negcache_netgr() :-)
> > > >
> > > > Done, thanks for hint.
> > > > > I also don't think we need the tmp_ctx change..
> > > >
> > > > I dare to disagree here. With prev. versions of patch I was experiencing
> > > > segfaults. IIRC step_context is being allocated on context of the netgroup
> > > > that is freed. I also think that it's not a good idea to allocate local data
> > > > on context that does not hold any reference to that data. But it might be
> > > > matter of personal taste.
> > >
> > > What kind of segfaults, what was the backtrace? I think it might be good to
> > > add talloc_zfree() instead of talloc_free() or explicitly NULL the pointer,
> > > that should solve the issue without adding a new context...
> > >
> > > Since it's per-domain, I wonder if step_ctx->dctx might be better
> > > candidate than step_ctx either way.
> >
> > Hello Jakub, I amended the patch as you proposed. You can now experience the segfault yourself - just query a non-existing netgroup.
> > I attached the backtrace as well.
> >
> > I can investiage and use talloc reporting if you wish. But still using tmp_ctx seems as way of the least effort...
>
> I think you are right, the step_ctx hierarchy is tricky, so the
> temporary context looks like the easiest solution. Please do one more
> change in the patch:

OK, please see attached patch.
> > From 0160ee92c4d9a9542abfd3e918686526a52e367d Mon Sep 17 00:00:00 2001
> > From: Pavel Reichl 
> > Date: Fri, 27 Nov 2015 07:53:00 -0500
> > Subject: [PATCH] NSS: Fix memory leak netgroup
>
> [...]
> > @@ -461,6 +469,11 @@ static errno_t create_negcache_netgr(struct setent_step_ctx *step_ctx)
> >       }
> >
> > done:
> > /* Free netgroup after step_ctx is not needed. */
> > if (netgr_to_be_freed) {
> >    talloc_zfree(netgr_to_be_freed);
> > }
>
> Since talloc_free(NULL) is a noop, we should drop the if completely.

OK, done.
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]

[Jakub Hrozek]

On Fri, Dec 04, 2015 at 05:18:53PM +0100, Pavel Reichl wrote:
> On 12/04/2015 03:37 PM, Jakub Hrozek wrote:
> > On Thu, Dec 03, 2015 at 03:29:22PM +0100, Pavel Reichl wrote:
> > > On 12/03/2015 02:06 PM, Jakub Hrozek wrote:
> > > > On Mon, Nov 30, 2015 at 06:45:00PM +0100, Pavel Reichl wrote:
> > > > > On 11/30/2015 06:02 PM, Jakub Hrozek wrote:
> > > > > > >>
> > > > > > >>Wouldn't it then be better to see if another same object is already in
> > > > > > >>the hashtable and free it before replacing?
> > > > > > >
> > > > > > >I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
> > > > > > >
> > > > > > >I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> > > > > >
> > > > > > >Code got even messier :-(
> > > > > >
> > > > > > I think the code would look nice if it was placed in the else branch of
> > > > > > create_negcache_netgr() :-)
> > > > >
> > > > > Done, thanks for hint.
> > > > > > I also don't think we need the tmp_ctx change..
> > > > >
> > > > > I dare to disagree here. With prev. versions of patch I was experiencing
> > > > > segfaults. IIRC step_context is being allocated on context of the netgroup
> > > > > that is freed. I also think that it's not a good idea to allocate local data
> > > > > on context that does not hold any reference to that data. But it might be
> > > > > matter of personal taste.
> > > >
> > > > What kind of segfaults, what was the backtrace? I think it might be good to
> > > > add talloc_zfree() instead of talloc_free() or explicitly NULL the pointer,
> > > > that should solve the issue without adding a new context...
> > > >
> > > > Since it's per-domain, I wonder if step_ctx->dctx might be better
> > > > candidate than step_ctx either way.
> > >
> > > Hello Jakub, I amended the patch as you proposed. You can now experience the segfault yourself - just query a non-existing netgroup.
> > > I attached the backtrace as well.
> > >
> > > I can investiage and use talloc reporting if you wish. But still using tmp_ctx seems as way of the least effort...
> >
> > I think you are right, the step_ctx hierarchy is tricky, so the
> > temporary context looks like the easiest solution. Please do one more
> > change in the patch:
>
> OK, please see attached patch.
> > > From 0160ee92c4d9a9542abfd3e918686526a52e367d Mon Sep 17 00:00:00 2001
> > > From: Pavel Reichl 
> > > Date: Fri, 27 Nov 2015 07:53:00 -0500
> > > Subject: [PATCH] NSS: Fix memory leak netgroup
> >
> > [...]
> > > @@ -461,6 +469,11 @@ static errno_t create_negcache_netgr(struct setent_step_ctx *step_ctx)
> > >      }
> > >
> > > done:
> > > /* Free netgroup after step_ctx is not needed. */
> > > if (netgr_to_be_freed) {
> > >    talloc_zfree(netgr_to_be_freed);
> > > }
> >
> > Since talloc_free(NULL) is a noop, we should drop the if completely.
>
> OK, done.

While testing the patches I found that sometimes (not always, thought) I
see this valgrind error when a non-existing netgroup is requested:
Mon Dec  7 14:53:15 2015) [sssd[nss]] [accept_fd_handler] (0x0400): Client connected!
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Received client version [1].
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Offered version [1].
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_parse_name_for_domains] (0x0200): name 'ngr' matched without domain, user is ngr
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [setnetgrent_send] (0x0100): Requesting info for netgroup [ngr] from [<ALL>]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0040): No results for netgroup ngr (domain ipa.test)
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [get_dp_name_and_id] (0x0400): Not a LOCAL view, continuing with provided values.
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_issue_request] (0x0400): Issuing request for [0x428c3b:4:[e-mail]]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_get_account_msg] (0x0400): Creating request for [ipa.test][0x1004][FAST BE_REQ_NETGROUP][1][name=ngr]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_internal_get_send] (0x0400): Entering request [0x428c3b:4:[e-mail]]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_get_reply] (0x1000): Got reply from Data Provider - DP error code: 0 errno: 0 error message: Success
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0400): Returning info for netgroup [[e-mail]]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_req_destructor] (0x0400): Deleting request: [0x428c3b:4:[e-mail]]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent] (0x0100): Requesting netgroup data
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent] (0x0400): Returning results for [ngr]
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent_process] (0x0200): No entries found
(Mon Dec  7 14:53:15 2015) [sssd[nss]] [client_recv] (0x0200): Client disconnected!
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [accept_fd_handler] (0x0400): Client connected!
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Received client version [1].
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Offered version [1].
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_parse_name_for_domains] (0x0200): name 'ngr2' matched without domain, user is ngr2
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [setnetgrent_send] (0x0100): Requesting info for netgroup [ngr2] from [<ALL>]
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0040): No results for netgroup ngr2 (domain ipa.test)
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [get_dp_name_and_id] (0x0400): Not a LOCAL view, continuing with provided values.
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_issue_request] (0x0400): Issuing request for [0x428c3b:4:[e-mail]]
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_get_account_msg] (0x0400): Creating request for [ipa.test][0x1004][FAST BE_REQ_NETGROUP][1][name=ngr2]
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_internal_get_send] (0x0400): Entering request [0x428c3b:4:[e-mail]]
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_get_reply] (0x1000): Got reply from Data Provider - DP error code: 0 errno: 0 error message: Success
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0080): No matching domain found for [ngr2], fail!
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [netgr_hash_remove] (0x1000): netgroup [ngr2] was already removed
==6830== Invalid read of size 8
==6830==    at 0x42469E: setent_notify (responder_cmd.c:218)
==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==  Block was alloc'd at
==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
==6830==    by 0x42547C: client_recv (responder_common.c:283)
==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
==6830== Invalid read of size 8
==6830==    at 0x42442B: setent_remove_ref (responder_cmd.c:188)
==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==  Block was alloc'd at
==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
==6830==    by 0x42547C: client_recv (responder_common.c:283)
==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
==6830== Invalid write of size 8
==6830==    at 0x424444: setent_remove_ref (responder_cmd.c:188)
==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==  Block was alloc'd at
==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
==6830==    by 0x42547C: client_recv (responder_common.c:283)
==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
==6830== 
==6830== Invalid read of size 8
==6830==    at 0x42444F: setent_remove_ref (responder_cmd.c:188)
==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==  Block was alloc'd at
==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
==6830==    by 0x42547C: client_recv (responder_common.c:283)
==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
==6830== 
==6830== Invalid read of size 8
==6830==    at 0x4244B1: setent_remove_ref (responder_cmd.c:188)
==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==  Block was alloc'd at
==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
==6830==    by 0x42547C: client_recv (responder_common.c:283)
==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
==6830== Invalid read of size 8
==6830==    at 0x424633: setent_notify (responder_cmd.c:228)
==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==  Block was alloc'd at
==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
==6830==    by 0x42547C: client_recv (responder_common.c:283)
==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
==6830== 
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_req_destructor] (0x0400): Deleting request: [0x428c3b:4:ng
(Mon Dec  7 14:53:20 2015) [sssd[nss]] [client_recv] (0x0200): Client disconnected!

[Pavel Reichl]

On 12/07/2015 03:59 PM, Jakub Hrozek wrote:
> On Fri, Dec 04, 2015 at 05:18:53PM +0100, Pavel Reichl wrote:
> > On 12/04/2015 03:37 PM, Jakub Hrozek wrote:
> > > On Thu, Dec 03, 2015 at 03:29:22PM +0100, Pavel Reichl wrote:
> > > > On 12/03/2015 02:06 PM, Jakub Hrozek wrote:
> > > > > On Mon, Nov 30, 2015 at 06:45:00PM +0100, Pavel Reichl wrote:
> > > > > > On 11/30/2015 06:02 PM, Jakub Hrozek wrote:
> > > > > > >>>
> > > > > > >>> Wouldn't it then be better to see if another same object is already in
> > > > > > >>> the hashtable and free it before replacing?
> > > > > > >>
> > > > > > >> I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
> > > > > > >>
> > > > > > >> I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> > > > > > >
> > > > > > >> Code got even messier :-(
> > > > > > >
> > > > > > > I think the code would look nice if it was placed in the else branch of
> > > > > > > create_negcache_netgr() :-)
> > > > > >
> > > > > > Done, thanks for hint.
> > > > > >
> > > > > > >
> > > > > > > I also don't think we need the tmp_ctx change..
> > > > > >
> > > > > > I dare to disagree here. With prev. versions of patch I was experiencing
> > > > > > segfaults. IIRC step_context is being allocated on context of the netgroup
> > > > > > that is freed. I also think that it's not a good idea to allocate local data
> > > > > > on context that does not hold any reference to that data. But it might be
> > > > > > matter of personal taste.
> > > > >
> > > > > What kind of segfaults, what was the backtrace? I think it might be good to
> > > > > add talloc_zfree() instead of talloc_free() or explicitly NULL the pointer,
> > > > > that should solve the issue without adding a new context...
> > > > >
> > > > > Since it's per-domain, I wonder if step_ctx->dctx might be better
> > > > > candidate than step_ctx either way.
> > > >
> > > > Hello Jakub, I amended the patch as you proposed. You can now experience the segfault yourself - just query a non-existing netgroup.
> > > > I attached the backtrace as well.
> > > >
> > > > I can investiage and use talloc reporting if you wish. But still using tmp_ctx seems as way of the least effort...
> > >
> > > I think you are right, the step_ctx hierarchy is tricky, so the
> > > temporary context looks like the easiest solution. Please do one more
> > > change in the patch:
> >
> > OK, please see attached patch.
> > > > From 0160ee92c4d9a9542abfd3e918686526a52e367d Mon Sep 17 00:00:00 2001
> > > > From: Pavel Reichl 
> > > > Date: Fri, 27 Nov 2015 07:53:00 -0500
> > > > Subject: [PATCH] NSS: Fix memory leak netgroup
> > >
> > > [...]
> > > > @@ -461,6 +469,11 @@ static errno_t create_negcache_netgr(struct setent_step_ctx *step_ctx)
> > > >       }
> > > >
> > > > done:
> > > > /* Free netgroup after step_ctx is not needed. */
> > > > if (netgr_to_be_freed) {
> > > >    talloc_zfree(netgr_to_be_freed);
> > > > }
> > >
> > > Since talloc_free(NULL) is a noop, we should drop the if completely.
> >
> > OK, done.
>
> While testing the patches I found that sometimes (not always, thought) I
> see this valgrind error when a non-existing netgroup is requested:
> Mon Dec  7 14:53:15 2015) [sssd[nss]] [accept_fd_handler] (0x0400): Client connected!
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Received client version [1].
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Offered version [1].
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_parse_name_for_domains] (0x0200): name 'ngr' matched without domain, user is ngr
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [setnetgrent_send] (0x0100): Requesting info for netgroup [ngr] from [<ALL>]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0040): No results for netgroup ngr (domain ipa.test)
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [get_dp_name_and_id] (0x0400): Not a LOCAL view, continuing with provided values.
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_issue_request] (0x0400): Issuing request for [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_get_account_msg] (0x0400): Creating request for [ipa.test][0x1004][FAST BE_REQ_NETGROUP][1][name=ngr]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_internal_get_send] (0x0400): Entering request [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_get_reply] (0x1000): Got reply from Data Provider - DP error code: 0 errno: 0 error message: Success
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0400): Returning info for netgroup [[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_req_destructor] (0x0400): Deleting request: [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent] (0x0100): Requesting netgroup data
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent] (0x0400): Returning results for [ngr]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent_process] (0x0200): No entries found
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [client_recv] (0x0200): Client disconnected!
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [accept_fd_handler] (0x0400): Client connected!
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Received client version [1].
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Offered version [1].
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_parse_name_for_domains] (0x0200): name 'ngr2' matched without domain, user is ngr2
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [setnetgrent_send] (0x0100): Requesting info for netgroup [ngr2] from [<ALL>]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0040): No results for netgroup ngr2 (domain ipa.test)
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [get_dp_name_and_id] (0x0400): Not a LOCAL view, continuing with provided values.
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_issue_request] (0x0400): Issuing request for [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_get_account_msg] (0x0400): Creating request for [ipa.test][0x1004][FAST BE_REQ_NETGROUP][1][name=ngr2]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_internal_get_send] (0x0400): Entering request [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_get_reply] (0x1000): Got reply from Data Provider - DP error code: 0 errno: 0 error message: Success
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0080): No matching domain found for [ngr2], fail!
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [netgr_hash_remove] (0x1000): netgroup [ngr2] was already removed
> ==6830== Invalid read of size 8
> ==6830==    at 0x42469E: setent_notify (responder_cmd.c:218)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830== Invalid read of size 8
> ==6830==    at 0x42442B: setent_remove_ref (responder_cmd.c:188)
> ==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
> ==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830== Invalid write of size 8
> ==6830==    at 0x424444: setent_remove_ref (responder_cmd.c:188)
> ==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
> ==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==
> ==6830== Invalid read of size 8
> ==6830==    at 0x42444F: setent_remove_ref (responder_cmd.c:188)
> ==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
> ==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==
> ==6830== Invalid read of size 8
> ==6830==    at 0x4244B1: setent_remove_ref (responder_cmd.c:188)
> ==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
> ==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830== Invalid read of size 8
> ==6830==    at 0x424633: setent_notify (responder_cmd.c:228)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_req_destructor] (0x0400): Deleting request: [0x428c3b:4:ng
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [client_recv] (0x0200): Client disconnected!
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]

Nice :-(.

Thanks for noticing. Shall we return to the first iteration of the patch?

[Jakub Hrozek]

On Mon, Dec 07, 2015 at 04:11:33PM +0100, Pavel Reichl wrote:
> Nice :-(.
>
> Thanks for noticing. Shall we return to the first iteration of the patch?

Do you know why this happens and why would the first iteration be
better? I don't see from the code what's wrong..

[Pavel Reichl]

On 12/07/2015 03:59 PM, Jakub Hrozek wrote:
> On Fri, Dec 04, 2015 at 05:18:53PM +0100, Pavel Reichl wrote:
> > On 12/04/2015 03:37 PM, Jakub Hrozek wrote:
> > > On Thu, Dec 03, 2015 at 03:29:22PM +0100, Pavel Reichl wrote:
> > > > On 12/03/2015 02:06 PM, Jakub Hrozek wrote:
> > > > > On Mon, Nov 30, 2015 at 06:45:00PM +0100, Pavel Reichl wrote:
> > > > > > On 11/30/2015 06:02 PM, Jakub Hrozek wrote:
> > > > > > >>>
> > > > > > >>> Wouldn't it then be better to see if another same object is already in
> > > > > > >>> the hashtable and free it before replacing?
> > > > > > >>
> > > > > > >> I agree it would be best. I tried that before and failed because I could not decipher out the relation of talloc contexts.
> > > > > > >>
> > > > > > >> I tried that again. Seems that leaks are gone. Segfaults were not happening during my testing.
> > > > > > >
> > > > > > >> Code got even messier :-(
> > > > > > >
> > > > > > > I think the code would look nice if it was placed in the else branch of
> > > > > > > create_negcache_netgr() :-)
> > > > > >
> > > > > > Done, thanks for hint.
> > > > > >
> > > > > > >
> > > > > > > I also don't think we need the tmp_ctx change..
> > > > > >
> > > > > > I dare to disagree here. With prev. versions of patch I was experiencing
> > > > > > segfaults. IIRC step_context is being allocated on context of the netgroup
> > > > > > that is freed. I also think that it's not a good idea to allocate local data
> > > > > > on context that does not hold any reference to that data. But it might be
> > > > > > matter of personal taste.
> > > > >
> > > > > What kind of segfaults, what was the backtrace? I think it might be good to
> > > > > add talloc_zfree() instead of talloc_free() or explicitly NULL the pointer,
> > > > > that should solve the issue without adding a new context...
> > > > >
> > > > > Since it's per-domain, I wonder if step_ctx->dctx might be better
> > > > > candidate than step_ctx either way.
> > > >
> > > > Hello Jakub, I amended the patch as you proposed. You can now experience the segfault yourself - just query a non-existing netgroup.
> > > > I attached the backtrace as well.
> > > >
> > > > I can investiage and use talloc reporting if you wish. But still using tmp_ctx seems as way of the least effort...
> > >
> > > I think you are right, the step_ctx hierarchy is tricky, so the
> > > temporary context looks like the easiest solution. Please do one more
> > > change in the patch:
> >
> > OK, please see attached patch.
> > > > From 0160ee92c4d9a9542abfd3e918686526a52e367d Mon Sep 17 00:00:00 2001
> > > > From: Pavel Reichl 
> > > > Date: Fri, 27 Nov 2015 07:53:00 -0500
> > > > Subject: [PATCH] NSS: Fix memory leak netgroup
> > >
> > > [...]
> > > > @@ -461,6 +469,11 @@ static errno_t create_negcache_netgr(struct setent_step_ctx *step_ctx)
> > > >       }
> > > >
> > > > done:
> > > > /* Free netgroup after step_ctx is not needed. */
> > > > if (netgr_to_be_freed) {
> > > >    talloc_zfree(netgr_to_be_freed);
> > > > }
> > >
> > > Since talloc_free(NULL) is a noop, we should drop the if completely.
> >
> > OK, done.
>
> While testing the patches I found that sometimes (not always, thought) I
> see this valgrind error when a non-existing netgroup is requested:
> Mon Dec  7 14:53:15 2015) [sssd[nss]] [accept_fd_handler] (0x0400): Client connected!
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Received client version [1].
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Offered version [1].
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_parse_name_for_domains] (0x0200): name 'ngr' matched without domain, user is ngr
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [setnetgrent_send] (0x0100): Requesting info for netgroup [ngr] from [<ALL>]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0040): No results for netgroup ngr (domain ipa.test)
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [get_dp_name_and_id] (0x0400): Not a LOCAL view, continuing with provided values.
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_issue_request] (0x0400): Issuing request for [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_get_account_msg] (0x0400): Creating request for [ipa.test][0x1004][FAST BE_REQ_NETGROUP][1][name=ngr]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_internal_get_send] (0x0400): Entering request [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_get_reply] (0x1000): Got reply from Data Provider - DP error code: 0 errno: 0 error message: Success
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [lookup_netgr_step] (0x0400): Returning info for netgroup [[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [sss_dp_req_destructor] (0x0400): Deleting request: [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent] (0x0100): Requesting netgroup data
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent] (0x0400): Returning results for [ngr]
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [nss_cmd_getnetgrent_process] (0x0200): No entries found
> (Mon Dec  7 14:53:15 2015) [sssd[nss]] [client_recv] (0x0200): Client disconnected!
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [accept_fd_handler] (0x0400): Client connected!
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Received client version [1].
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_cmd_get_version] (0x0200): Offered version [1].
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_parse_name_for_domains] (0x0200): name 'ngr2' matched without domain, user is ngr2
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [setnetgrent_send] (0x0100): Requesting info for netgroup [ngr2] from [<ALL>]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0040): No results for netgroup ngr2 (domain ipa.test)
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [get_dp_name_and_id] (0x0400): Not a LOCAL view, continuing with provided values.
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_issue_request] (0x0400): Issuing request for [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_get_account_msg] (0x0400): Creating request for [ipa.test][0x1004][FAST BE_REQ_NETGROUP][1][name=ngr2]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_internal_get_send] (0x0400): Entering request [0x428c3b:4:[e-mail]]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_get_reply] (0x1000): Got reply from Data Provider - DP error code: 0 errno: 0 error message: Success
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0100): Requesting info for [[e-mail]]
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [lookup_netgr_step] (0x0080): No matching domain found for [ngr2], fail!
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [netgr_hash_remove] (0x1000): netgroup [ngr2] was already removed
> ==6830== Invalid read of size 8
> ==6830==    at 0x42469E: setent_notify (responder_cmd.c:218)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830== Invalid read of size 8
> ==6830==    at 0x42442B: setent_remove_ref (responder_cmd.c:188)
> ==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
> ==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830== Invalid write of size 8
> ==6830==    at 0x424444: setent_remove_ref (responder_cmd.c:188)
> ==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
> ==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==
> ==6830== Invalid read of size 8
> ==6830==    at 0x42444F: setent_remove_ref (responder_cmd.c:188)
> ==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
> ==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==
> ==6830== Invalid read of size 8
> ==6830==    at 0x4244B1: setent_remove_ref (responder_cmd.c:188)
> ==6830==    by 0x89E8492: ??? (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x89E210A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x87D532F: tevent_req_received (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D5368: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x89E2707: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x418CE9: nss_cmd_setnetgrent_done (nsssrv_netgroup.c:722)
> ==6830==    by 0x87D51C1: _tevent_req_error (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x42462E: setent_notify (responder_cmd.c:225)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830== Invalid read of size 8
> ==6830==    at 0x424633: setent_notify (responder_cmd.c:228)
> ==6830==    by 0x406ED2: nss_setent_notify_error (nsssrv_cmd.c:94)
> ==6830==    by 0x418C0D: lookup_netgr_dp_callback (nsssrv_netgroup.c:681)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Address 0xbcd03e0 is 112 bytes inside a block of size 160 free'd
> ==6830==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E2262: _talloc_free (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x4182C9: create_negcache_netgr (nsssrv_netgroup.c:472)
> ==6830==    by 0x418A08: lookup_netgr_step (nsssrv_netgroup.c:632)
> ==6830==    by 0x418BD7: lookup_netgr_dp_callback (nsssrv_netgroup.c:672)
> ==6830==    by 0x4094D7: nsssrv_dp_send_acct_req_done (nsssrv_cmd.c:885)
> ==6830==    by 0x42969D: sss_dp_internal_get_done (responder_dp.c:802)
> ==6830==    by 0x570C9E1: ??? (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5710400: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.14.6)
> ==6830==    by 0x5080E59: sbus_dispatch (sssd_dbus_connection.c:96)
> ==6830==    by 0x87D81DC: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D9209: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==  Block was alloc'd at
> ==6830==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==6830==    by 0x89E4965: _talloc_zero (in /usr/lib64/libtalloc.so.2.1.3)
> ==6830==    by 0x417D3C: setnetgrent_retry (nsssrv_netgroup.c:320)
> ==6830==    by 0x417B42: setnetgrent_send (nsssrv_netgroup.c:248)
> ==6830==    by 0x41764D: nss_cmd_setnetgrent (nsssrv_netgroup.c:129)
> ==6830==    by 0x4243C9: sss_cmd_execute (responder_cmd.c:161)
> ==6830==    by 0x42527A: client_cmd_execute (responder_common.c:249)
> ==6830==    by 0x42547C: client_recv (responder_common.c:283)
> ==6830==    by 0x4256B9: client_fd_handler (responder_common.c:335)
> ==6830==    by 0x87D93C2: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D7906: ??? (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==    by 0x87D411C: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.25)
> ==6830==
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [sss_dp_req_destructor] (0x0400): Deleting request: [0x428c3b:4:ng
> (Mon Dec  7 14:53:20 2015) [sssd[nss]] [client_recv] (0x0200): Client disconnected!
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]

Hello,

attached patch does not seem to suffer from these errors any more.

Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...

Thanks.

[Jakub Hrozek]

On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> Hello,
>
> attached patch does not seem to suffer from these errors any more.
>
> Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
>
> Thanks.

Yes, please. We can do the review in the meantime.

[Jakub Hrozek]

On Mon, Jan 25, 2016 at 08:54:53PM +0100, Jakub Hrozek wrote:
> On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > Hello,
> >
> > attached patch does not seem to suffer from these errors any more.
> >
> > Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> >
> > Thanks.
>
> Yes, please. We can do the review in the meantime.

The new code looks good to me and there is definitely improvement in
memory consumption. Before the patch, after requesting 100000 netgroups
the memory usage went from ~30 to ~250MBs. After the patch, the memory
went from ~30 to ~50 MBs.

Did you take a look in your testing what is the extra 20MBs or should I?

[Pavel Reichl]

On 01/28/2016 12:28 PM, Jakub Hrozek wrote:
> On Mon, Jan 25, 2016 at 08:54:53PM +0100, Jakub Hrozek wrote:
> > On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > > Hello,
> > >
> > > attached patch does not seem to suffer from these errors any more.
> > >
> > > Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> > >
> > > Thanks.
> >
> > Yes, please. We can do the review in the meantime.
>
> The new code looks good to me and there is definitely improvement in
> memory consumption. Before the patch, after requesting 100000 netgroups
> the memory usage went from ~30 to ~250MBs. After the patch, the memory
> went from ~30 to ~50 MBs.
>
> Did you take a look in your testing what is the extra 20MBs or should I?

I would expect that this memory is occupied by dummy records to speed up negative queries...but these records should be freed in final time...how long have you been waiting after your 'massive' query?
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]

[Pavel Reichl]

On 01/28/2016 12:28 PM, Jakub Hrozek wrote:
> On Mon, Jan 25, 2016 at 08:54:53PM +0100, Jakub Hrozek wrote:
> > On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > > Hello,
> > >
> > > attached patch does not seem to suffer from these errors any more.
> > >
> > > Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> > >
> > > Thanks.
> >
> > Yes, please. We can do the review in the meantime.
>
> The new code looks good to me and there is definitely improvement in
> memory consumption. Before the patch, after requesting 100000 netgroups
> the memory usage went from ~30 to ~250MBs. After the patch, the memory
> went from ~30 to ~50 MBs.
>
> Did you take a look in your testing what is the extra 20MBs or should I?
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]

How exactly did you measure the memory use? In my testing I used talloc_full_report (the patch was attached in 1st version of patch). I rerun query for 100000 netgroups, right after the test talloc reported 'full talloc report on 'null_context' 
(total 1738774 bytes in 48682 blocks)', a few minutes after the test finished I got: 'full talloc report on 'null_context' (total  16258 bytes in 417 blocks)'.

I'm not seeing any such leaks as you observed.

[Pavel Reichl]

On 01/25/2016 08:54 PM, Jakub Hrozek wrote:
> On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > Hello,
> >
> > attached patch does not seem to suffer from these errors any more.
> >
> > Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> >
> > Thanks.
>
> Yes, please. We can do the review in the meantime.
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]

User just confirmed that packages seem to be running fine on the test host, no memory usage growth.

[Jakub Hrozek]

On Tue, Feb 02, 2016 at 04:06:37PM +0100, Pavel Reichl wrote:
> On 01/25/2016 08:54 PM, Jakub Hrozek wrote:
> > On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > > Hello,
> > >
> > > attached patch does not seem to suffer from these errors any more.
> > >
> > > Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> > >
> > > Thanks.
> >
> > Yes, please. We can do the review in the meantime.
> > _______________________________________________
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/admin/lists/[e-mail]
>
> User just confirmed that packages seem to be running fine on the test host, no memory usage growth.

Right and with the help of the talloc patch in the first mail I've also
verified that the talloc usage stays the same.

ACK

[Jakub Hrozek]

On Thu, Feb 04, 2016 at 03:52:35PM +0100, Jakub Hrozek wrote:
> On Tue, Feb 02, 2016 at 04:06:37PM +0100, Pavel Reichl wrote:
> > On 01/25/2016 08:54 PM, Jakub Hrozek wrote:
> > > On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > > > Hello,
> > > >
> > > > attached patch does not seem to suffer from these errors any more.
> > > >
> > > > Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> > > >
> > > > Thanks.
> > >
> > > Yes, please. We can do the review in the meantime.
> > > _______________________________________________
> > > sssd-devel mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/admin/lists/[e-mail]
> >
> > User just confirmed that packages seem to be running fine on the test host, no memory usage growth.
>
> Right and with the help of the talloc patch in the first mail I've also
> verified that the talloc usage stays the same.
>
> ACK

* master: 4231a17e66e0809a9c3d42207b45f95429cbb46c
* sssd-1-13: 3bca87239e3368d61c25f2f6bd2329191eca0dee

[Lukas Slebodnik]

On (04/02/16 16:22), Jakub Hrozek wrote:
> On Thu, Feb 04, 2016 at 03:52:35PM +0100, Jakub Hrozek wrote:
> > On Tue, Feb 02, 2016 at 04:06:37PM +0100, Pavel Reichl wrote:
> > > On 01/25/2016 08:54 PM, Jakub Hrozek wrote:
> > > > On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > > > > Hello,
> > > > >
> > > > > attached patch does not seem to suffer from these errors any more.
> > > > >
> > > > > Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> > > > >
> > > > > Thanks.
> > > >
> > > > Yes, please. We can do the review in the meantime.
> > >
> > > User just confirmed that packages seem to be running fine on the test host, no memory usage growth.
> >
> > Right and with the help of the talloc patch in the first mail I've also
> > verified that the talloc usage stays the same.
> >
> > ACK
> master: 4231a17e66e0809a9c3d42207b45f95429cbb46c
> sssd-1-13: 3bca87239e3368d61c25f2f6bd2329191eca0dee

sssd-1-12:
* 4a6cae7d917a4c8e1a02cfaf5d27a021dd6545c1

LS

[Lukas Slebodnik]

On (10/02/16 10:31), Lukas Slebodnik wrote:
> On (04/02/16 16:22), Jakub Hrozek wrote:
> > On Thu, Feb 04, 2016 at 03:52:35PM +0100, Jakub Hrozek wrote:
> > > On Tue, Feb 02, 2016 at 04:06:37PM +0100, Pavel Reichl wrote:
> > > > On 01/25/2016 08:54 PM, Jakub Hrozek wrote:
> > > > > On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > > > > > Hello,
> > > > > >
> > > > > > attached patch does not seem to suffer from these errors any more.
> > > > > >
> > > > > > Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> > > > > >
> > > > > > Thanks.
> > > > >
> > > > > Yes, please. We can do the review in the meantime.
> > > >
> > > > User just confirmed that packages seem to be running fine on the test host, no memory usage growth.
> > >
> > > Right and with the help of the talloc patch in the first mail I've also
> > > verified that the talloc usage stays the same.
> > >
> > > ACK
> > master: 4231a17e66e0809a9c3d42207b45f95429cbb46c
> > sssd-1-13: 3bca87239e3368d61c25f2f6bd2329191eca0dee
>
> sssd-1-12:
> 4a6cae7d917a4c8e1a02cfaf5d27a021dd6545c1

There were some conflicts on 1-11
Attached are backported patch + dependencies for 1.11 branch.

Changes are bigger than I expected therefore I executed
tests I'm not sure when they will finish.
Meanwhile it would be good to review/(cross-check) bacported
version of patches.

LS

[Jakub Hrozek]

On Mon, Feb 15, 2016 at 01:59:27PM +0100, Lukas Slebodnik wrote:
> On (10/02/16 10:31), Lukas Slebodnik wrote:
> > On (04/02/16 16:22), Jakub Hrozek wrote:
> > > On Thu, Feb 04, 2016 at 03:52:35PM +0100, Jakub Hrozek wrote:
> > > > On Tue, Feb 02, 2016 at 04:06:37PM +0100, Pavel Reichl wrote:
> > > > > On 01/25/2016 08:54 PM, Jakub Hrozek wrote:
> > > > > > On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > > > > > >Hello,
> > > > > > >
> > > > > > >attached patch does not seem to suffer from these errors any more.
> > > > > > >
> > > > > > >Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> > > > > > >
> > > > > > >Thanks.
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > Yes, please. We can do the review in the meantime.
> > > > >
> > > > > User just confirmed that packages seem to be running fine on the test host, no memory usage growth.
> > > >
> > > > Right and with the help of the talloc patch in the first mail I've also
> > > > verified that the talloc usage stays the same.
> > > >
> > > > ACK
> > > master: 4231a17e66e0809a9c3d42207b45f95429cbb46c
> > > sssd-1-13: 3bca87239e3368d61c25f2f6bd2329191eca0dee
> >
> > sssd-1-12:
> > 4a6cae7d917a4c8e1a02cfaf5d27a021dd6545c1
>
> There were some conflicts on 1-11
> Attached are backported patch + dependencies for 1.11 branch.
>
> Changes are bigger than I expected therefore I executed
> tests I'm not sure when they will finish.
> Meanwhile it would be good to review/(cross-check) bacported
> version of patches.
>
> LS

I only inspected the diffs, but they look OK to me, please push..

[Lukas Slebodnik]

On (15/02/16 18:30), Jakub Hrozek wrote:
> On Mon, Feb 15, 2016 at 01:59:27PM +0100, Lukas Slebodnik wrote:
> > On (10/02/16 10:31), Lukas Slebodnik wrote:
> > > On (04/02/16 16:22), Jakub Hrozek wrote:
> > > > On Thu, Feb 04, 2016 at 03:52:35PM +0100, Jakub Hrozek wrote:
> > > > > On Tue, Feb 02, 2016 at 04:06:37PM +0100, Pavel Reichl wrote:
> > > > > > On 01/25/2016 08:54 PM, Jakub Hrozek wrote:
> > > > > > >On Mon, Jan 25, 2016 at 05:16:37PM +0100, Pavel Reichl wrote:
> > > > > > >>Hello,
> > > > > > >>
> > > > > > >>attached patch does not seem to suffer from these errors any more.
> > > > > > >>
> > > > > > >>Shall I ask user who reported the bug If he is willing to test this new version of the patch? IIRC he needs more then a week for a testing to be conclusive...
> > > > > > >>
> > > > > > >>Thanks.
> > > > > > >>
> > > > > > >>
> > > > > > >
> > > > > > >Yes, please. We can do the review in the meantime.
> > > > > > >
> > > > > >
> > > > > > User just confirmed that packages seem to be running fine on the test host, no memory usage growth.
> > > > >
> > > > > Right and with the help of the talloc patch in the first mail I've also
> > > > > verified that the talloc usage stays the same.
> > > > >
> > > > > ACK
> > > > master: 4231a17e66e0809a9c3d42207b45f95429cbb46c
> > > > sssd-1-13: 3bca87239e3368d61c25f2f6bd2329191eca0dee
> > >
> > > sssd-1-12:
> > > 4a6cae7d917a4c8e1a02cfaf5d27a021dd6545c1
> >
> > There were some conflicts on 1-11
> > Attached are backported patch + dependencies for 1.11 branch.
> >
> > Changes are bigger than I expected therefore I executed
> > tests I'm not sure when they will finish.
> > Meanwhile it would be good to review/(cross-check) bacported
> > version of patches.
> >
> > LS
>
> I only inspected the diffs, but they look OK to me, please push..

Test passed even with valgrind :-)

sssd-1-11:
* 5232e1e7e3267be04f9bdee614b5f81367050fbd
* af0834223167694bdec7964c451d174213fba1e1
* e25867df505ef5cb2b3843c1a859337782f13383
* e78fc9b264e14058607ee2af6d3830c694a39ff5

LS