[SSSD] [PATCH] LDAP: Conditional jump depends on uninitialised value

Lukas Slebodnik lslebodn at redhat.com
Tue Jan 27 08:02:42 UTC 2015


On (26/01/15 20:14), Sumit Bose wrote:
>I think there is still a chance that has_posix is uninitialised.
>After ret != EOK is returned in 325 we jump in the if-block starting at
>331. If sdap_id_op_done() finishes successful has_posix is still
>undefined but ret == EOK in line 346.
>
> 325     ret = sdap_posix_check_recv(subreq, &has_posix);
> 326     talloc_zfree(subreq);
> 327     if (ret != EOK) {
> 328         /* We can only finish the id_op on error as the connection
> 329          * is re-used by the user search
> 330          */
> 331         ret = sdap_id_op_done(state->op, ret, &dp_error);
> 332         if (dp_error == DP_ERR_OK && ret != EOK) {
> 333             /* retry */
> 334             ret = users_get_retry(req);
> 335             if (ret != EOK) {
> 336                 tevent_req_error(req, ret);
> 337             }
> 338             return;
> 339         }
> 340     }
> 341 
> 342     state->ctx->srv_opts->posix_checked = true;
> 343 
> 344     /* If the check ran to completion, we know for certain about the attributes
> 345      */
> 346     if (has_posix == false) {
> 347         state->sdap_ret = ERR_NO_POSIX;
> 348         tevent_req_done(req);
> 349         return;
> 350     }
>
You are right.
The variable ret is used for checking 3 functions sdap_posix_check_recv,
sdap_id_op_done, users_get_retry.

I decided to create another variable.

The other alternative is to initialize boolean variable has_posix to true,
becuase if there is any problem with connection we should still expect there
can be posix attributes.

LS
-------------- next part --------------
>From ea165f3c419502ac48ac91dcae3937be4fb46040 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Mon, 26 Jan 2015 15:43:08 +0100
Subject: [PATCH] LDAP: Conditional jump depends on uninitialised value

==31767==    at 0x5B66CFC: users_get_posix_check_done (ldap_id.c:346)
==31767==    by 0x4DCC6AA: _tevent_req_notify_callback (tevent_req.c:112)
==31767==    by 0x4DCC724: tevent_req_finish (tevent_req.c:149)
==31767==    by 0x4DCC782: _tevent_req_error (tevent_req.c:167)
==31767==    by 0x5B7ED43: sdap_posix_check_done (sdap_async.c:2486)
==31767==    by 0x4DCC6AA: _tevent_req_notify_callback (tevent_req.c:112)
==31767==    by 0x4DCC724: tevent_req_finish (tevent_req.c:149)
==31767==    by 0x4DCC782: _tevent_req_error (tevent_req.c:167)
==31767==    by 0x5B7DE37: sdap_get_generic_op_finished (sdap_async.c:1523)
==31767==    by 0x5B7D62B: sdap_process_result (sdap_async.c:357)
==31767==    by 0x4DCFC1C: tevent_common_loop_timer_delay (tevent_timed.c:341)
==31767==    by 0x4DD0E12: epoll_event_loop_once (tevent_epoll.c:911)
==31767==    by 0x4DCF23E: std_event_loop_once (tevent_standard.c:114)
==31767==    by 0x4DCB38F: _tevent_loop_once (tevent.c:530)
==31767==    by 0x4DCB58B: tevent_common_loop_wait (tevent.c:634)
==31767==    by 0x4DCF1BE: std_event_loop_wait (tevent_standard.c:140)
==31767==    by 0x4DCB627: _tevent_loop_wait (tevent.c:653)
==31767==    by 0x489AB98: server_loop (server.c:668)
==31767==    by 0x10D035: main (data_provider_be.c:2915)
---
 src/providers/ldap/ldap_id.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 2e58f4e49eb33a85cbb8b4144c69004c6b5b312b..2c096003c0cca3899e65c6f9c7a47bdb94e161a9 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -315,6 +315,7 @@ static void users_get_connect_done(struct tevent_req *subreq)
 static void users_get_posix_check_done(struct tevent_req *subreq)
 {
     errno_t ret;
+    errno_t ret2;
     bool has_posix;
     int dp_error;
     struct tevent_req *req = tevent_req_callback_data(subreq,
@@ -328,8 +329,8 @@ static void users_get_posix_check_done(struct tevent_req *subreq)
         /* We can only finish the id_op on error as the connection
          * is re-used by the user search
          */
-        ret = sdap_id_op_done(state->op, ret, &dp_error);
-        if (dp_error == DP_ERR_OK && ret != EOK) {
+        ret2 = sdap_id_op_done(state->op, ret, &dp_error);
+        if (dp_error == DP_ERR_OK && ret2 != EOK) {
             /* retry */
             ret = users_get_retry(req);
             if (ret != EOK) {
@@ -343,7 +344,7 @@ static void users_get_posix_check_done(struct tevent_req *subreq)
 
     /* If the check ran to completion, we know for certain about the attributes
      */
-    if (has_posix == false) {
+    if (ret == EOK && has_posix == false) {
         state->sdap_ret = ERR_NO_POSIX;
         tevent_req_done(req);
         return;
-- 
2.1.0



More information about the sssd-devel mailing list