On (12/08/16 16:14), Jakub Hrozek wrote: >On Fri, Aug 12, 2016 at 04:05:22PM +0200, Lukas Slebodnik wrote: >> On (10/08/16 20:59), Michal Židek wrote: >> >On 08/10/2016 08:36 PM, Lukas Slebodnik wrote: >> >> On (10/08/16 17:41), Michal Židek wrote: >> >> > Hi, >> >> > >> >> > see the attached patch. >> >> > >> >> > I modified the detection of duplicates when >> >> > extending the maps (sysdb_attr:ldap_attr). >> >> > >> >> > When we try to add entry to the map >> >> > that already exists in the map, then >> >> > without this patch we will fail. >> >> > >> >> > With this patch, we only fail if the >> >> > newly added extension would redefine >> >> > already existing entry in the map. >> >> > >> >> > Otherwise it is just skipped without >> >> > a failure (we just skip adding what >> >> > is already there). >> >> > >> >> > I created simple CI test for this (first >> >> > patch). >> >> > >> >> > Michal >> >> >> >> > From 5a2ef2a98e483701603a42bc50e9a11d8ee651ff Mon Sep 17 00:00:00 2001 >> >> > From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >> >> > Date: Wed, 10 Aug 2016 15:41:34 +0200 >> >> > Subject: [PATCH 2/2] sdap: Skip exact duplicates when extending maps >> >> > >> >> > When extending map with entry that already >> >> > exists in the map in the exacty same form, >> >> > then there is no need to fail. >> >> > >> >> > We should only fail if we try to >> >> > change purpose of already used sysdb >> >> > attribute. >> >> > >> >> > Resolves: >> >> > https://fedorahosted.org/sssd/ticket/3120 >> >> > --- >> >> > src/providers/ldap/sdap.c | 41 +++++++++++++++++++++++++++++++++++------ >> >> > 1 file changed, 35 insertions(+), 6 deletions(-) >> >> > >> >> > diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c >> >> > index 97b8f12..e1cf70f 100644 >> >> > --- a/src/providers/ldap/sdap.c >> >> > +++ b/src/providers/ldap/sdap.c >> >> > @@ -122,19 +122,39 @@ static errno_t split_extra_attr(TALLOC_CTX >> >> > *mem_ctx, >> >> > return EOK; >> >> > } >> >> > >> >> > -static bool is_sysdb_duplicate(struct sdap_attr_map *map, >> >> > - int num_entries, >> >> > - const char *sysdb_attr) >> >> > +/* _already_in_map is set to true if the attribute >> >> > + * already exists in the map and is used for the same >> >> > + * LDAP attribute. >> >> > + * >> >> > + * _conflicts_with_map is set to true if the attribute >> >> > + * already exists in map, but is used for different >> >> > + * LDAP attribute. >> >> > + * */ >> >> > +static void check_duplicate(struct sdap_attr_map *map, >> >> > + int num_entries, >> >> > + const char *sysdb_attr, >> >> > + const char *ldap_attr, >> >> > + bool *_already_in_map, >> >> > + bool *_conflicts_with_map) >> >> > { >> >> This function has 3 output boolean argumets: >> >> It would be better to return enum instead of >> >> adding new parametrs. >> >> >> >> LS >> > >> >Ok, attached is version with enum. >> > >> >Michal >> > >> >> I tried to rest use-case from ticket #3120 >> http://www.freeipa.org/page/Web_App_Authentication/Example_setup >> >> but sssd_be crashed >> (gdb) bt >> #0 0x00007fc29afb8961 in __strncasecmp_l_avx () from /lib64/libc.so.6 >> #1 0x00007fc29f199ea0 in sysdb_attrs_get_el_ext >> (attrs=attrs@entry=0x7fc2a1adc740, name=name@entry=0x0, >> alloc=alloc@entry=true, el=el@entry=0x7ffd0d466810) at src/db/sysdb.c:290 >> #2 0x00007fc29f199fad in sysdb_attrs_get_el >> (attrs=attrs@entry=0x7fc2a1adc740, name=name@entry=0x0, >> el=el@entry=0x7ffd0d466810) at src/db/sysdb.c:323 >> #3 0x00007fc28fe41400 in sdap_attrs_add_ldap_attr >> (ldap_attrs=ldap_attrs@entry=0x7fc2a1adc740, attr_name=0x0, >> attr_desc=attr_desc@entry=0x0, multivalued=multivalued@entry=true, >> name=<optimized out>, attrs=attrs@entry=0x7fc2a1ac4860) at >> src/providers/ldap/sdap_utils.c:40 >> #4 0x00007fc28fe1a2c7 in sdap_save_user >> (memctx=memctx@entry=0x7fc2a1adf600, opts=0x7fc2a1a7eae0, >> dom=0x7fc2a1a54ae0, attrs=<optimized out>, _usn_value=_usn_value@entry=0x0, >> now=now@entry=0) at src/providers/ldap/sdap_async_users.c:482 >> #5 0x00007fc28fe2b667 in sdap_get_initgr_user (subreq=0x0) at >> src/providers/ldap/sdap_async_initgroups.c:2961 >> #6 0x00007fc28fe13d99 in generic_ext_search_handler (subreq=0x0, >> opts=<optimized out>) at src/providers/ldap/sdap_async.c:1688 >> #7 0x00007fc28fe16407 in sdap_get_generic_op_finished (op=<optimized out>, >> reply=<optimized out>, error=<optimized out>, pvt=<optimized out>) at >> src/providers/ldap/sdap_async.c:1578 >> #8 0x00007fc28fe14ded in sdap_process_message (ev=<optimized out>, >> sh=<optimized out>, msg=0x7fc2a1aba9f0) at >> src/providers/ldap/sdap_async.c:353 >> #9 sdap_process_result (ev=<optimized out>, pvt=<optimized out>) at >> src/providers/ldap/sdap_async.c:197 >> #10 0x00007fc29b85fb4f in tevent_common_loop_timer_delay () from >> /lib64/libtevent.so.0 >> #11 0x00007fc29b860b5a in epoll_event_loop_once () from /lib64/libtevent.so.0 >> #12 0x00007fc29b85f257 in std_event_context_init () from >> /lib64/libtevent.so.0 >> #13 0x00007fc29b85b40d in _tevent_loop_until () from /lib64/libtevent.so.0 >> #14 0x00007fc2a1a4bd20 in ?? () >> #15 0x00007fc29f1e7c47 in ?? () from /usr/lib64/sssd/libsss_util.so >> #16 0x00007fc29b85f1f7 in std_event_loop_once () from /lib64/libtevent.so.0 >> #17 0x00007fc29f1cb7f3 in server_loop (main_ctx=0x7fc2a1a4d080) at >> src/util/server.c:702 >> #18 0x00007fc29fa45952 in main (argc=8, argv=<optimized out>) at >> src/providers/data_provider_be.c:587 >> >> >> it crashed because one agruments from strcasecmp was NULL >> (dereference of NULL pointer) >> >> I guess that we hit the last value in user_map (zeroed structure) >> In other words, opts->user_map_cnt does not match reallity. >> >> (gdb) l 482 >> 477 } >> 478 } >> 479 } >> 480 >> 481 for (i = SDAP_FIRST_EXTRA_USER_AT; i < opts->user_map_cnt; i++) { >> 482 ret = sdap_attrs_add_list(attrs, opts->user_map[i].sys_name, >> 483 NULL, user_name, user_attrs); >> 484 if (ret) { >> 485 goto done; >> 486 } >> >> NACK > >Do you think you can fix the patch with additional one given that this is >a pretty bad regression and Michal is out for a couple of weeks? I will try to look
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
