Re: (ITS#3388) Seg11 when used with pam_ldap under dtlogin

[jdb@force10networks.com]

Will dig into the realloc call and try to figure that out.  the 
ber->ber_ptr==0x6 makes a lot more sense now...

Thanks.

- joel



On Nov 10, 2004, at 4:21 PM, Howard Chu wrote:

> Joel Boutros wrote:
>
>> Hi, Howard,
>>
>> I've tried to put this together to make it more clear what the 
>> problem I'm seeing is...  This is the call tree which results in the 
>> bug.  As things are pushed onto the stack, I indent.  When it's done, 
>> I un-indent.  if something allocates, i mark it with a -->.  I've 
>> removed anything that isn't relevant to the problem at hand (ie, 
>> assert()s, error checks, unrelated assignments, etc).
>>
>> If you've got alternatives or can explain why this isn't the cause of 
>> the crash, please!, i'm very interested.  I'd really like to get this 
>> problem solved, so a refutal would tell me where i should look 
>> instead.  As it stands, this "feels" like an OpenLDAP issue.
>
> The fact is that ldap_build_search_req is always called by 
> ldap_search, and if it was broken, every ldap search request would be 
> crashing. Since ldapsearch works, and the entire OpenLDAP test suite 
> works in any given release, the problem must be elsewhere.
>
>>
>> pam_ldap code
>>     ldap_search_s
>>         ldap_search
>>             ldap_build_search_req
>>                 ldap_alloc_ber_with_options
>>                     ber_alloc_t
>>                         LBER_CALLOC   --> ber
>>                         ber->valid = LBER_VALID_BERELEMENT;
>>                         ber->ber_tag = LBER_DEFAULT;
>>                         ber->ber_options = options;
>>                         ber->ber_debug = ber_int_debug;
>>                 LDAP_NEXT_MSGID
>>                 ber_printf(ld, "{it{seeiib", ...)
>>                     ber_start_seq
>>                         ber_start_seqorset
>>                             ber_mamcalloc_x  --> new
>> 1                            new->sos_first = ber->ber_ptr;
>> 1                            new->sos_ptr = new->sos_first + 
>> ber_calc_taglen(tag) + FOUR_BYTE_LEN;
>>                             ber->ber_sos = new
>>                     ber_put_int
>>                         ber_put_int_or_enum
>>                             ber_put_tag
>>                                 ber_write
>>                                     if ( nosos || ber->ber_sos == 
>> NULL ) {   ber->ber_sos same as new above, so !NULL
>> 2                                    if ( ber->ber_ptr + len > 
>> ber->ber_end ) { }  ber->ber_ptr == 0, ber->ber_end == 0
>> 3                                    ber_realloc( ber, len )
>>                                     }
>>                                     AC_MEMCPY( ber->ber_ptr, buf, 
>> (size_t)len );
>>                                     ber->ber_ptr += len;
>>
>>
>>
>> The two lines marked (1) lines are the problem, as far as i can tell. 
>>  ber->ber_ptr is NULL.  Nothing ever set it to non-NULL after the 
>> calloc in ber_alloc_t.  So new->sos_first is 0, ber_calc_taglen(tag) 
>> == 1, and FOUR_BYTE_LEN == 5.  new->sos_ptr becomes 0x6, and then we 
>> dereference it in ber_write()'s memcpy (much later).
>>
>> It isn't clear what the results of (2) are, but it seems likely to 
>> evaluate true, as ber->ber_ptr is 6, len is at least something, and 
>> ber->ber_end is probably (never got set to anything else).
>
> 2 will always be TRUE since ber->ber_end is zero/NULL. ber_realloc 
> will be called, which will realloc the ber->ber_buf. Since 
> ber->ber_buf is still NULL at that point, this is semantically the 
> same as a malloc.
>
>> ber->berber_realloc (3) is the only place that ber->ber_ptr can 
>> possibly move.  It getting moved depends on ber_memrealloc_x() 
>> returning moved memory.  It isn't clear how it really behaves here, 
>> though -- if it moves at all, ber->ber_ptr = (ber->ber_buf + 
>> (ber->ber_ptr - oldbuf)); when ber->ber_ptr is 0x6, and we're 
>> subtracting oldbuf.  We'll end up with total trash on ber->ber_ptr.  
>> That's if it moves.  If it doesn't, ber->ber_ptr continues to be 0x6, 
>> which is what i've seen in the AC_MEMCPY() call.
>
>> Maybe this problem only comes up when ber_memrealloc_x() returns a 
>> moved region.  This would explain why i see it under dtlogin, but 
>> don't usually see it otherwise.  I have verified that ber->ber_ptr is 
>> 0x6 at the point ber_printf() is called when debugging under a 
>> working case, but haven't had a lot of luck identifying why it then 
>> continues to work (my "working" test case involves time limits 
>> because it's under login(1), and i just can't get to the call quickly 
>> enough).
>
> Since the oldbuf was NULL, if ber_memrealloc_x succeeds then any 
> returned memory counts as a moved region. At that point the ber_sos 
> pointers are adjusted, and they are completely valid when ber_realloc 
> returns. If ber_memrealloc_x fails then it will return a NULL pointer 
> which will cause ber_realloc to return a failure code. That failure 
> gets propagated back to the original caller.
>
>> Again, thanks for your time, I realize i'm being more irritating at 
>> following this up than most people are.  I'm just firmly convinced 
>> i'm right, that this really is an OpenLDAP issue, as, so far, there's 
>> something to suggest there's a different issue at-fault.
>
> I suggest you single-step through ber_realloc and see what is really 
> happening.
>
> -- 
>  -- Howard Chu
>  Chief Architect, Symas Corp.       Director, Highland Sun
>  http://www.symas.com               http://highlandsun.com/hyc
>  Symas: Premier OpenSource Development and Support
>