On (10/12/14 21:32), Sumit Bose wrote:
> 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
http://en.cppreference.com/w/c/language/operator_precedence
> Maybe this should be better written with local variables to avoid this
> kind of issues?
+1
> Additionally there is a NULL check missing after talloc_realloc().
+1
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel I'm really sorry
about the crash.
Please see if the new patch is crash free.
Thanks!