#47808 If be_txn plugin fails in ldbm_back_add, adding entry is double freed.
Closed: wontfix None Opened 9 years ago by nhosoi.

Thanks to Mark for finding out this problem. (Note: 1.2.11 does not have this bug.)

Mark Reynolds wrote:
> steps to reproduce:
>
> ldapmodify -h $HOST -p $PORT -D "cn=directory manager" -w $PW -a <<EOF
> dn: cn=attribute uniqueness,cn=plugins,cn=config
> changetype: modify
> replace: nsslapd-pluginEnabled
> nsslapd-pluginEnabled: on
> -
> replace: nsslapd-pluginarg0
> nsslapd-pluginarg0: sn
> -
> replace: nsslapd-pluginarg1
> nsslapd-pluginarg1: dc=example,dc=com
> EOF
>
> 3. Add user:
> ldapmodify -h $HOST -p $PORT -D "cn=directory manager" -w $PW -a <<EOF
> dn: cn=tuser1,ou=people,dc=example,dc=com
> objectclass: person
> objectclass: top
> sn: tuser1
> cn: tuser1
> EOF
>
> 4. Restart server
>
> 5. Add user with value 'sn' equal to value of sn of cn=tuser1:
> ldapmodify -h $HOST -p $PORT -D "cn=directory manager" -w $PW -a <<EOF
> dn: cn=tuser2,ou=people,dc=example,dc=com
> objectclass: person
> objectclass: top
> sn: tuser1
> cn: tuser2
>
> --> Add is rejected by the attr uniqueness plugin
>
> Crash!  
>
> #0  0x00007f605b554c39 in __GI_raise (sig=sig@entry=6)
>     at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
> #1  0x00007f605b556348 in __GI_abort () at abort.c:89
> #2  0x00007f605b594d04 in __libc_message (do_abort=do_abort@entry=2, 
>     fmt=fmt@entry=0x7f605b69b528 "*** Error in `%s': %s: 0x%s ***\n")
>     at ../sysdeps/posix/libc_fatal.c:175
> #3  0x00007f605b59bff8 in malloc_printerr (ptr=<optimized out>, 
>     str=0x7f605b698cd7 "free(): invalid pointer", action=3) at malloc.c:4930
> #4  _int_free (av=0x7f605b8d7760 <main_arena>, p=<optimized out>, have_lock=0)
>     at malloc.c:3782
> #5  0x00007f605dd41302 in slapi_ch_free (ptr=0x7f6028001198)
>     at ../ds/ldap/servers/slapd/ch_malloc.c:363
> #6  0x00007f605dd4cc3d in slapi_sdn_done (sdn=0x7f6028001190)
>     at ../ds/ldap/servers/slapd/dn.c:2332
> #7  0x00007f605dd58be3 in slapi_entry_free (e=0x7f6028001190)
>     at ../ds/ldap/servers/slapd/entry.c:2044
> #8  0x00007f605dd3606d in op_shared_add (pb=0x7f60467fbb10)
>     at ../ds/ldap/servers/slapd/add.c:800
> #9  0x00007f605dd34d2e in do_add (pb=0x7f60467fbb10)
>     at ../ds/ldap/servers/slapd/add.c:258
> #10 0x0000000000416034 in connection_dispatch_operation (conn=0x7f605e167410, 
>     op=0xb36330, pb=0x7f60467fbb10) at ../ds/ldap/servers/slapd/connection.c:645
> #11 0x0000000000418043 in connection_threadmain ()
>     at ../ds/ldap/servers/slapd/connection.c:2534
> #12 0x00007f605c15be2b in _pt_root (arg=0x8acae0)
>     at ../../../nspr/pr/src/pthreads/ptthread.c:212
> #13 0x00007f605bafbf33 in start_thread (arg=0x7f60467fc700) at pthread_create.c:309
> #14 0x00007f605b613ded in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
>
>
> valgrind(also attached):
>
> ==14540== Invalid read of size 8
> ==14540==    at 0x4EA581F: factory_destroy_extension (factory.c:367)
> ==14540==    by 0x4E9CBD6: slapi_entry_free (entry.c:2043)
> ==14540==    by 0x4E7A06C: op_shared_add (add.c:800)
> ==14540==    by 0x4E78D2D: do_add (add.c:258)
> ==14540==    by 0x416033: connection_dispatch_operation (connection.c:645)
> ==14540==    by 0x418042: connection_threadmain (connection.c:2534)
> ==14540==    by 0x6B2FE2A: _pt_root (ptthread.c:212)
> ==14540==    by 0x716EF32: start_thread (pthread_create.c:309)
> ==14540==    by 0x768EDEC: clone (clone.S:111)
> ==14540==  Address 0xeb85910 is 160 bytes inside a block of size 184 free'd
> ==14540==    at 0x4C28577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==14540==    by 0x4E85301: slapi_ch_free (ch_malloc.c:363)
> ==14540==    by 0x4E9CCA6: slapi_entry_free (entry.c:2057)
> ==14540==    by 0x10BE7225: backentry_free (backentry.c:57)
> ==14540==    by 0x10BE9801: entrycache_return (cache.c:1159)
> ==14540==    by 0x10BE96A7: cache_return (cache.c:1132)
> ==14540==    by 0x10C25E65: ldbm_back_add (ldbm_add.c:1268)
> ==14540==    by 0x4E79E09: op_shared_add (add.c:735)
> ==14540==    by 0x4E78D2D: do_add (add.c:258)
> ==14540==    by 0x416033: connection_dispatch_operation (connection.c:645)
> ==14540==    by 0x418042: connection_threadmain (connection.c:2534)
> ==14540==    by 0x6B2FE2A: _pt_root (ptthread.c:212)
> ==14540==    by 0x716EF32: start_thread (pthread_create.c:309)
> ==14540==    by 0x768EDEC: clone (clone.S:111)

