Re: [PATCH 2/5] __FD_ELT: Implement correct buffer overflow check

["Carlos O'Donell" <carlos@redhat.com>]

On 05/01/2013 04:11 PM, KOSAKI Motohiro wrote:
>>> Does compiling ruby (or similar code) with this header
>>> result in calls to __fdelt_buffer_warn or __fdelt_buffer_chk?
>>
>> Unfortunately, No. __builtin_object_size() require compiler know the
>> buffer size.
>> In the other words, it doesn't work if an allocate function and
>> FD_{SET,CLR} functions
>> doesn't exist in the same place. This is the same limitation with
>> other string buffer
>> overflow checks.
> 
> I inspected several other project codes.

Thank you very much for looking at these examples. They are quite
informative.

[snip]

> Summary: alomost software only need to add alloc_size() annotation to xmalloc() or 
> similar in almost case. but there are several exceptions. some software have a complicated
> fd size management and can't use __builtin_object_size(). but that's ok. In this case, the
> software correctly expand buffers by realloc() or similar, so there is no chance to happen
> buffer overflow.

So with your patch we enhance the number of cases that the check
is correct by using __bos0, and that's forward progress. I know
that it is less progress than you would like, but it is good
progress.

We keep _FORTIFY_SOURCE working usefully, even though it still
yields false positives.

The question as always with these checks is: Do you prefer false
positives or buffer overflows?

What's more harmful? The ecosystem thinking glibc and
the tools are "wrong" or buffer overflows leading to
security issues?

As a conservative project, and given the goal of _FORTIFY_SOURCE,
it seems like we have to leave the existing checks in place.

Cheers,
Carlos.