Re: [SSSD] [PATCH] proxy: Do not try to store same alias twice
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(a)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).
The rest of the code looks good to me.