[SSSD] [PATCH] SYSDB: sysdb_search_object_by_sid should return ENOENT

Lukas Slebodnik lslebodn at redhat.com
Wed Dec 3 09:27:30 UTC 2014


ehlo,

Pavel wanted to send patch in different thread.

LS
-------------- next part --------------
>From 3ff06a7f1582767da5a32d5da6214a2f8a92de3e Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 28 Nov 2014 19:46:38 +0100
Subject: [PATCH] SYSDB: sysdb_search_object_by_sid should return ENOENT

There was a crash in function get_object_from_cache because
sysdb_search_object_by_sid didn' returned ENOENT and
res->msgs was NULL (dereference of NULL pointer)

Part od solution for:
https://fedorahosted.org/sssd/ticket/1991
---
 src/db/sysdb.h                 |  2 +-
 src/db/sysdb_ops.c             | 53 +-----------------------------------------
 src/responder/nss/nsssrv_cmd.c | 24 +++++++++----------
 src/responder/pac/pacsrv_cmd.c | 12 ++++++----
 src/tests/sysdb-tests.c        |  7 ++----
 5 files changed, 23 insertions(+), 75 deletions(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 5bd7f90acb685bbaff5c98f433c7dce8175c33ca..9b88b4a63619456f8f3cc1961e29bbf3946ca5b8 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1033,7 +1033,7 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
                                    struct sss_domain_info *domain,
                                    const char *sid_str,
                                    const char **attrs,
-                                   struct ldb_result **msg);
+                                   struct ldb_result **res);
 
 errno_t sysdb_search_object_by_uuid(TALLOC_CTX *mem_ctx,
                                     struct sss_domain_info *domain,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 998046a2ca1c746b2032f430e5f9c4a7151e1dbc..e88e41f5bfbe0fb8f0875a170d5b55473f18d111 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -3564,61 +3564,10 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
                                    struct sss_domain_info *domain,
                                    const char *sid_str,
                                    const char **attrs,
-                                   struct ldb_result **msg)
+                                   struct ldb_result **res)
 {
-/* TODO: use
     return sysdb_search_object_by_str_attr(mem_ctx, domain, SYSDB_SID_FILTER,
                                            sid_str, attrs, res);
-
-    when verified that all callers can handle ENOENT correctly. */
-
-    TALLOC_CTX *tmp_ctx;
-    const char *def_attrs[] = { SYSDB_NAME, SYSDB_UIDNUM, SYSDB_GIDNUM,
-                                ORIGINALAD_PREFIX SYSDB_NAME,
-                                SYSDB_OBJECTCLASS, NULL };
-    struct ldb_dn *basedn;
-    int ret;
-    struct ldb_result *res = NULL;
-
-    tmp_ctx = talloc_new(NULL);
-    if (!tmp_ctx) {
-        return ENOMEM;
-    }
-
-    basedn = ldb_dn_new_fmt(tmp_ctx, domain->sysdb->ldb, SYSDB_DOM_BASE, domain->name);
-    if (basedn == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_new_fmt failed.\n");
-        ret = ENOMEM;
-        goto done;
-    }
-
-    ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &res,
-                     basedn, LDB_SCOPE_SUBTREE, attrs?attrs:def_attrs,
-                     SYSDB_SID_FILTER, sid_str);
-    if (ret != EOK) {
-        ret = sysdb_error_to_errno(ret);
-        DEBUG(SSSDBG_OP_FAILURE, "ldb_search failed.\n");
-        goto done;
-    }
-
-    if (res->count > 1) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Search for SID [%s] returned more than " \
-                                    "one object.\n", sid_str);
-        ret = EINVAL;
-        goto done;
-    }
-
-    *msg = talloc_steal(mem_ctx, res);
-
-done:
-    if (ret == ENOENT) {
-        DEBUG(SSSDBG_TRACE_FUNC, "No such entry.\n");
-    } else if (ret) {
-        DEBUG(SSSDBG_OP_FAILURE, "Error: %d (%s)\n", ret, strerror(ret));
-    }
-
-    talloc_zfree(tmp_ctx);
-    return ret;
 }
 
 errno_t sysdb_search_object_by_uuid(TALLOC_CTX *mem_ctx,
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index ea58920bc3611c84958e8d0aca0e122d90c68e5c..b6b4a947443aec4b9219004bc5e305415bb8d5d7 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -4491,18 +4491,7 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx)
 
     ret = sysdb_search_object_by_sid(cmdctx, dom, cmdctx->secid, NULL,
                                      &dctx->res);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to make request to our cache!\n");
-        return EIO;
-    }
-
-    if (dctx->res->count > 1) {
-        DEBUG(SSSDBG_FATAL_FAILURE, "getbysid call returned more than one " \
-                                     "result !?!\n");
-        return ENOENT;
-    }
-
-    if (dctx->res->count == 0 && !dctx->check_provider) {
+    if (ret == ENOENT && !dctx->check_provider) {
         DEBUG(SSSDBG_OP_FAILURE, "No results for getbysid call.\n");
 
         /* set negative cache only if not result of cache check */
@@ -4515,6 +4504,17 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx)
         return ENOENT;
     }
 
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to make request to our cache!\n");
+        return EIO;
+    }
+
+    if (dctx->res->count > 1) {
+        DEBUG(SSSDBG_FATAL_FAILURE, "getbysid call returned more than one "
+                                    "result !?!\n");
+        return ENOENT;
+    }
+
     /* if this is a caching provider (or if we haven't checked the cache
      * yet) then verify that the cache is uptodate */
     if (dctx->check_provider) {
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index cc92592893899b1fa269188facc1a8154f80991d..c089c3cbdd52d059c2a2e18e32cc78a702e9a462 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -297,17 +297,19 @@ static void pac_lookup_sids_done(struct tevent_req *req)
             msg = NULL;
             ret = sysdb_search_object_by_sid(pr_ctx, dom, entries[c].key.str,
                                              NULL, &msg);
+            if (ret == ENOENT) {
+                DEBUG(SSSDBG_OP_FAILURE, "No entry found for SID [%s].\n",
+                                          entries[c].key.str);
+                continue;
+            }
+
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_object_by_sid " \
                                           "failed.\n");
                 continue;
             }
 
-            if (msg->count == 0) {
-                DEBUG(SSSDBG_OP_FAILURE, "No entry found for SID [%s].\n",
-                                          entries[c].key.str);
-                continue;
-            } else if (msg->count > 1) {
+            if (msg->count > 1) {
                 DEBUG(SSSDBG_CRIT_FAILURE, "More then one result returned " \
                                             "for SID [%s].\n",
                                             entries[c].key.str);
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 26835d9eb3647c99a8cf50d98143088dc0655f10..a1a5fc0542c35c4af121e66ee4c52c0a509e7487 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -4809,13 +4809,10 @@ START_TEST (test_sysdb_search_return_ENOENT)
     talloc_zfree(res);
 
     /* Search object */
-    /* TODO: Should return ENOENT */
     ret = sysdb_search_object_by_sid(test_ctx, test_ctx->domain,
                                      "S-5-4-3-2-1", NULL, &res);
-    fail_unless(ret == EOK, "sysdb_search_object_by_sid_str failed with "
-                             "[%d][%s].", ret, strerror(ret));
-    fail_unless(res->count == 0, "sysdb_search_object_by_sid_str should not "
-                                 "return anything.");
+    fail_unless(ret == ENOENT, "sysdb_search_object_by_sid_str failed with "
+                               "[%d][%s].", ret, strerror(ret));
     talloc_zfree(res);
 
     /* Search can return more results */
-- 
2.1.0



More information about the sssd-devel mailing list