Date: Sun, 19 May 2024 00:12:58 -0500 From: Kyle Evans <kevans@FreeBSD.org> To: Pedro Giffuni <pfg@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD Message-ID: <216ca4a4-c0b8-4d72-bc6f-95e82a6b77da@FreeBSD.org> In-Reply-To: <1413980952.1357400.1716093599901@mail.yahoo.com> References: <02326b5e-a1fe-4411-a869-d21f9a76130c@email.android.com> <999469960.1638478.1716080957814@mail.yahoo.com> <6276b721-6c7b-41cd-9d1b-4169e86ec5e9@FreeBSD.org> <1413980952.1357400.1716093599901@mail.yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?216ca4a4-c0b8-4d72-bc6f-95e82a6b77da>