On Thu, Nov 13, 2014 at 07:00:02PM +0100, Michal Židek wrote: > On 11/13/2014 06:45 PM, Jakub Hrozek wrote: > >On Wed, Nov 12, 2014 at 02:53:00PM +0100, Michal Židek wrote: > >>On 11/11/2014 01:37 PM, Jakub Hrozek wrote: > >>>On Thu, Nov 06, 2014 at 07:48:20PM +0100, Michal Židek wrote: > >>>>On 11/06/2014 07:43 PM, Michal Židek wrote: > >>>>>On 11/05/2014 04:53 PM, Michal Židek wrote: > >>>>>>I found this bug while working on > >>>>>>https://fedorahosted.org/sssd/ticket/2461 > >>>>>> > >>>>>>It turned out that not only case_sensitive = preserving, > >>>>>>but also case_sensitive = false did not work > >>>>>>with proxy provider. > >>>>>> > >>>>>>But this patch does not solve the issue in the ticket, > >>>>>>so I did not link them. > >>>>> > >>>>>After some investigation I actually think this was > >>>>>the main issue that caused the bug mentioned in > >>>>>the ticket so I added the "Fixes" label to it. > >>>>> > >>>>>I also added second patch that preservers the > >>>>>service name with proxy provider. > >>>>> > >>>>>Michal > >>>>> > >>>> > >>>>And now with patches :) > >>>> > >>> > >>>Coverity found a warning in the patches: > >>>Error: UNUSED_VALUE (CWE-563): > >>>sssd-1.12.3/src/providers/proxy/proxy_id.c:609: value_overwrite: Value > >>>from "sysdb_attrs_add_string(attrs, "nameAlias", lc_gr_name)" is > >>>overwritten with value "12". > >>>sssd-1.12.3/src/providers/proxy/proxy_id.c:614: value_overwrite: Value > >>>from "sysdb_attrs_add_string(attrs, "nameAlias", lc_gr_name)" is > >>>overwritten with value from "sysdb_attrs_add_string(attrs, "nameAlias", > >>>cased_alias)". > >>>sssd-1.12.3/src/providers/proxy/proxy_id.c:623: value_overwrite: Value > >>>from "sysdb_attrs_add_string(attrs, "nameAlias", lc_gr_name)" is > >>>overwritten with value from "sysdb_store_group(dom, real_name, > >>>grp->gr_gid, attrs, cache_timeout, now)". > >>>sssd-1.12.3/src/providers/proxy/proxy_id.c:603: returned_value: Value from > >>>"sysdb_attrs_add_string(attrs, "nameAlias", lc_gr_name)" is assigned to > >>>"ret" here, but that stored value is not used before it is overwritten. > >>> > >>>Looks like an unchecked call. Apart from that, the code looks good. > >> > >>Thank you Jakub and Pavel for the review. New patches are attached. > >> > >>Michal > >> > > > >> From fbd49da512a60d18dad11a5f1751b45db7ded168 Mon Sep 17 00:00:00 2001 > >>From: Michal Zidek <mzi...@redhat.com> > >>Date: Fri, 31 Oct 2014 16:39:25 +0100 > >>Subject: [PATCH 1/2] proxy: Do not try to store same alias twice > >> > >>LDB does not store attributes if they have the > >>same name and value and errors out instead. > >> > >>Fixes: > >>https://fedorahosted.org/sssd/ticket/2461 > >>--- > >> src/providers/proxy/proxy_id.c | 76 > >> ++++++++++++++++++++++++++---------------- > >> 1 file changed, 48 insertions(+), 28 deletions(-) > >> > > > >[...] > > > >> if (alias) { > >> cased_alias = sss_get_cased_name(attrs, alias, !lowercase); > >> if (!cased_alias) { > >>- talloc_zfree(attrs); > >>- return ENOMEM; > >>+ goto done; > > > >ret is undefined on this jump (probably would be EOK). > > Sorry for that. Fixed now. > > > > >The rest of the code looks good to me. > > New version is attached. I only changed that one line. > > Michal
ACK _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel