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@dom] to group [somedn]. Skipping.
[sssd[be[dom]]] [sysdb_update_members_ex] (0x0020): Could not add member
[user@dom] to group [somedn2]. Skipping.
[sssd[be[dom]]] [sysdb_update_members_ex] (0x0020): Could not add member
[user@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 <prei...@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@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>From e3283791bd2383a47bcffffdb3631554df589c73 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <prei...@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@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!
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
