Re: [IDEA+RFC] Possible solution for min()/max() war

[Jamie Lokier]

Linus Torvalds wrote:
> > 	if (len <= (int) sizeof(short) || len > (int) sizeof(*sunaddr))
> 
> You're so full of shit that it's incredible.
> 
> I'mnot going to argue this, when people call stuff like the above the
> "natural way". This is not worth it.

While I agree with Linus that the above line is ugly, there is a problem
with the original line:

    if (len <= sizeof(short) || len > sizeof(*sunaddr))

The problem?  Thinking this is natural, suppose you decide you only need
to check len against sizeof(short), perhaps here, perhaps copying this
idea to another part of the program:

    if (len <= sizeof(short))

_This_ code has a bug.  The comparison is unsigned; i.e. len is
converted to unsigned int first, so if the user passes a negative value,
the comparison returns false and the code will break.  It only works in
the original example because the second comparison checks the bogus
results of the first one.

Now, any later use of len should not lead to a buffer overflow or
anything, because you always check against a maximum size for copying,
yes?

No.  Consider this hypothetical code to copy things up to a page at a
time.  len is a signed type s.t. sizeof(len) <= sizeof(size_t):

   if (len <= sizeof(struct thing) || len > MAX_BUFFER_SIZE) {
           printk(KERN_ERR "Sanity check failed!\n");
           goto out;
   }
   memcpy (to, from, len);

Inexperienced C programmers, and sleepy ones, may not see the fault.

cheers,
-- Jamie