git patch file (master) -- CI test: add test case for ticket 47808
0001-Ticket-47808-CI-test-add-test-case-for-ticket-47808.patch

Bug description: Backend add ldbm_back_add frees the entry to be added,
if the add fails in the be_txn plugins. But the frontend releases the
entry when an error is returned from the backend.

Fix desxription: When the entry is freed in the backend, set NULL to
the pblock SLAPI_ADD_ENTRY to tell the frontend not to free the entry.

Looks good. Question, are del, mod, and modrdn operations affected by this as well, or only add operations?

Replying to [comment:4 mreynolds]:

Looks good. Question, are del, mod, and modrdn operations affected by this as well, or only add operations?

Thanks Mark! I tested mod and modrdn. They have no problem. Wondering what's the easy way to make del to fail in a plugin...

Replying to [comment:5 nhosoi]:

Replying to [comment:4 mreynolds]:

Looks good. Question, are del, mod, and modrdn operations affected by this as well, or only add operations?

Thanks Mark! I tested mod and modrdn. They have no problem. Wondering what's the easy way to make del to fail in a plugin...

Force RI plugin to fail?

Replying to [comment:6 mreynolds]:

Replying to [comment:5 nhosoi]:

Wondering what's the easy way to make del to fail in a plugin...
Force RI plugin to fail?
I "forced" to fail in the debugger. Del has no problem, too. whew :)

Replying to [comment:7 nhosoi]:

Replying to [comment:6 mreynolds]:

Replying to [comment:5 nhosoi]:

Wondering what's the easy way to make del to fail in a plugin...
Force RI plugin to fail?
I "forced" to fail in the debugger. Del has no problem, too. whew :)

Thanks for checking, ack!

Reviewed by Mark (Thank you!!)

Pushed to master:
a81a3ea..d50f994 master -> master
commit d50f994

Pushed to 389-ds-base-1.3.2:
945d113..1f3510a 389-ds-base-1.3.2 -> 389-ds-base-1.3.2
commit 1f3510a

Pushed to 389-ds-base-1.3.1:
2574dff..c6b5630 389-ds-base-1.3.1 -> 389-ds-base-1.3.1
commit c6b5630cd094e7a25d3f9c9ca347c4a652417756

Thank you, Thierry. I've pushed your patch to master.
286559d..36323c9 master -> master
commit 36323c9

Metadata Update from @nhosoi:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.3.1.23

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1139

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata