ehlo, Attached patches fix failures in test on big endian http://s390.koji.fedoraproject.org/kojifiles/work/tasks/4511/2274511/build.log
It is not just bug in test becuase the same bug is in sysdb_enumpwent_filter and sysdb_enumgrent_filter LS
>From 748589e789c580140b576b498844cca2788d06d8 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <[email protected]> Date: Fri, 1 Jul 2016 23:18:46 +0200 Subject: [PATCH 1/2] test_sysdb_ts_cache: Do not use wrong pointer for output argument The function sysdb_search_groups expects pointer to size_t as an output argument msgs_count. However, struct ldb_result has type unsigned for element count. The size of unsigned is lower then size of size_t on some platforms. Therefore we should not cast to pointer to size_t if we want to write count of messages into struct ldb_result -> count. The valgrind did not detect write out of boundary for the element count because it is the 1st element in structure ldb_result. It didn't cause any problem on little endian because the most significant part of size_t was properly stored to type unsigned. We firstly store to output argument _msgs_count and then to output argument _msgs in the function sysdb_cache_search_entry therefore element msgs was not damaged and contained correct data. --- src/tests/cmocka/test_sysdb_ts_cache.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/tests/cmocka/test_sysdb_ts_cache.c b/src/tests/cmocka/test_sysdb_ts_cache.c index e04daee1fc38ab454158b6067a967a44fbc0d2dd..c76dd1e8e6c40310076c7e6831713eeedd169286 100644 --- a/src/tests/cmocka/test_sysdb_ts_cache.c +++ b/src/tests/cmocka/test_sysdb_ts_cache.c @@ -795,6 +795,7 @@ static void test_merge_ldb_results(void **state) struct ldb_result *res; struct ldb_result *res1; struct ldb_result *res2; + size_t msgs_count; res1 = talloc_zero(test_ctx, struct ldb_result); assert_non_null(res1); @@ -831,7 +832,8 @@ static void test_merge_ldb_results(void **state) assert_non_null(filter); ret = sysdb_search_groups(res1, test_ctx->tctx->dom, filter, gr_fetch_attrs, - (size_t *) &res1->count, &res1->msgs); + &msgs_count, &res1->msgs); + res1->count = (unsigned)msgs_count; talloc_free(filter); assert_int_equal(ret, EOK); assert_int_equal(res1->count, 2); @@ -842,7 +844,8 @@ static void test_merge_ldb_results(void **state) assert_non_null(filter); ret = sysdb_search_groups(res2, test_ctx->tctx->dom, filter, gr_fetch_attrs, - (size_t *) &res2->count, &res2->msgs); + &msgs_count, &res2->msgs); + res2->count = (unsigned)msgs_count; talloc_free(filter); assert_int_equal(ret, EOK); assert_int_equal(res2->count, 2); -- 2.7.4
>From 3a86f2e5090b3a4d83941bdac72cae7b6a77e13b Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <[email protected]> Date: Fri, 1 Jul 2016 22:27:24 +0200 Subject: [PATCH 2/2] sysdb: Use ldb_result as output in sysdb_search_ts_{users,groups} Passing address of unsigned to the output argument size_t causes access out of boundaries for type unsigned and and wrong data on big endian. It looks like functions sysdb_search_ts_{users,groups} need to store results in structure ldb_result anyway for further processing. Therefore it will be better to convert output arguments size_t* + ldb_message*** into structure ldb_result and avoid using additional helper variable with type size_t before each invocation of these functions. --- src/db/sysdb_ops.c | 46 ++++++++++++++++++++++++++++------ src/db/sysdb_private.h | 6 ++--- src/db/sysdb_search.c | 6 ++--- src/tests/cmocka/test_sysdb_ts_cache.c | 33 ++++++++---------------- 4 files changed, 53 insertions(+), 38 deletions(-) diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 9ee8f6fd93a2b2f180291f4a7c43bbfe8d50d44c..34e8a5ef4ef32d86c0ba497e3fe494faa20c58e5 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -3440,15 +3440,30 @@ int sysdb_search_ts_users(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, const char *sub_filter, const char **attrs, - size_t *msgs_count, - struct ldb_message ***msgs) + struct ldb_result *res) { + size_t msgs_count; + struct ldb_message **msgs; + int ret; + + if (res == NULL) { + return EINVAL; + } + + ZERO_STRUCT(*res); + if (domain->sysdb->ldb_ts == NULL) { return ENOENT; } - return sysdb_cache_search_users(mem_ctx, domain, domain->sysdb->ldb_ts, - sub_filter, attrs, msgs_count, msgs); + ret = sysdb_cache_search_users(mem_ctx, domain, domain->sysdb->ldb_ts, + sub_filter, attrs, &msgs_count, &msgs); + if (ret == EOK) { + res->count = (unsigned)msgs_count; + res->msgs = msgs; + } + + return ret; } /* =Delete-User-by-Name-OR-uid============================================ */ @@ -3642,15 +3657,30 @@ int sysdb_search_ts_groups(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, const char *sub_filter, const char **attrs, - size_t *msgs_count, - struct ldb_message ***msgs) + struct ldb_result *res) { + size_t msgs_count; + struct ldb_message **msgs; + int ret; + + if (res == NULL) { + return EINVAL; + } + + ZERO_STRUCT(*res); + if (domain->sysdb->ldb_ts == NULL) { return ENOENT; } - return sysdb_cache_search_groups(mem_ctx, domain, domain->sysdb->ldb_ts, - sub_filter, attrs, msgs_count, msgs); + ret = sysdb_cache_search_groups(mem_ctx, domain, domain->sysdb->ldb_ts, + sub_filter, attrs, &msgs_count, &msgs); + if (ret == EOK) { + res->count = (unsigned)msgs_count; + res->msgs = msgs; + } + + return ret; } /* =Delete-Group-by-Name-OR-gid=========================================== */ diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index 6f6a19a743514a5e0241269041b615261f7898e6..fda33dc74177c631df0c5033afe07c5cd9b141c7 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -241,15 +241,13 @@ int sysdb_search_ts_users(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, const char *sub_filter, const char **attrs, - size_t *msgs_count, - struct ldb_message ***msgs); + struct ldb_result *res); int sysdb_search_ts_groups(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, const char *sub_filter, const char **attrs, - size_t *msgs_count, - struct ldb_message ***msgs); + struct ldb_result *res); /* Compares the modifyTimestamp attribute between old_entry and * new_entry. Returns true if they differ (or either entry is missing diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index 61b07acc93bb8e03907de89a79f9ab0540d354f9..ce30e61c3d971a08215cc97ad2dc9e7cc4d1a231 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -592,10 +592,9 @@ int sysdb_enumpwent_filter(TALLOC_CTX *mem_ctx, } DEBUG(SSSDBG_TRACE_LIBS, "Searching timestamp cache with [%s]\n", ts_filter); - ZERO_STRUCT(ts_res); ret = sysdb_search_ts_users(tmp_ctx, domain, ts_filter, sysdb_ts_cache_attrs, - (size_t *) &ts_res.count, &ts_res.msgs); + &ts_res); if (ret != EOK && ret != ENOENT) { goto done; } @@ -1093,10 +1092,9 @@ int sysdb_enumgrent_filter(TALLOC_CTX *mem_ctx, } DEBUG(SSSDBG_TRACE_LIBS, "Searching timestamp cache with [%s]\n", ts_filter); - ZERO_STRUCT(ts_res); ret = sysdb_search_ts_groups(tmp_ctx, domain, ts_filter, sysdb_ts_cache_attrs, - (size_t *) &ts_res.count, &ts_res.msgs); + &ts_res); if (ret != EOK && ret != ENOENT) { goto done; } diff --git a/src/tests/cmocka/test_sysdb_ts_cache.c b/src/tests/cmocka/test_sysdb_ts_cache.c index c76dd1e8e6c40310076c7e6831713eeedd169286..d5492299647f54e379ea3f305ccc1501c7f6c79f 100644 --- a/src/tests/cmocka/test_sysdb_ts_cache.c +++ b/src/tests/cmocka/test_sysdb_ts_cache.c @@ -522,8 +522,7 @@ static void test_sysdb_group_delete(void **state) test_ctx->tctx->dom, TS_FILTER_ALL, sysdb_ts_cache_attrs, - (size_t *) &ts_res->count, - &ts_res->msgs); + ts_res); assert_int_equal(ret, ENOENT); group_attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1); @@ -542,8 +541,7 @@ static void test_sysdb_group_delete(void **state) test_ctx->tctx->dom, TS_FILTER_ALL, sysdb_ts_cache_attrs, - (size_t *) &ts_res->count, - &ts_res->msgs); + ts_res); assert_int_equal(ret, EOK); assert_int_equal(ts_res->count, 1); @@ -562,8 +560,7 @@ static void test_sysdb_group_delete(void **state) test_ctx->tctx->dom, TS_FILTER_ALL, sysdb_ts_cache_attrs, - (size_t *) &ts_res->count, - &ts_res->msgs); + ts_res); assert_int_equal(ret, ENOENT); res = sysdb_getgrnam_res(test_ctx, test_ctx->tctx->dom, TEST_GROUP_NAME); @@ -610,8 +607,7 @@ static void test_sysdb_group_rename(void **state) test_ctx->tctx->dom, filter, sysdb_ts_cache_attrs, - (size_t *) &ts_res->count, - &ts_res->msgs); + ts_res); assert_int_equal(ret, ENOENT); talloc_free(filter); @@ -866,9 +862,8 @@ static void test_group_bysid(void **state) const char *gr_fetch_attrs[] = SYSDB_GRSRC_ATTRS; struct sysdb_attrs *group_attrs = NULL; struct ldb_result *res; - size_t msgs_count; struct ldb_message *msg = NULL; - struct ldb_message **msgs = NULL; + struct ldb_result ts_res; group_attrs = create_sidstr_attrs(test_ctx, TEST_GROUP_SID); assert_non_null(group_attrs); @@ -931,8 +926,7 @@ static void test_group_bysid(void **state) test_ctx->tctx->dom, TS_FILTER_ALL, sysdb_ts_cache_attrs, - &msgs_count, - &msgs); + &ts_res); assert_int_equal(ret, ENOENT); } @@ -1062,8 +1056,7 @@ static void test_sysdb_user_delete(void **state) test_ctx->tctx->dom, TS_FILTER_ALL, sysdb_ts_cache_attrs, - (size_t *) &ts_res->count, - &ts_res->msgs); + ts_res); assert_int_equal(ret, ENOENT); user_attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1); @@ -1085,8 +1078,7 @@ static void test_sysdb_user_delete(void **state) test_ctx->tctx->dom, TS_FILTER_ALL, sysdb_ts_cache_attrs, - (size_t *) &ts_res->count, - &ts_res->msgs); + ts_res); assert_int_equal(ret, EOK); assert_int_equal(ts_res->count, 1); @@ -1109,8 +1101,7 @@ static void test_sysdb_user_delete(void **state) test_ctx->tctx->dom, TS_FILTER_ALL, sysdb_ts_cache_attrs, - (size_t *) &ts_res->count, - &ts_res->msgs); + ts_res); assert_int_equal(ret, ENOENT); get_pw_timestamp_attrs(test_ctx, TEST_USER_NAME, @@ -1235,9 +1226,8 @@ static void test_user_bysid(void **state) const char *pw_fetch_attrs[] = SYSDB_PW_ATTRS; struct sysdb_attrs *user_attrs = NULL; struct ldb_result *res; - size_t msgs_count; struct ldb_message *msg = NULL; - struct ldb_message **msgs = NULL; + struct ldb_result ts_res; user_attrs = create_sidstr_attrs(test_ctx, TEST_USER_SID); assert_non_null(user_attrs); @@ -1298,8 +1288,7 @@ static void test_user_bysid(void **state) test_ctx->tctx->dom, TS_FILTER_ALL, sysdb_ts_cache_attrs, - &msgs_count, - &msgs); + &ts_res); assert_int_equal(ret, ENOENT); } -- 2.7.4
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
