[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#5578) sortvals error



On Thu, Jun 26, 2008 at 10:00:00AM +0000, Andrew Findlay wrote:

> I have uploaded a proposed fix for this:
> 
> ftp://ftp.openldap.org/incoming/andrew-findlay-sortvals-20080626.patch
> 
> My reasoning is that the break statement terminates the loop before
> the last value in the list has been compared with the assertion value.
> Thus, even if the last value matches, the loop ends with -1 in 'match'.

Hmm: I now have a crasher in attr_valadd when adding a member to a
group that already contains two members. Not sure yet whether I caused it
with the minus-one-line fix above, or whether I have uncovered a new
problem.

I would expect the new value to sort at the front of the list.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1518945392 (LWP 1431)]
0x0806e650 in attr_valadd (a=0xa6c71084, vals=0x847a148, nvals=0x847a190, nn=1) at attr.c:410
410                                             a->a_nvals[j+1] = a->a_nvals[j];
(gdb) where
#0  0x0806e650 in attr_valadd (a=0xa6c71084, vals=0x847a148, nvals=0x847a190, nn=1) at attr.c:410
#1  0x080b4db3 in modify_add_values (e=0xa5769cb0, mod=0x8479a98, permissive=0, text=0xa576b17c, textbuf=0xa5769bb0 "",
    textlen=256) at mods.c:155
#2  0x080d0d4e in bdb_modify_internal (op=0x83964e0, tid=0x847b400, modlist=0x8479a98, e=0xa5769cb0, text=0xa576b17c,
    textbuf=0xa5769bb0 "", textlen=256) at modify.c:101
#3  0x080d1926 in bdb_modify (op=0x83964e0, rs=0xa576b168) at modify.c:578
#4  0x080c6b21 in overlay_op_walk (op=0x83964e0, rs=0xa576b168, which=op_modify, oi=0x82f9a18, on=0x82fa0c8) at backover.c:646
#5  0x080c709d in over_op_func (op=0x83964e0, rs=0xa576b168, which=op_modify) at backover.c:698
#6  0x0807dd36 in fe_op_modify (op=0x83964e0, rs=0xa576b168) at modify.c:300
#7  0x0807e6a8 in do_modify (op=0x83964e0, rs=0xa576b168) at modify.c:175
#8  0x08065646 in connection_operation (ctx=0xa576b238, arg_v=0x83964e0) at connection.c:1084
#9  0x08065c42 in connection_read_thread (ctx=0xa576b238, argv=0x1f) at connection.c:1211
#10 0x0817e454 in ldap_int_thread_pool_wrapper (xpool=0x82b4310) at tpool.c:663
#11 0xb7c19112 in start_thread () from /lib/libpthread.so.0
#12 0xb7ba52ee in clone () from /lib/libc.so.6
(gdb) print j
$1 = -305887
(gdb) print a
$2 = (Attribute *) 0xa6c71084
(gdb) print *a
$3 = {a_desc = 0x82e12e8, a_vals = 0x847ba28, a_nvals = 0x847b6e8, a_numvals = 2, a_flags = 16, a_next = 0xa6c7bd34}
(gdb) print slot
$4 = 0
(gdb) print nvals
$5 = (BerVarray) 0x847a190
(gdb)

The relevant code block is this:

        /* If sorted and old vals exist, must insert */
        if (( a->a_flags & SLAP_ATTR_SORTED_VALS ) && a->a_numvals ) {
                unsigned slot;
                int j, rc;
                v2 = nvals ? nvals : vals;
                for ( i = 0; i < nn; i++ ) {
                        rc = attr_valfind( a, SLAP_MR_EQUALITY | SLAP_MR_VALUE_OF_ASSERTION_SYNTAX |
                                SLAP_MR_ASSERTED_VALUE_NORMALIZED_MATCH | SLAP_MR_ATTRIBUTE_VALUE_NORMALIZED_MATCH,
                                &v2[i], &slot, NULL );
                        if ( rc != LDAP_NO_SUCH_ATTRIBUTE ) {
                                /* should never happen */
                                if ( rc == LDAP_SUCCESS )
                                        rc = LDAP_TYPE_OR_VALUE_EXISTS;
                                return rc;
                        }
                        for ( j = a->a_numvals; j >= slot; j-- ) {
                                a->a_vals[j+1] = a->a_vals[j];			<==================== Crash here
                                if ( nvals )
                                        a->a_nvals[j+1] = a->a_nvals[j];
                        }
                        ber_dupbv( &a->a_nvals[slot], &v2[i] );
                        if ( nvals )
                                ber_dupbv( &a->a_vals[slot], &vals[i] );
                        a->a_numvals++;
                }
                BER_BVZERO( &a->a_vals[a->a_numvals] );
                if ( a->a_vals != a->a_nvals )
                        BER_BVZERO( &a->a_nvals[a->a_numvals] );
        } else {

I cannot see how j got that large negative value. It should have started the
loop at 2 (a->a_numvals) and decremented until it reached zero.
Must have been overwritten somewhere.

Andrew
-- 
-----------------------------------------------------------------------
|                 From Andrew Findlay, Skills 1st Ltd                 |
| Consultant in large-scale systems, networks, and directory services |
|     http://www.skills-1st.co.uk/                +44 1628 782565     |
-----------------------------------------------------------------------