Bad pointer arithmetic in io.c (ITS#325)

[kst@sdsc.edu]

Full_Name: Keith Thompson
Version: 1.2.7
OS: Cray Unicos
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (132.249.22.138)


This is a problem we ran into in the UMich-LDAP implementation on the Cray T90,
as part of the Globus software package.  The bug also exists in OpenLDAP.
(Globus is going to move from UMich-LDAP to OpenLDAP.)

In ldap/libraries/liblber/io.c, line 562, function ber_get_next(), we see the
following line of code:

    toread = (unsigned long)ber->ber_end - (unsigned long)ber->ber_rwptr;

where toread is unsigned long, and ber_end and ber_rwptr are char*.  This code
assumes that char* pointers can be converted to an integer type and that
integer
arithmetic can be meaningfully performed on them.  This happens to work on most
machine architectures (where a pointer is internally represented as a simple
integer pointing to a single byte).  The C language does not guarantee this
behavior, and in fact it breaks down on the Cray T90.

The T90 is a word-addressed machine, where each word is 64 bits.  Byte pointers
(like char*) are represented (kludged) by setting the high-order 3 bits to
indicate
which byte within the word the pointer designates.  This means that pointer
on a char* has to treat the high-order 3 bits specially; plain integer
arithmetic
just won't work.  And, in fact, the above line compiles on the T90, but it
assigns garbage to toread.

In any case, there's simply no need to go through these contortions.  Pointer
arithmetic is built into the C language:
    pointer + integer --> pointer
    integer + pointer --> pointer
    pointer - integer --> pointer
    pointer - pointer --> integer (ptrdiff_t, in <stddef.h>)

The solution is simple: just drop the casts:

    toread = ber->ber_end - ber->ber_rwptr;

I haven't actually had a chance to test whether this works; we're using
UMich-LDAP
as part of Globus, and it's not quite working yet on the T90.  But I encourage
you to make the above change and run it through your testing procedures.

(BTW, please don't add this as a T90-only fix with #ifdef's.)

Thanks.