[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