[SSSD] [PATCH] ldap_child: fix coverity warning

Lukas Slebodnik lslebodn at redhat.com
Tue Mar 10 09:40:07 UTC 2015


On (09/03/15 11:24), Pavel Reichl wrote:
>Hello,
>
>please see attached trivial patch fixing 2 coverity warnings - ret was
>overriden without being read first.
>
>>done:
>>    if (krberr != 0) KRB5_SYSLOG(krberr);
>>    if (keytab) krb5_kt_close(context, keytab);
>>    if (context) krb5_free_context(context);
>>    if (ccname_file_dummy) {
>>    DEBUG(SSSDBG_TRACE_INTERNAL, "Unlinking [%s]\n", ccname_file_dummy);
>>        ret = unlink(ccname_file_dummy);
>>        if (ret == -1) {
>>            ret = errno;
>>            DEBUG(SSSDBG_MINOR_FAILURE,
>>                  "Unlink failed [%d][%s].\n", ret, strerror(ret));
>>        }
>>    }
>>    talloc_free(tmp_ctx);
>>    return krberr;
>ci:
>
>http://sssd-ci.duckdns.org/logs/job/9/03/summary.html
>
>
>Thanks!

>From f2faea03742598aa8185db6deeaa0b23b4fa1e52 Mon Sep 17 00:00:00 2001
>From: Pavel Reichl <preichl at redhat.com>
>Date: Mon, 9 Mar 2015 05:45:20 -0400
>Subject: [PATCH] ldap_child: fix coverity warning
>
>In ldap_child_get_tgt_sync() variable 'ret' got overriden in done
>section without ever before being read.
>---
> src/providers/ldap/ldap_child.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
>index 8f034affa48095b6e512c866f8a3c33465e5c595..e9e845f35df53a780f365f90332a04338ad16026 100644
>--- a/src/providers/ldap/ldap_child.c
>+++ b/src/providers/ldap/ldap_child.c
>@@ -399,6 +399,9 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
>                                         DB_PATH, realm_name);
>     if (ccname_file_dummy == NULL) {
>         ret = ENOMEM;
>+        DEBUG(SSSDBG_CRIT_FAILURE,
>+              "talloc_asprintf failed: %s:[%d].\n",
>+              strerror(ret), ret);
>         goto done;
>     }
> 
>@@ -407,6 +410,9 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
>     umask(old_umask);
>     if (fd == -1) {
>         ret = errno;
>+        DEBUG(SSSDBG_CRIT_FAILURE,
>+              "mkstemp failed: %s:[%d].\n",
>+              strerror(ret), ret);
>         goto done;

ACK for debug messages.
NACK for patch.

As you can see ldap_child_get_tgt_sync returns krb5_error_code and not
int or errnot_t.

It means that function ldap_child_get_tgt_sync might return wrong error code.
It's very likely that krberr was set to 0 in previous steps.

LS



More information about the sssd-devel mailing list