RE: commit: ldap/servers/slapd/back-ldbm attr.c

["Howard Chu" <hyc@highlandsun.com>]

> -----Original Message-----
> From: Hallvard B Furuseth [mailto:h.b.furuseth@usit.uio.no]

Pierangelo and I were just having a conversation about the correct fix for
this... Definitely the last commits are unsafe, particularly on platforms
where sizeof(int) != sizeof(pointer).

> hyc@OpenLDAP.org writes:
> > Modified Files:
> > 	attr.c  1.33 -> 1.34

> Also back-ldap/bind.c 1.52 -> 1.53, back-bdb/attr.c 1.13 -> 1.14
>
> > Log Message:
> > Fix AVL comparisons
>
> > -       return desc - a->ai_desc;
> > +       return (unsigned)desc - (unsigned)a->ai_desc;
>
> I don't know what this is supposed to fix, but:
>
> (a) Both new and original code gives the wrong result or signals an
>     overflow for pointers to memory locations that are more than
>     INT_MAX bytes or so apart.
>
> (b) Pointer comparison is only defined between pointers to the same
>     array.  The result of the original code is undefined if desc and
>     a->ai_desc point to the results of different malloc operations.
>
> (c) The result of casting a pointer to an integer is implementation-
>     defined.  I think it might e.g. return 42 for any pointer.
>
> There is no way to fix all three problems.
>
> This fixes (a) and (c):
>
>     if (desc < a->ai_desc)
>         return -1;
>     return (desc != a-ai_desc);

This is probably fine.

Problem (b) is true per the ANSI spec, but is not significant here. The
elements of the AVL tree are all homogeneous - in back-ldap they are
connection pointers, and the connections are all elements of a single static
array. The AttributeDescriptions being compared in back-bdb and back-ldbm are
all created by the same malloc function, as is most of the memory used by
slapd. It all comes from one heap, which is a single array. Also for this
reason, problem (a) is not relevant; the range of addresses being compared
will never be so far apart.

  -- Howard Chu
  Chief Architect, Symas Corp.       Director, Highland Sun
  http://www.symas.com               http://highlandsun.com/hyc
  Symas: Premier OpenSource Development and Support