[SSSD] [PATCH] proxy: Do not try to store same alias twice

Jakub Hrozek jhrozek at redhat.com
Thu Nov 13 18:27:04 UTC 2014


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 <mzidek at 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



More information about the sssd-devel mailing list