399021 - doublefree in nsldapi_free_request

Status: RESOLVED FIXED

(Directory :: LDAP C SDK, defect)

Description

[Anton Bobrov]

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

[Anton Bobrov]

Attachment: 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

[Anton Bobrov]

Attachment: context diff.

same patch but with some context which i stupidly missed the first time.

Comment 3

[Anton Bobrov]

Attachment: context diff.

same patch but with some context which i stupidly missed the first time.

Comment 4

[Rich Megginson]

(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]]

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

[Anton Bobrov]

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

[Anton Bobrov]

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