This is the mail archive of the
libc-ports@sources.redhat.com
mailing list for the libc-ports project.
Re: [PATCH 2/5] __FD_ELT: Implement correct buffer overflow check
- From: KOSAKI Motohiro <kosaki dot motohiro at gmail dot com>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: KOSAKI Motohiro <kosaki dot motohiro at gmail dot com>, libc-alpha <libc-alpha at sourceware dot org>, "libc-ports at sourceware dot org" <libc-ports at sourceware dot org>
- Date: Wed, 01 May 2013 16:31:55 -0400
- Subject: Re: [PATCH 2/5] __FD_ELT: Implement correct buffer overflow check
- References: <1365900451-19026-1-git-send-email-kosaki dot motohiro at gmail dot com> <1365900451-19026-3-git-send-email-kosaki dot motohiro at gmail dot com> <518080FD dot 1090402 at redhat dot com> <CAHGf_=pDgABHdv5RKd6U870J1t1gM6GhbDpxGoQMjJEsMPHgLQ at mail dot gmail dot com> <518129C7 dot 2020808 at redhat dot com>
>>> 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.
>
> Then we need a flag, and ruby needs to use the flag to disable the
> check on Linux.
>
> The fundamental truth is that glibc implements POSIX, not "Linux."
> And in POSIX there is a limit of FD_SETSIZE.
>
> The default checking should be for POSIX.
>
> We should provide a way to disable _FORTIFY_SOURCE checks that
> are POSIX-only.
>
> I still think your current macro is *better* because if __bos0
> works then you have a dynamic check that is better than a static
> check.
>
> Thus the final solution is a combination of your new __bos0
> changes and a flag to disable the check in the event that __bos0
> fails.
>
> What do you think?
Hmmm....
I'm puzzuled why you started to talk about ruby again. In ruby case,
recompilation and flag are ok. That's no problem.
But, as we've alread seen, several other software also uses the same technique.
and if not disable, Ubuntu need to recompile all of their packages. Do you
suggest to recompile all?
Moreover, IMHO fallbacking static check is completely useless because compiler
always can know the exact buffer size when using fd_set on stack. That's easy task
to distingush static array size form point of compiler view. In the other
hands, if compipler need to fall back, the buffer was allocated from heap in 99%
case. and when using buffers allocated from heap, the size is larger than 1024
in almost all case. Then evetually, static check fallbacks makes false positive
aborting in almost all case.
Do you disagree?
I guess my conservative and your conservative are slightly different. My conservative
meant not to make false positive aborting. Your conservative seems preserve old behavior
as far as possible. In general, I agree with you. but in this case, I don't think __bos0()
fails to preserve to detect wrong FD_SET usage for buffers on stack. Do you have any
specific (and practical) examples that my code fails to work?
# I know several hacky code _can_ trick my code. but I have not found practical and real world
# example.
But again, It's ok from ruby POV and I'm not argue if you really want to do it.