Last Comment Bug 399021 - doublefree in nsldapi_free_request
: doublefree in nsldapi_free_request
Status: RESOLVED FIXED
:
Product: Directory
Classification: Components
Component: LDAP C SDK (show other bugs)
: other
: All All
: -- normal (vote)
: ---
Assigned To: Mark Smith (:mcs)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-08 09:32 PDT by neuroc0der@gmail.com
Modified: 2007-10-08 15:17 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
one liner fix (338 bytes, patch)
2007-10-08 09:42 PDT, neuroc0der@gmail.com
no flags Details | Diff | Splinter Review
context diff. (965 bytes, patch)
2007-10-08 09:51 PDT, neuroc0der@gmail.com
no flags Details | Diff | Splinter Review
context diff. (965 bytes, patch)
2007-10-08 09:51 PDT, neuroc0der@gmail.com
mcs: review+
Details | Diff | Splinter Review

Description neuroc0der@gmail.com 2007-10-08 09:32:13 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.7) Gecko/20070914 Firefox/2.0.0.7
Build Identifier: 

this is basically a nasty doublefree in nsldapi_free_request when chasing
referrals. if chase fails due to connection error and we call ldap_unbind
to clean up nsldapi_free_request will free child request twice, once as
part of freeing request list one by one and the second time when freeing
child requests for each parent request.

normally this doublefree is not a problem, its harmless and since standard
libc free ignores doublefree in most implementations it happens without a
trace however some allocators such as mtmalloc, umem, watchmalloc and alike
dont take doublefree lightly and would either call abort() or generate some
term signal explicitly as doublefree is not allowed in those implementations
by default. this obviously does cause problems.

below is libumem audit and transaction output that illustrates the problem:

Status:         ready and active
Concurrency:    4
Logs:           transaction=64k 
Message buffer:
free(8098bc8): double-free or invalid buffer
stack trace:
libumem.so.1'umem_err_recoverable+0x39
libumem.so.1'process_free+0xe1
libumem.so.1'free+0x17
libldap60.so'?? (0xbff47bf4)
libldap60.so'ber_free+0x74
libldap60.so'?? (0xbff293a2)
libldap60.so'?? (0xbff292ef)
libldap60.so'?? (0xbff3d6ae)
libldap60.so'ldap_unbind+0x88
modify'do_modify+0x213
modify'main+0x3a
modify'_start+0x7a

            ADDR          BUFADDR        TIMESTAMP           THREAD
                            CACHE          LASTLOG         CONTENTS
         8071bb8          8098bc0     1da416de429a                1
                          8086010          807076c                0
                 libumem.so.1`umem_cache_free_debug+0x135
                 libumem.so.1`umem_cache_free+0x42
                 libumem.so.1`umem_free+0xd8
                 libumem.so.1`process_free+0x55
                 libumem.so.1`free+0x17
                 libldap60.so`nslberi_free+0x44
                 libldap60.so`ber_free+0x74
                 libldap60.so`nsldapi_free_request+0x192
                 libldap60.so`ldap_ld_free+0x15e
                 libldap60.so`ldap_unbind+0x88
                 do_modify+0x213
                 main+0x3a
                 _start+0x7a

         807076c          8098bc0     1da416c2f4fe                1
                          8086010                0                0
                 libumem.so.1`umem_cache_alloc_debug+0x14f
                 libumem.so.1`umem_cache_alloc+0x180
                 libumem.so.1`umem_alloc+0xc5
                 libumem.so.1`malloc+0x27
                 libumem.so.1`calloc+0x47
                 libldap60.so`nslberi_calloc+0x48
                 libldap60.so`ber_alloc_t+0x3c
                 libldap60.so`nsldapi_alloc_ber_with_options+0x104
                 libldap60.so`re_encode_request+0x24e
                 libldap60.so`chase_one_referral+0x3a7
                 libldap60.so`nsldapi_chase_v3_refs+0x178
                 libldap60.so`check_for_refs+0x193
                 libldap60.so`read1msg+0x4b7
                 libldap60.so`wait4msg+0x10e6
                 libldap60.so`nsldapi_result_nolock+0x111

the stack trace out of message buffer above maps to something like this in dbx:

  [1] _libc_kill(0x0, 0x6, 0x0, 0xffbfdc88, 0x0, 0xff30eaf4), at 0xff29fc08 
  [2] umem_do_abort(0x1, 0xa, 0xa000000, 0x7efefeff, 0x81010100, 0xff00), at 0xff303dcc 
  [3] umem_err_recoverable(0xff31064c, 0x41008, 0xff310630, 0xffbfdca0, 0x2c388, 0x534), at 0xff304074 
  [4] free(0x41008, 0x246e4, 0x21b3c, 0xff29792c, 0x3dc20, 0x4c), at 0xff303938 
=>[5] nslberi_free(ptr = 0x41008), line 1650 in "io.c"
  [6] ber_free(ber = 0x41008, freebuf = 1), line 327 in "io.c"
  [7] nsldapi_free_request(ld = 0x37bc8, lr = 0x3dc28, free_conn = 0), line 839 in "request.c"
  [8] nsldapi_free_request(ld = 0x37bc8, lr = 0x3dcc8, free_conn = 0), line 821 in "request.c"
  [9] ldap_ld_free(ld = 0x37bc8, serverctrls = (nil), clientctrls = (nil), close = 1), line 87 in "unbind.c"
  [10] ldap_unbind(ld = 0x37bc8), line 45 in "unbind.c"
  [11] do_modify(opt = 0xffbfe338), line 80 in "modify.c"
  [12] main(argc = 5, argv = 0xffbfe3b4), line 95 in "modify.c"


Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Comment 1 neuroc0der@gmail.com 2007-10-08 09:42:17 PDT
Created attachment 284008 [details] [diff] [review]
one liner fix

this one liner should be safe to get rid of the doublefree without causing any 
leaks when freeing requests but i might be missing something so please review.
Comment 2 neuroc0der@gmail.com 2007-10-08 09:51:32 PDT
Created attachment 284009 [details] [diff] [review]
context diff.

same patch but with some context which i stupidly missed the first time.
Comment 3 neuroc0der@gmail.com 2007-10-08 09:51:38 PDT
Created attachment 284010 [details] [diff] [review]
context diff.

same patch but with some context which i stupidly missed the first time.
Comment 4 Rich Megginson 2007-10-08 09:53:35 PDT
(In reply to comment #3)
> Created an attachment (id=284010) [details]
> context diff.
> 
> same patch but with some context which i stupidly missed the first time.
> 

Looks good.
Comment 5 Mark Smith (:mcs) 2007-10-08 10:04:29 PDT
Comment on attachment 284010 [details] [diff] [review]
context diff.

OK.  I think this is a safe fix.  Thanks for tracking this down; I am surprised we have not found it before.
Comment 6 neuroc0der@gmail.com 2007-10-08 10:19:45 PDT
thanx Rich and Mark! the credit for tracking this down goes to Pierre Rogier of
our DS sustaining team. we have seen this as early as 2004 i think with our Msg
server because they link with mtmalloc by default however it is rather rare in
occurrence because it requires specific DS setup to reproduce. Pierre been able
to put such DS setup in place and write the testcase for this.
Comment 7 neuroc0der@gmail.com 2007-10-08 15:17:04 PDT
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/request.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/request.c,v  <--  request.c
new revision: 5.10; previous revision: 5.9
done

Note You need to log in before you can comment on or make changes to this bug.