[SSSD] [PATCHES] Do not call sss_cmd_done in function check_cache

Lukas Slebodnik lslebodn at redhat.com
Wed Jun 26 08:42:01 UTC 2013


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 <lslebodn at 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.

>> +            ret = ENOENT;
>> +            /* FALLTHROUGH */
>> +        default:
>>              goto done;
>>          }
>>          tevent_req_done(req);

LS



More information about the sssd-devel mailing list