On Wed, Dec 10, 2014 at 05:43:37PM +0100, Lukas Slebodnik wrote:
> On (10/12/14 11:26), Sumit Bose wrote:
> >On Wed, Dec 10, 2014 at 10:44:32AM +0100, Pavel Reichl wrote:
> >>
> >> On 12/09/2014 01:36 PM, Sumit Bose wrote:
> >> >On Tue, Nov 25, 2014 at 03:42:14PM +0100, Pavel Reichl wrote:
> >> >>Hello,
> >> >>
> >> >>please see attached patch for
https://fedorahosted.org/sssd/ticket/2492
> >> >>
> >> >>Thanks!
> >> >Hi Pavel,
> >> >
> >> >thank you for the patch, it works well in my tests and I didn't see
any
> >> >regressions in IPA setup with and without trsut to AD, so ACK.
> >> >
> >> >I would just like to ask you to add a comment to
> >> >
> >> >>@@ -842,6 +913,23 @@ static int sdap_save_grpmem(TALLOC_CTX
*memctx,
> >> >> goto fail;
> >> >> }
> >> >> }
> >> >>+ if (opts->schema_type == SDAP_SCHEMA_IPA_V1) {
> >> >>+ ret = sysdb_attrs_get_string(attrs, SYSDB_SID_STR,
&group_sid);
> >> >>+ if (ret != EOK) {
> >> >>+ DEBUG(SSSDBG_TRACE_FUNC, "Failed to get group
sid\n");
> >> >>+ group_sid = NULL;
> >> >>+ }
> >> >>+
> >> >>+ if (group_sid != NULL) {
> >> >>+ ret = retain_extern_members(memctx, dom, group_name,
group_sid,
> >> >>+ &userdns,
&nuserdns);
> >> >>+ if (ret != EOK) {
> >> >>+ DEBUG(SSSDBG_MINOR_FAILURE,
> >> >>+ "retain_extern_members failed:
%d:[%s].\n",
> >> >>+ ret, sss_strerror(ret));
> >> >>+ }
> >> >>+ }
> >> >>+ }
> >> >which explains that this is a temporary solution until the IPA provider
> >> >can resolve external group membership. I have created
> >> >https://fedorahosted.org/sssd/ticket/2522 for this. Feel free to
> >> >explicitly add the ticket URL into the comment.
> >> >
> >> >bye,
> >> >Sumit
> >> Thanks for review.
> >> Please see updated patch.
> >>
> >
> >Thank you, ACK.
> NACK, sssd crashed with this patch.
> How to reproduce:
> sssd with ipa server mode:
>
> id admin(a)rdustv1911.test //ipa admin
> id aduser1(a)ipaad2012r2.test // aduser
> id aduser2(a)ipaad2012r2.test
> sss_cache -E
> id aduser1(a)ipaad2012r2.test // aduser
good catch, thank you. Looks like I always only tested with one external
member. Does
diff --git a/src/providers/ldap/sdap_async_groups.c
b/src/providers/ldap/sdap_async_groups.c
index 43744ac..4bc2aa7 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -861,7 +861,7 @@ retain_extern_members(TALLOC_CTX *mem_ctx,
(*_nuserdns)++;
*_userdns = talloc_realloc(mem_ctx, *_userdns, char*, *_nuserdns);
- *_userdns[(*_nuserdns)-1] = talloc_steal(mem_ctx, dns[i]);
+ (*_userdns)[(*_nuserdns)-1] = talloc_steal(mem_ctx, dns[i]);
}
}
fix the issue for you?
Yet, it fixed.
I could see the mistake in code and I was looking at the code at least fo 10
minutes. Someone should write 100 times I will learn C operator precedence
Maybe this should be better written with local variables to avoid this
kind of issues?
Additionally there is a NULL check missing after talloc_realloc().