URL: https://github.com/SSSD/sssd/pull/70
Author: sumit-bose
Title: #70: check_duplicate: check name member before using it
Action: opened
PR body:
"""
The second patch resolves https://fedorahosted.org/sssd/ticket/3231 and adds a
test for the issue.
The first patches fixes a potential memory leak which so far was only relevant
in the tests.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/70/head:pr70
git checkout pr70
From 36e7eb70799df910f6b739a0d4ad25340766e9bf Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 4 Nov 2016 17:13:30 +0100
Subject: [PATCH 1/2] sdap_extend_map: make sure memory can be freed
If there is an error after calling talloc_realloc() the caller cannot
free the memory properly because neither src_map nor _map were pointing
to a valid memory location. With this patch _map will always point to
the current valid location so that it can always be used with
talloc_free().
---
src/providers/ldap/sdap.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index dc7d5e0..9b3e876 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -148,6 +148,27 @@ static enum duplicate_t check_duplicate(struct sdap_attr_map *map,
return NOT_FOUND;
}
+/**
+ * @brief Add attributes to a map
+ *
+ * sdap_extend_map() will call talloc_realloc() on the second argument so the
+ * original storage location might change. The return value _map will always
+ * contain the current memory location which can be used with talloc_free()
+ * even if there is an error.
+ *
+ * @param[in] memctx Talloc memory context
+ * @param[in] src_map Original map, should not be accessed anymore
+ * @param[in] num_entries Number of entries in the original map
+ * @param[in] extra_attrs NULL-terminated array of extra attribute pairs
+ * sysdb_attr:ldap_attr
+ * @param[out] _map New map
+ * @param[out] _new_size Number of entries in the new map
+ *
+ * @return
+ * - EOK success
+ * - ENOMEM memory allocation failed
+ * - ERR_DUP_EXTRA_ATTR sysdb attribute is already used
+ */
int sdap_extend_map(TALLOC_CTX *memctx,
struct sdap_attr_map *src_map,
size_t num_entries,
@@ -162,9 +183,9 @@ int sdap_extend_map(TALLOC_CTX *memctx,
char *sysdb_attr;
errno_t ret;
+ *_map = src_map;
if (extra_attrs == NULL) {
DEBUG(SSSDBG_FUNC_DATA, "No extra attributes\n");
- *_map = src_map;
*_new_size = num_entries;
return EOK;
}
@@ -177,6 +198,7 @@ int sdap_extend_map(TALLOC_CTX *memctx,
if (map == NULL) {
return ENOMEM;
}
+ *_map = map;
for (i = 0; *extra_attrs != NULL; extra_attrs++) {
ret = split_extra_attr(map, *extra_attrs, &sysdb_attr, &ldap_attr);
@@ -221,7 +243,6 @@ int sdap_extend_map(TALLOC_CTX *memctx,
/* Sentinel */
memset(&map[num_entries+nextra], 0, sizeof(struct sdap_attr_map));
- *_map = map;
*_new_size = num_entries + nextra;
return EOK;
}
From f83e18dae3760f5a3d4ccbfef05d9503e32adf9a Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 4 Nov 2016 17:17:13 +0100
Subject: [PATCH 2/2] check_duplicate: check name member before using it
Resolves https://fedorahosted.org/sssd/ticket/3231
---
src/providers/ldap/sdap.c | 2 +-
src/tests/ipa_ldap_opt-tests.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index 9b3e876..6b8a9f7 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -137,7 +137,7 @@ static enum duplicate_t check_duplicate(struct sdap_attr_map *map,
for (i = 0; i < num_entries; i++) {
if (strcmp(map[i].sys_name, sysdb_attr) == 0) {
- if (strcmp(map[i].name, ldap_attr) == 0) {
+ if (map[i].name != NULL && strcmp(map[i].name, ldap_attr) == 0) {
return ALREADY_IN_MAP;
} else {
return CONFLICT_WITH_MAP;
diff --git a/src/tests/ipa_ldap_opt-tests.c b/src/tests/ipa_ldap_opt-tests.c
index 622a617..30e09f7 100644
--- a/src/tests/ipa_ldap_opt-tests.c
+++ b/src/tests/ipa_ldap_opt-tests.c
@@ -467,6 +467,37 @@ START_TEST(test_extra_opts_dup)
extra_attrs,
&out_map, &new_size);
fail_unless(ret == ERR_DUP_EXTRA_ATTR, "[%s]", sss_strerror(ret));
+
+ talloc_free(out_map);
+}
+END_TEST
+
+START_TEST(test_extra_opts_empty_name)
+{
+ errno_t ret;
+ char *extra_attrs[] = { discard_const(SYSDB_UUID":bar"),
+ NULL };
+ struct sdap_attr_map *in_map;
+ struct sdap_attr_map *out_map;
+ size_t new_size;
+
+ ret = sdap_copy_map(global_talloc_context, rfc2307_user_map,
+ SDAP_OPTS_USER, &in_map);
+ fail_unless(ret == EOK, "[%s]", strerror(ret));
+
+ /* Make sure the name if really NULL */
+ fail_unless(rfc2307_user_map[SDAP_AT_USER_UUID].name == NULL,
+ "The reference name is not NULL anymore, "
+ "please choose a different attribute.");
+
+ ret = sdap_extend_map(global_talloc_context,
+ in_map,
+ SDAP_OPTS_USER,
+ extra_attrs,
+ &out_map, &new_size);
+ fail_unless(ret == ERR_DUP_EXTRA_ATTR, "[%s]", sss_strerror(ret));
+
+ talloc_free(out_map);
}
END_TEST
@@ -500,6 +531,7 @@ Suite *ipa_ldap_opt_suite (void)
tcase_add_test (tc_extra_opts, test_no_extra_opts);
tcase_add_test (tc_extra_opts, test_extra_opts_neg);
tcase_add_test (tc_extra_opts, test_extra_opts_dup);
+ tcase_add_test (tc_extra_opts, test_extra_opts_empty_name);
suite_add_tcase (s, tc_extra_opts);
return s;
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
