On 03/21/2016 01:10 PM, Lukas Slebodnik wrote:
On (21/03/16 11:02), Petr Cech wrote:
On 03/16/2016 12:37 PM, Pavel Březina wrote:

[...]

*Patch 2: SYSDB: Add new funtions into sysdb_sudo*

+errno_t
+sysdb_set_sudo_rule_attr(struct sss_domain_info *domain,
+                         const char *name,
+                         struct sysdb_attrs *attrs,
+                         int mod_op)
+{
+    errno_t ret;
+    struct ldb_dn *dn;
+    TALLOC_CTX *tmp_ctx;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
+
+    dn = sysdb_sudo_rule_dn(tmp_ctx, domain, name);
+    NULL_CHECK(dn, ret, done);
+
+    ret = sysdb_set_entry_attr(domain->sysdb, dn, attrs, mod_op);
+
+done:
+    talloc_free(tmp_ctx);
+    return ret;
Temporary context is not needed here, you can use NULL instead and
talloc_free(dn) in the end.

I am not sure about it. I have tried it but it didn't work for me.


What didn't work for you?

errno_t
sysdb_set_sudo_rule_attr(struct sss_domain_info *domain,
                          const char *name,
                          struct sysdb_attrs *attrs,
                          int mod_op)
{
     struct ldb_dn *dn;
     errno_t ret;

     dn = sysdb_sudo_rule_dn(NULL, domain, name);
     if (dn == NULL) {
         return ENOMEM;
     }

     ret = sysdb_set_entry_attr(domain->sysdb, dn, attrs, mod_op);
     talloc_free(dn);
     return ret;
}

Hi list,

I think that I find bug in ldb-1.1.25:


So, I have done little investigation.

In short, backtrace in gdb says:

#0 ldb_dn_new_fmt (mem_ctx=0x0, ldb=0x6578f0, new_fmt=0x7ffff79b3538
"name=%s,cn=%s,cn=custom,cn=%s,cn=sysdb") at ../common/ldb_dn.c:174
#1 0x00007ffff796727c in sysdb_custom_dn (mem_ctx=0x0, dom=0x658500,
object_name=0x409f34 "test_rule3", subtree_name=0x40aecf "sudorules") at
../src/db/sysdb.c:148
#2 0x00000000004073e8 in sysdb_sudo_rule_dn (mem_ctx=0x0, domain=0x658500,
name=0x409f34 "test_rule3") at ../src/db/sysdb_sudo.c:962
#3 0x0000000000407416 in sysdb_set_sudo_rule_attr (domain=0x658500,
name=0x409f34 "test_rule3", attrs=0x658390, mod_op=2) at
../src/db/sysdb_sudo.c:974
#4 0x000000000040458b in test_set_sudo_rule_attr_exist (state=0x614a90) at
../src/tests/cmocka/test_sysdb_sudo.c:400
#5 0x00007ffff7bd6659 in cmocka_run_one_test_or_fixture
(function_name=0x40a8e9 "test_set_sudo_rule_attr_exist", test_func=0x4044d7
<test_set_sudo_rule_attr_exist>, setup_func=setup_func@entry=0x0,
teardown_func=teardown_func@entry=0x0, state=state@entry=0x614a90,
heap_check_point=heap_check_point@entry=0x0) at
/usr/src/debug/cmocka-1.0.1/src/cmocka.c:2305
#6 0x00007ffff7bd6d8f in cmocka_run_one_tests (test_state=0x614a80) at
/usr/src/debug/cmocka-1.0.1/src/cmocka.c:2413
#7 _cmocka_run_group_tests (group_name=0x40a823 "tests", tests=<optimized
out>, num_tests=15, group_setup=<optimized out>, group_teardown=0x0) at
/usr/src/debug/cmocka-1.0.1/src/cmocka.c:2518
#8 0x0000000000404d7a in main (argc=1, argv=0x7fffffffe3f8) at
../src/tests/cmocka/test_sysdb_sudo.c:645


On the line
/usr/src/debug/ldb-1.1.25/common/ldb_dn.c:174:

167 struct ldb_dn *ldb_dn_new_fmt(TALLOC_CTX *mem_ctx,
168 struct ldb_context *ldb,
169 const char *new_fmt, ...)
170 {
171 char *strdn;
172 va_list ap;
173
174 if ( (! mem_ctx) || (! ldb)) return NULL;
175
176 va_start(ap, new_fmt);
177 strdn = talloc_vasprintf(mem_ctx, new_fmt, ap);
178 va_end(ap);
179
180 if (strdn) {
181 struct ldb_dn *dn = ldb_dn_new(mem_ctx, ldb, strdn);
182 talloc_free(strdn);
183 return dn;
184 }
185
186 return NULL;
187 }
188

How you can see, if we send NULL to sysdb_sudo_rule_dn, it will explode very
badly :-)

It might be a bug. But we should try to avoid using NULL context

We had some memory leaks caused by allocating something on NULL context.

LS

Except in this case this is the same thing as using temporary context but shorter.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to