On (12/08/14 10:54), Jakub Hrozek wrote:
Hi,
the attached patch fixes
https://fedorahosted.org/sssd/ticket/2391
To reproduce, request a group by SID, like this:
$ python
import pysss_nss_idmap
pysss_nss_idmap.getnamebysid('S-1-5-21-1104576142-166023250-1132398744-513')
{'S-1-5-21-1104576142-166023250-1132398744-513': {'type': 2,
'name': u'domain users(a)win.example.com'}}
From 95ef335f17739c441bb6c9793a6b4d7c6185ca2b Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Tue, 12 Aug 2014 10:32:33 +0200
Subject: [PATCH] IPA: handle searches by SID in apply_subdomain_homedir
https://fedorahosted.org/sssd/ticket/2391
apply_subdomain_homedir() didn't handle the situation where an entity
that doesn't match was requested from the cache. For user and group
lookups this wasn't a problem because the negative match was caught
sooner.
But SID lookups can match either user or group. When a group SID was
requested, the preceding LDAP request matched the SID and stored the
group in the cache. Then apply_subdomain_homedir() only tried to search
user by SID, didn't find the entry and accessed a NULL pointer.
A simple reproducer is:
$ python
>>> import pysss_nss_idmap
>>> pysss_nss_idmap.getnamebysid(group_sid)
The group_sid can be anything, including Domain Users (XXX-513)
---
src/providers/ipa/ipa_subdomains_id.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/providers/ipa/ipa_subdomains_id.c
b/src/providers/ipa/ipa_subdomains_id.c
index 113bc6c06f82bc631b3efa92b87a1cadc7f22605..7857cd566f7585663d2e0fc94b1265132f4af739
100644
--- a/src/providers/ipa/ipa_subdomains_id.c
+++ b/src/providers/ipa/ipa_subdomains_id.c
@@ -516,18 +516,22 @@ apply_subdomain_homedir(TALLOC_CTX *mem_ctx, struct sss_domain_info
*dom,
goto done;
}
- if (ret != EOK && ret != ENOENT) {
- DEBUG(SSSDBG_OP_FAILURE,
- "Failed to make request to our cache: [%d]: [%s]\n",
- ret, sss_strerror(ret));
- goto done;
- }
-
if ((res && res->count == 0) || (msg && msg->num_elements ==
0)) {
ret = ENOENT;
goto done;
}
+ if (ret != EOK && ret != ENOENT) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Failed to make request to our cache: [%d]: [%s]\n",
+ ret, sss_strerror(ret));
+ goto done;
+ } else if (ret == ENOENT) {
+ DEBUG(SSSDBG_TRACE_FUNC, "Cannot find [%s] with search type [%d]\n",
+ filter_value, filter_type);
+ goto done;
+ }
+
if (res != NULL) {
msg = res->msgs[0];
}
I admit it can fix the crash, but it is opposite of change PavelR did in
different code.
commit 09be002e58babda513b4b75d2b9eb9b2c351fa26
Author: Pavel Reichl <preichl(a)redhat.com>
Date: Wed Feb 26 16:58:24 2014 +0000
NSS: sysdb_getnetgr check return value first
It could be also chance to improve problems decribed in ticket #1991.
BTW I was not able to reproduce crash. Could you describe hierarchy of groups
in your testing environment.
LS