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

[hyc@symas.com]

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