[SSSD] [PATCH] SYSDB: add more debug msgs. to group membership code

Pavel Reichl preichl at redhat.com
Wed Aug 12 11:49:44 UTC 2015


On 08/12/2015 01:06 PM, Lukas Slebodnik wrote:
> On (12/08/15 10:28), Pavel Reichl wrote:
>> On 08/12/2015 10:11 AM, Lukas Slebodnik wrote:
>>> On (12/08/15 09:42), Pavel Reichl wrote:
>>>> On 08/12/2015 06:18 AM, Lukas Slebodnik wrote:
>>>>> On (11/08/15 18:36), Pavel Reichl wrote:
>>>>>> Hello,
>>>>>>
>>>>>> I'm investigating log file with debug_level 9 that contains following lines
>>>>>>
>>>>>>> [sssd[be[dom]]] [sysdb_update_members_ex] (0x0020): Could not add member
>>>>>>> [user at dom] to group [somedn]. Skipping.
>>>>>>> [sssd[be[dom]]] [sysdb_update_members_ex] (0x0020): Could not add member
>>>>>>> [user at dom] to group [somedn2]. Skipping.
>>>>>>> [sssd[be[dom]]] [sysdb_update_members_ex] (0x0020): Could not add member
>>>>>>> [user at dom] to group [somedn3]. Skipping.
>>>>>> It's hard to see what went wrong. Attached patch should add more information.
>>>>>>
>>>>> >From 9ea7017e0244ce3db56e0bedd8b1ca0b8b206d78 Mon Sep 17 00:00:00 2001
>>>>>> From: Pavel Reichl <preichl at redhat.com>
>>>>>> Date: Tue, 11 Aug 2015 12:25:22 -0400
>>>>>> Subject: [PATCH] SYSDB: add more debug msgs. to group membership code
>>>>>>
>>>>>> ---
>>>>>> src/db/sysdb_ops.c | 14 ++++++++++++--
>>>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
>>>>>> index d1d43ebe6c71611f3371b2f4ccf5f7911909c9de..c551418094311be76acad2882f6a44ec20dedfb9 100644
>>>>>> --- a/src/db/sysdb_ops.c
>>>>>> +++ b/src/db/sysdb_ops.c
>>>>>> @@ -2171,11 +2171,13 @@ sysdb_group_membership_mod(struct sss_domain_info *domain,
>>>>>>      } else if (type == SYSDB_MEMBER_GROUP) {
>>>>>>          member_dn = sysdb_group_dn(tmp_ctx, domain, member);
>>>>>>      } else {
>>>>>> +        DEBUG(SSSDBG_MINOR_FAILURE, "Unsupported member_type: %d\n", type);
>>>>>>          ret = EINVAL;
>>>>>>          goto done;
>>>>>>      }
>>>>>>
>>>>>>      if (!member_dn) {
>>>>>> +        DEBUG(SSSDBG_MINOR_FAILURE, "Failed to create member_dn.\n");
>>>>>>          ret = ENOMEM;
>>>>> Here is a small problem.
>>>>>
>>>>> Allocation failed but debug_fn might allocate some memory especially
>>>>> with enabled journald. So debug messge needn't be printed.
>>>>> It would be good to release all unused resources (tmp_ctx if available)
>>>>> or do not print such messages at all.
>>>> I added the freeing of tmp context, but I think that in case we are really
>>>> out of memory this might not be enough. The more probable scenario to end up
>>>> in this branch is IMO that sanitizing DN fails or some other failure.
>>>>
>>> So it would be better to add debug message directly to sysdb_dn_sanitize.
>>> instead of after each invocation of sysdb_dn_sanitize. (sysdb_netgroup_dn)
>> OK, in logs you can lost one layer of calling hierarchy, but that's still
>> better than nothing at all.
>>> So you can distinguish there whether it's a memory allocation failure
>>> or ldb function failed.
>>>
>>> LS
>>> _______________________________________________
>>> sssd-devel mailing list
>>> sssd-devel at lists.fedorahosted.org
>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >From e3283791bd2383a47bcffffdb3631554df589c73 Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <preichl at redhat.com>
>> Date: Tue, 11 Aug 2015 12:25:22 -0400
>> Subject: [PATCH] SYSDB: add more debug msgs. to group membership code
>>
>> ---
>> src/db/sysdb.c     |  9 +++++++++
>> src/db/sysdb_ops.c | 16 ++++++++++++++--
>> 2 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/db/sysdb.c b/src/db/sysdb.c
>> index 9da655759c0c35d52854b668693195b3360c5f8b..729d03593cc59e2c9f08e9f1fbe07234cc4a38ab 100644
>> --- a/src/db/sysdb.c
>> +++ b/src/db/sysdb.c
>> @@ -84,6 +84,9 @@ errno_t sysdb_dn_sanitize(TALLOC_CTX *mem_ctx, const char *input,
>>
>>      *sanitized = ldb_dn_escape_value(mem_ctx, val);
>>      if (!*sanitized) {
>> +        talloc_zfree(val.data);
>> +        DEBUG(SSSDBG_MINOR_FAILURE, "ldb_dn_escape_value failed for: %s.\n",
>> +              input);
>>          ret = ENOMEM;
>>      }
>>
>> @@ -162,6 +165,9 @@ struct ldb_dn *sysdb_user_dn(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom,
>>
>>      ret = sysdb_dn_sanitize(NULL, name, &clean_name);
>>      if (ret != EOK) {
>> +        DEBUG(SSSDBG_MINOR_FAILURE,
>> +              "sysdb_dn_sanitize failed for: '%s' - [%d]: %s.\n",
>> +              name, ret, sss_strerror(ret));
>>          return NULL;
>>      }
>>
>> @@ -188,6 +194,9 @@ struct ldb_dn *sysdb_group_dn(TALLOC_CTX *mem_ctx,
>>
>>      ret = sysdb_dn_sanitize(NULL, name, &clean_name);
>>      if (ret != EOK) {
>> +        DEBUG(SSSDBG_MINOR_FAILURE,
>> +              "sysdb_dn_sanitize failed for: %s - [%d]: %s.\n",
>> +              name, ret, sss_strerror(ret));
>>          return NULL;
>>      }
> I'm sorry but my previous was not probably clear.
> So my prefference was to add debug message to sysdb_dn_sanitize,
> which you did. The another option was to add debug messages after
> sysdb_dn_sanitize. So you did it just patrially and it was not necessary.
>
> BTW I checked function ldb_dn_escape_value and it can
> fail only in case of insufficient memory. Your assumption needn't be correct.
> So debug messages might not help. They will just complicate
> situation with additional allocations in debug messages after real ENOMEM
> situation.
>
> IMHO the better way would be to check argument "input"
> for NULL in functions sysdb_dn_sanitize (and also on other places.
> We had "Out of memory" false positive reports in past. They were caused by
> fact that we wanted do allocate copy of NULL
> (inssuficient validation of inputs).
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Lukas, I need this messages to debug user case, messages are not 
perfect, and won't be. Please provide better patch or pick one from the 
changed versions you requested. I don't think we need to spent any more 
time nitpicking. Thanks!



More information about the sssd-devel mailing list