On Thu, Jun 27, 2013 at 08:00:02PM +0200, Jakub Hrozek wrote: > On Wed, Jun 26, 2013 at 10:42:01AM +0200, Lukas Slebodnik wrote: > > On (25/06/13 15:16), Jakub Hrozek wrote: > > >On Tue, Jun 25, 2013 at 10:36:14AM +0200, Lukas Slebodnik wrote: > > >> ehlo, > > >> > > >> Attached patches should fix https://fedorahosted.org/sssd/ticket/1980 > > >> > > >> The first patch adds check after sysdb_getnetgr. If sysdb_getnetgr > > >> returns more > > >> result than 1, sssd will return error. sysdb_getpwnam has already had > > >> this check. > > >> > > >> The second patch removes function call sss_cmd_done inside of > > >> check_cache, > > >> because function is sss_cmd_done is called in parent functions. > > >> This was a reason of sssd crash. > > >> > > >> How to reproduce this crash. > > >> 1.Add Netgroup to sysdb cache with base cn=Netgroups,cn=<domain>,cn=sysdb > > >> This netgroup should have the same attribute (name or nameAlias or > > >> memberOf) > > >> as another netgroup. > > >> 2. call sudo with user, which is member of ^^^ netgroup. > > >> > > >> Those patches fix only sssd crash, but we should find out: > > >> Why were those netgroups stored in sysdb. > > >> > > >> LS > > > > > >> From 5877db33fc0d52cf31f8b052a8a10ed16a307b0c Mon Sep 17 00:00:00 2001 > > >> From: Lukas Slebodnik <lsleb...@redhat.com> > > >> Date: Tue, 25 Jun 2013 09:01:45 +0200 > > >> Subject: [PATCH 1/2] Handle too many results from getnetgr. > > >> > > >> --- > > >> src/responder/nss/nsssrv_netgroup.c | 14 +++++++++++++- > > >> 1 file changed, 13 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/src/responder/nss/nsssrv_netgroup.c > > >> b/src/responder/nss/nsssrv_netgroup.c > > >> index > > >> 4ec4161cf9c043792fffede1aa95acaf929a7071..12be52bf832cc5315f29b8faac22d4b6c44b3b22 > > >> 100644 > > >> --- a/src/responder/nss/nsssrv_netgroup.c > > >> +++ b/src/responder/nss/nsssrv_netgroup.c > > >> @@ -363,7 +363,14 @@ static errno_t setnetgrent_retry(struct tevent_req > > >> *req) > > >> } > > >> > > >> ret = lookup_netgr_step(step_ctx); > > >> - if (ret != EOK) { > > >> + switch (ret) { > > >> + case EOK: > > >> + break; > > >> + case EMSGSIZE: > > >> + state->netgr->ready = true; > > > > > >Do we need to set the entry to hash? Can't we just shortcut to > > >tevent_req_error ? > > > > > Here is pseudocode: > > ret = get_netgroup_entry // get getgroup from hash > > if (ret == EOK) { > > /* snip */ > > } else if (ret == ENOENT) { > > // netgr is stored in hash > > /* snip */ > > ret = lookup_netgr_step(step_ctx); > > // my changes > > } > > > > At first, lookup_netgr_step will return EMSGSIZE and will jump to label done > > with ret == EMSGSIZE. > > Second time, get_netgroup_entry will found netgroup in hash, > > netgroup will have both flags (found, ready) set to false. > > Flag ready will be false. > > /* Result object is still being constructed > > * Register for notification when it's ready > > */ > > And function will jump to done label with ret == EOK. > > There will not be any notification after object construction, > > because netgroup is created and therefore sss_cmd_done will not be called. > > SSSD will wain until expiration timeout. > > > > With my patch, Flag ready will be true and tevent_req_error(req, ENOENT) > > will be called. > > Yes, I understood that, I was just thinking if we can just fail without > setting the entry in the hash table. > > But this patch gets rid of the segfault and is correct. We can think > about setting the entry or not later. > > Ack.
Pushed to master. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
