Date: Sun, 19 May 2024 14:47:22 +0000 (UTC) From: Pedro Giffuni <pfg@freebsd.org> To: Kyle Evans <kevans@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: <418178403.1722964.1716130042183@mail.yahoo.com> In-Reply-To: <216ca4a4-c0b8-4d72-bc6f-95e82a6b77da@FreeBSD.org> 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> <216ca4a4-c0b8-4d72-bc6f-95e82a6b77da@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
------=_Part_1722963_885190984.1716130042180 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hmm... well. In all honesty I understand I am doomed to lose this battle :).FORTIFY_SOUR= CE is in linux and in Apple and that weights enough that it had to find it'= s way to FreeBSD sooner or later. Plus I am just not much involved in FreeB= SD or OSs anymore so I don't feel like stopping other people from doing dev= elopment. best of lucks! Pedro. ps. Just for the reference, the Google guys did develop a document on how t= hey implemented this for clang on bionic:https://goo.gl/8HS2dW On Sunday, May 19, 2024 at 12:13:00 AM GMT-5, Kyle Evans <kevans@freebs= d.org> wrote: =20 =20 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=20 > not planning to retake this either... >=20 > clang just couldn't do the static=C2=A0 fortify_source checks=C2=A0 due t= o the way=20 > llvm uses an intermediate representation; the size just couldn't be=20 > handled in the preprocessor. Google did spend some time adding extra=20 > attributes to clang to improve the debugging and you can see that=20 > implemented in bionic libc but that was it. musl didn't even try. >=20 Admittedly, I have no idea what you're talking about here; none of this=20 implementation requires any knowledge of anything at preproc time.=20 __builtin_object_size() does the right thing, and the typically=20 performance critical string/memory ops use __builtin___foo_chk() that do=20 successfully get optimized away in the common case to the underlying=20 foo() call.=C2=A0 This all works very well with clang, I haven't tested it= =20 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= =20 > alternatives and that turns out to be annoying when debugging. In a way= =20 > it breaks that principle C programmers once had, where developers are=20 > expected to know what they are doing, and if the error is caught at=20 > 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=20 > fortify_source >=3D2. As a consequence, when we ran an exp-run on the=20 > ports tree (with GCC), fortify_source didn't find anything: it was=20 > basically a waste of time. >=20 > Another reason for not setting it by default is performance. And here I= =20 > answer Shawn's comment on why not enable stack-protector-all and=20 > safestack and fortify_source at the same time: running unnecessary=20 > checks over and over again wastes energy and can have some performance=20 > hit. The later may seem negligible in modern processors, but why do them= =20 > if they bring no benefit? (No need to answer ... just left as food for=20 > thought) >=20 > Pedro. >=20 > On Saturday, May 18, 2024 at 09:08:52 PM GMT-5, Kyle Evans=20 > <kevans@freebsd.org> wrote: >=20 >=20 >=20 >=20 > On 5/18/24 20:09, Pedro Giffuni wrote: >=C2=A0 > (sorry for top posting .. my mailer just sucks) >=C2=A0 > Hi; >=C2=A0 > >=C2=A0 > I used to like the limited static checking FORTIFY_SOURCE provide= s and >=C2=A0 > when I ran it over FreeBSD it did find a couple of minor issues. = It only >=C2=A0 > works for GCC though. >=C2=A0 > >=20 > 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.=C2=A0 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.=C2=A0 It would be useful to also document > deficiencies in the tests. >=20 >=C2=A0 > I guess it doesn't really hurt to have FORTIFY_SOURCE around and = NetBSD >=C2=A0 > had the least intrusive implementation the last time I checked bu= t I >=C2=A0 > would certainly request it should never be activated by default, >=C2=A0 > specially with clang. The GCC version has seen more development o= n glibc >=C2=A0 > but I still think its a dead end. >=C2=A0 > >=20 > 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 >=20 >=C2=A0 > What I would like to see working on FreeBSD is Safestack as a >=C2=A0 > replacement for the stack protector, which we were so very slow t= o adopt >=C2=A0 > even when it was originally developed in FreeBSD. I think other p= rojects >=C2=A0 > based on FreeBSD (Chimera and hardenedBSD) have been using it but= I >=C2=A0 > don't know the details. >=C2=A0 > >=20 > 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). >=20 >=C2=A0 > This is just all my $0.02 >=C2=A0 > >=C2=A0 > Pedro. >=20 > Thanks, >=20 > Kyle Evans >=20 >=C2=A0 > >=C2=A0 > On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans >=C2=A0 > <kaevans@fastmail.com <mailto:kaevans@fastmail.com>> wrote: >=C2=A0 > >=C2=A0 > >=C2=A0 > >=C2=A0 > >=C2=A0 > On May 18, 2024 13:42, Pedro Giffuni <pfg@freebsd.org=20 > <mailto:pfg@freebsd.org>> wrote: >=C2=A0 > >=C2=A0 >=C2=A0 =C2=A0 Oh no .. please not... >=C2=A0 > >=C2=A0 >=C2=A0 =C2=A0 We went into that in a GSoC: >=C2=A0 > >=C2=A0 >=20 > https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions= =20 > <https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions>= <https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions <= https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions>>; >=C2=A0 > >=C2=A0 > >=C2=A0 >=C2=A0 =C2=A0 Ultimately it proved to be useless since stack-prote= ctor-strong. >=C2=A0 > >=C2=A0 > >=C2=A0 > Respectfully, I disagree with your conclusion here: >=C2=A0 > >=C2=A0 > 1.) _FORTIFY_SOURCE provides more granular detection of overflow;= I >=C2=A0 > don't have to overflow all the way into the canary at the end of = the >=C2=A0 > frame to be detected, so my minor bug now can be caught before so= mething >=C2=A0 > causes the stack frame to be rearranged and turn it into a securi= ty >=C2=A0 > issue later >=C2=A0 > >=C2=A0 > 2.) __builtin_object_size doesn't work on heap objects, but it ac= tually >=C2=A0 > can work on subobjects from a heap allocation (e.g., &foo->name),= so the >=C2=A0 > coverage extends beyond the stack into starting to detect other k= inds of >=C2=A0 > overflow >=C2=A0 > >=C2=A0 > While the security value over stack-protector-strong may be margi= nal (I >=C2=A0 > won't debate this specifically), the feature still has value in g= eneral. >=C2=A0 > >=C2=A0 > Thanks, >=C2=A0 > >=C2=A0 > Kyle Evans >=C2=A0=20 =20 ------=_Part_1722963_885190984.1716130042180 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable <html><head></head><body><div class=3D"ydpeac7e5yahoo-style-wrap" style=3D"= font-family:Helvetica Neue, Helvetica, Arial, sans-serif;font-size:16px;"><= div></div> <div dir=3D"ltr" data-setdir=3D"false">Hmm... well.</div><div dir= =3D"ltr" data-setdir=3D"false"><br></div><div dir=3D"ltr" data-setdir=3D"fa= lse">In all honesty I understand I am doomed to lose this battle :).</div><= div dir=3D"ltr" data-setdir=3D"false">FORTIFY_SOURCE is in linux and in App= le and that weights enough that it had to find it's way to FreeBSD sooner o= r later. Plus I am just not much involved in FreeBSD or OSs anymore so I do= n't feel like stopping other people from doing development.</div><div dir= =3D"ltr" data-setdir=3D"false"><br></div><div dir=3D"ltr" data-setdir=3D"fa= lse">best of lucks!</div><div dir=3D"ltr" data-setdir=3D"false"><br></div><= div dir=3D"ltr" data-setdir=3D"false">Pedro.</div><div dir=3D"ltr" data-set= dir=3D"false"><br></div><div dir=3D"ltr" data-setdir=3D"false"><div><div di= r=3D"ltr" data-setdir=3D"false" style=3D"color: rgb(0, 0, 0); font-family: = Helvetica Neue, Helvetica, Arial, sans-serif; font-size: 16px; outline: non= e !important;">ps. Just for the reference, the Google guys did develop a do= cument on how they implemented this for clang on bionic:</div><div dir=3D"l= tr" data-setdir=3D"false" style=3D"color: rgb(0, 0, 0); font-family: Helvet= ica Neue, Helvetica, Arial, sans-serif; font-size: 16px; outline: none !imp= ortant;"><a href=3D"https://goo.gl/8HS2dW" style=3D"color: rgb(25, 106, 212= ); text-decoration-line: underline; outline: none !important;" rel=3D"nofol= low" target=3D"_blank">https://goo.gl/8HS2dW</a></div></div><br></div><div>= <br></div> =20 </div><div id=3D"ydp256e56f3yahoo_quoted_7121969370" class=3D"ydp25= 6e56f3yahoo_quoted"> <div style=3D"font-family:'Helvetica Neue', Helvetica, Arial, s= ans-serif;font-size:13px;color:#26282a;"> =20 <div> On Sunday, May 19, 2024 at 12:13:00 AM GMT-5, Kyle = Evans <kevans@freebsd.org> wrote: </div> <div><br></div> <div><br></div> =20 =20 <div><div dir=3D"ltr">On 5/18/24 23:39, Pedro Giffuni wrote= :<br></div><div dir=3D"ltr">> FWIW .. and let me be clear I haven't work= ed on this in ages and I am <br></div><div dir=3D"ltr">> not planning to= retake this either...<br></div><div dir=3D"ltr">> <br></div><div dir=3D= "ltr">> clang just couldn't do the static fortify_source checks&nb= sp; due to the way <br></div><div dir=3D"ltr">> llvm uses an intermediat= e representation; the size just couldn't be <br></div><div dir=3D"ltr">>= handled in the preprocessor. Google did spend some time adding extra <br><= /div><div dir=3D"ltr">> attributes to clang to improve the debugging and= you can see that <br></div><div dir=3D"ltr">> implemented in bionic lib= c but that was it. musl didn't even try.<br></div><div dir=3D"ltr">> <br= ></div><div dir=3D"ltr"><br></div><div dir=3D"ltr">Admittedly, I have no id= ea what you're talking about here; none of this <br></div><div dir=3D"ltr">= implementation requires any knowledge of anything at preproc time. <br></di= v><div dir=3D"ltr">__builtin_object_size() does the right thing, and the ty= pically <br></div><div dir=3D"ltr">performance critical string/memory ops u= se __builtin___foo_chk() that do <br></div><div dir=3D"ltr">successfully ge= t optimized away in the common case to the underlying <br></div><div dir=3D= "ltr">foo() call. This all works very well with clang, I haven't test= ed it <br></div><div dir=3D"ltr">under GCC but, as you've noted, would assu= me that it works at least as well.<br></div><div dir=3D"ltr"><br></div><div= dir=3D"ltr">> fortify_source does replace some key libc functions with = memory checking <br></div><div dir=3D"ltr">> alternatives and that turns= out to be annoying when debugging. In a way <br></div><div dir=3D"ltr">>= ; it breaks that principle C programmers once had, where developers are <br= ></div><div dir=3D"ltr">> expected to know what they are doing, and if t= he error is caught at <br></div><div dir=3D"ltr">> runtime by the stack = protector anyways it ends up being redundant.<br></div><div dir=3D"ltr">>= ; > One more thing about the static checks. Most of the linux distributi= ons<br></div><div dir=3D"ltr">> out there indeed have built their softwa= re packages with GCC and <br></div><div dir=3D"ltr">> fortify_source >= ;=3D2. As a consequence, when we ran an exp-run on the <br></div><div dir= =3D"ltr">> ports tree (with GCC), fortify_source didn't find anything: i= t was <br></div><div dir=3D"ltr">> basically a waste of time.<br></div><= div dir=3D"ltr">> <br></div><div dir=3D"ltr">> Another reason for not= setting it by default is performance. And here I <br></div><div dir=3D"ltr= ">> answer Shawn's comment on why not enable stack-protector-all and <br= ></div><div dir=3D"ltr">> safestack and fortify_source at the same time:= running unnecessary <br></div><div dir=3D"ltr">> checks over and over a= gain wastes energy and can have some performance <br></div><div dir=3D"ltr"= >> hit. The later may seem negligible in modern processors, but why do t= hem <br></div><div dir=3D"ltr">> if they bring no benefit? (No need to a= nswer ... just left as food for <br></div><div dir=3D"ltr">> thought)<br= ></div><div dir=3D"ltr">> <br></div><div dir=3D"ltr">> Pedro.<br></di= v><div dir=3D"ltr">> <br></div><div dir=3D"ltr">> On Saturday, May 18= , 2024 at 09:08:52 PM GMT-5, Kyle Evans <br></div><div dir=3D"ltr">> <= ;<a href=3D"mailto:kevans@freebsd.org" rel=3D"nofollow" target=3D"_blank">k= evans@freebsd.org</a>> wrote:<br></div><div dir=3D"ltr">> <br></div><= div dir=3D"ltr">> <br></div><div dir=3D"ltr">> <br></div><div dir=3D"= ltr">> <br></div><div dir=3D"ltr">> On 5/18/24 20:09, Pedro Giffuni w= rote:<br></div><div dir=3D"ltr">> > (sorry for top posting .. m= y mailer just sucks)<br></div><div dir=3D"ltr">> > Hi;<br></div= ><div dir=3D"ltr">> ><br></div><div dir=3D"ltr">> >= I used to like the limited static checking FORTIFY_SOURCE provides and<br>= </div><div dir=3D"ltr">> > when I ran it over FreeBSD it did fi= nd a couple of minor issues. It only<br></div><div dir=3D"ltr">> &= gt; works for GCC though.<br></div><div dir=3D"ltr">> ><br></di= v><div dir=3D"ltr">> <br></div><div dir=3D"ltr">> I don't think this = is particularly true anymore; I haven't found a case<br></div><div dir=3D"l= tr">> yet where __builtin_object_size(3) doesn't give me the correct siz= e<br></div><div dir=3D"ltr">> while GCC did. I'd welcome counter-e= xamples here, though -- we have<br></div><div dir=3D"ltr">> funding to b= oth finish the project (widen the _FORTIFY_SOURCE net to<br></div><div dir= =3D"ltr">> more of libc/libsys) and add tests to demonstrate that it's b= oth<br></div><div dir=3D"ltr">> functional and correct. It would b= e useful to also document<br></div><div dir=3D"ltr">> deficiencies in th= e tests.<br></div><div dir=3D"ltr">> <br></div><div dir=3D"ltr">>&nbs= p; > I guess it doesn't really hurt to have FORTIFY_SOURCE around and Ne= tBSD<br></div><div dir=3D"ltr">> > had the least intrusive impl= ementation the last time I checked but I<br></div><div dir=3D"ltr">>&nbs= p; > would certainly request it should never be activated by default,<br= ></div><div dir=3D"ltr">> > specially with clang. The GCC versi= on has seen more development on glibc<br></div><div dir=3D"ltr">> = > but I still think its a dead end.<br></div><div dir=3D"ltr">> = ><br></div><div dir=3D"ltr">> <br></div><div dir=3D"ltr">> I don'= t see a compelling reason to avoid enabling it by default; see<br></div><di= v dir=3D"ltr">> above, the functionality that we need in clang appears t= o be just fine<br></div><div dir=3D"ltr">> (and, iirc, was also fine whe= n I checked at the beginning of working on<br></div><div dir=3D"ltr">> t= his in 2021) and it provides useful<br></div><div dir=3D"ltr">> <br></di= v><div dir=3D"ltr">> > What I would like to see working on Free= BSD is Safestack as a<br></div><div dir=3D"ltr">> > replacement= for the stack protector, which we were so very slow to adopt<br></div><div= dir=3D"ltr">> > even when it was originally developed in FreeB= SD. I think other projects<br></div><div dir=3D"ltr">> > based = on FreeBSD (Chimera and hardenedBSD) have been using it but I<br></div><div= dir=3D"ltr">> > don't know the details.<br></div><div dir=3D"l= tr">> ><br></div><div dir=3D"ltr">> <br></div><div dir=3D"lt= r">> No comment there, though I think Shawn Webb / HardenedBSD had been<= br></div><div dir=3D"ltr">> playing around with SafeStack (and might hav= e enabled it? I haven't<br></div><div dir=3D"ltr">> actually looked in a= while now).<br></div><div dir=3D"ltr">> <br></div><div dir=3D"ltr">>= > This is just all my $0.02<br></div><div dir=3D"ltr">> = ><br></div><div dir=3D"ltr">> > Pedro.<br></div><div dir=3D"= ltr">> <br></div><div dir=3D"ltr">> Thanks,<br></div><div dir=3D"ltr"= >> <br></div><div dir=3D"ltr">> Kyle Evans<br></div><div dir=3D"ltr">= > <br></div><div dir=3D"ltr">> ><br></div><div dir=3D"ltr">&= gt; > On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans<b= r></div><div dir=3D"ltr">> > <<a href=3D"mailto:kaevans@fast= mail.com" rel=3D"nofollow" target=3D"_blank">kaevans@fastmail.com</a> <m= ailto:kaevans@fastmail.com>> wrote:<br></div><div dir=3D"ltr">>&nb= sp; ><br></div><div dir=3D"ltr">> ><br></div><div dir=3D"ltr= ">> ><br></div><div dir=3D"ltr">> ><br></div><div d= ir=3D"ltr">> > On May 18, 2024 13:42, Pedro Giffuni <<a href= =3D"mailto:pfg@freebsd.org" rel=3D"nofollow" target=3D"_blank">pfg@freebsd.= org</a> <br></div><div dir=3D"ltr">> <mailto:pfg@freebsd.org>> = wrote:<br></div><div dir=3D"ltr">> ><br></div><div dir=3D"ltr">= > > Oh no .. please not...<br></div><div dir=3D"lt= r">> ><br></div><div dir=3D"ltr">> > W= e went into that in a GSoC:<br></div><div dir=3D"ltr">> ><br></= div><div dir=3D"ltr">> > <br></div><div dir=3D"ltr">> <a hre= f=3D"https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtension= s" rel=3D"nofollow" target=3D"_blank">https://wiki.freebsd.org/SummerOfCode= 2015/FreeBSDLibcSecurityExtensions</a> <br></div><div dir=3D"ltr">> <= <a href=3D"https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExt= ensions" rel=3D"nofollow" target=3D"_blank">https://wiki.freebsd.org/Summer= OfCode2015/FreeBSDLibcSecurityExtensions</a>> <<a href=3D"https://wik= i.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions" rel=3D"nofoll= ow" target=3D"_blank">https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibc= SecurityExtensions</a> <<a href=3D"https://wiki.freebsd.org/SummerOfCode= 2015/FreeBSDLibcSecurityExtensions" rel=3D"nofollow" target=3D"_blank">http= s://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions</a>>= ><br></div><div dir=3D"ltr">> ><br></div><div dir=3D"ltr">&g= t; ><br></div><div dir=3D"ltr">> > Ultima= tely it proved to be useless since stack-protector-strong.<br></div><div di= r=3D"ltr">> ><br></div><div dir=3D"ltr">> ><br></di= v><div dir=3D"ltr">> > Respectfully, I disagree with your concl= usion here:<br></div><div dir=3D"ltr">> ><br></div><div dir=3D"= ltr">> > 1.) _FORTIFY_SOURCE provides more granular detection o= f overflow; I<br></div><div dir=3D"ltr">> > don't have to overf= low all the way into the canary at the end of the<br></div><div dir=3D"ltr"= >> > frame to be detected, so my minor bug now can be caught be= fore something<br></div><div dir=3D"ltr">> > causes the stack f= rame to be rearranged and turn it into a security<br></div><div dir=3D"ltr"= >> > issue later<br></div><div dir=3D"ltr">> ><br><= /div><div dir=3D"ltr">> > 2.) __builtin_object_size doesn't wor= k on heap objects, but it actually<br></div><div dir=3D"ltr">> >= ; can work on subobjects from a heap allocation (e.g., &foo->name), = so the<br></div><div dir=3D"ltr">> > coverage extends beyond th= e stack into starting to detect other kinds of<br></div><div dir=3D"ltr">&g= t; > overflow<br></div><div dir=3D"ltr">> ><br></div><= div dir=3D"ltr">> > While the security value over stack-protect= or-strong may be marginal (I<br></div><div dir=3D"ltr">> > won'= t debate this specifically), the feature still has value in general.<br></d= iv><div dir=3D"ltr">> ><br></div><div dir=3D"ltr">> &g= t; Thanks,<br></div><div dir=3D"ltr">> ><br></div><div dir=3D"l= tr">> > Kyle Evans<br></div><div dir=3D"ltr">> <br></d= iv></div> </div> </div></body></html> ------=_Part_1722963_885190984.1716130042180--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?418178403.1722964.1716130042183>