On 5/18/24 23:39, Pedro Giffuni wrote:
FWIW .. and let me be clear I haven't worked on this in ages and I am
not planning to retake this either...
clang just couldn't do the static fortify_source checks due to the way
llvm uses an intermediate representation; the size just couldn't be
handled in the preprocessor. Google did spend some time adding extra
attributes to clang to improve the debugging and you can see that
implemented in bionic libc but that was it. musl didn't even try.
Admittedly, I have no idea what you're talking about here; none of this
implementation requires any knowledge of anything at preproc time.
__builtin_object_size() does the right thing, and the typically
performance critical string/memory ops use __builtin___foo_chk() that do
successfully get optimized away in the common case to the underlying
foo() call. This all works very well with clang, I haven't tested it
under GCC but, as you've noted, would assume that it works at least as well.
fortify_source does replace some key libc functions with memory checking
alternatives and that turns out to be annoying when debugging. In a way
it breaks that principle C programmers once had, where developers are
expected to know what they are doing, and if the error is caught at
runtime by the stack protector anyways it ends up being redundant.
> One more thing about the static checks. Most of the linux distributions
out there indeed have built their software packages with GCC and
fortify_source >=2. As a consequence, when we ran an exp-run on the
ports tree (with GCC), fortify_source didn't find anything: it was
basically a waste of time.
Another reason for not setting it by default is performance. And here I
answer Shawn's comment on why not enable stack-protector-all and
safestack and fortify_source at the same time: running unnecessary
checks over and over again wastes energy and can have some performance
hit. The later may seem negligible in modern processors, but why do them
if they bring no benefit? (No need to answer ... just left as food for
thought)
Pedro.
On Saturday, May 18, 2024 at 09:08:52 PM GMT-5, Kyle Evans
<[email protected]> wrote:
On 5/18/24 20:09, Pedro Giffuni wrote:
> (sorry for top posting .. my mailer just sucks)
> Hi;
>
> I used to like the limited static checking FORTIFY_SOURCE provides and
> when I ran it over FreeBSD it did find a couple of minor issues. It only
> works for GCC though.
>
I don't think this is particularly true anymore; I haven't found a case
yet where __builtin_object_size(3) doesn't give me the correct size
while GCC did. I'd welcome counter-examples here, though -- we have
funding to both finish the project (widen the _FORTIFY_SOURCE net to
more of libc/libsys) and add tests to demonstrate that it's both
functional and correct. It would be useful to also document
deficiencies in the tests.
> I guess it doesn't really hurt to have FORTIFY_SOURCE around and NetBSD
> had the least intrusive implementation the last time I checked but I
> would certainly request it should never be activated by default,
> specially with clang. The GCC version has seen more development on glibc
> but I still think its a dead end.
>
I don't see a compelling reason to avoid enabling it by default; see
above, the functionality that we need in clang appears to be just fine
(and, iirc, was also fine when I checked at the beginning of working on
this in 2021) and it provides useful
> What I would like to see working on FreeBSD is Safestack as a
> replacement for the stack protector, which we were so very slow to adopt
> even when it was originally developed in FreeBSD. I think other projects
> based on FreeBSD (Chimera and hardenedBSD) have been using it but I
> don't know the details.
>
No comment there, though I think Shawn Webb / HardenedBSD had been
playing around with SafeStack (and might have enabled it? I haven't
actually looked in a while now).
> This is just all my $0.02
>
> Pedro.
Thanks,
Kyle Evans
>
> On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans
> <[email protected] <mailto:[email protected]>> wrote:
>
>
>
>
> On May 18, 2024 13:42, Pedro Giffuni <[email protected]
<mailto:[email protected]>> wrote:
>
> Oh no .. please not...
>
> We went into that in a GSoC:
>
>
https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions
<https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions> <https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions <https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions>>
>
>
> Ultimately it proved to be useless since stack-protector-strong.
>
>
> Respectfully, I disagree with your conclusion here:
>
> 1.) _FORTIFY_SOURCE provides more granular detection of overflow; I
> don't have to overflow all the way into the canary at the end of the
> frame to be detected, so my minor bug now can be caught before something
> causes the stack frame to be rearranged and turn it into a security
> issue later
>
> 2.) __builtin_object_size doesn't work on heap objects, but it actually
> can work on subobjects from a heap allocation (e.g., &foo->name), so the
> coverage extends beyond the stack into starting to detect other kinds of
> overflow
>
> While the security value over stack-protector-strong may be marginal (I
> won't debate this specifically), the feature still has value in general.
>
> Thanks,
>
> Kyle Evans
