Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

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 
> <kevans@freebsd.org> 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
>  > <kaevans@fastmail.com <mailto:kaevans@fastmail.com>> wrote:
>  >
>  >
>  >
>  >
>  > On May 18, 2024 13:42, Pedro Giffuni <pfg@freebsd.org 
> <mailto:pfg@freebsd.org>> 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
>