From nobody Sun May 19 14:47:22 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Vj3Sw4fnYz5K9Ry for ; Sun, 19 May 2024 14:47:28 +0000 (UTC) (envelope-from pfg@freebsd.org) Received: from sonic314-20.consmr.mail.ne1.yahoo.com (sonic314-20.consmr.mail.ne1.yahoo.com [66.163.189.146]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4Vj3Sw0jcgz4RPK for ; Sun, 19 May 2024 14:47:28 +0000 (UTC) (envelope-from pfg@freebsd.org) Authentication-Results: mx1.freebsd.org; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1716130046; bh=MdP7lKzjEjjXLyMi5pW4qrgyir4AUPwN22e20aRAgIU=; h=Date:From:To:Cc:In-Reply-To:References:Subject:From:Subject:Reply-To; b=eSiJuUccv33Qa1OuxFJY2+gfoRZz2kD4mEghpquiF25ANP0iiCR0sFgHXdKMuNfFb6bU62nTPXEdgf49gLeolP8rbQ+ZP45xArWxw6N1lvU1o4D0iNt/UFGCEqMGhbINtmg2JDY7OLMUoyZqxSH1E+gFQsQ3686js6jRyJklUJKkUTpIuMnTo2CAKqQgXih3hO9VnyXHp485y63tDZ6ZVlLpLQKJBazNlQcotbeesVWwJtXBxpTKC1Vr6Yf2mesY7m0v3SdGSFPwY50D78pVxjNNn47v/WzDE6FRtk++4BAI6gs83BNCIQF6cank+zLOZikYWrJ88S9+UhNezHgnrQ== X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1716130046; bh=W3XqQYYe0xuG1vJ//Llc5YvKhlOPanyvD4nanxiMPPz=; h=X-Sonic-MF:Date:From:To:Subject:From:Subject; b=latoFbXdChx+vtmYXkSfHf90u3J4mYiYLPdCeE1R2/3/b/r37/4yry2OhAXBuDaJd0I90chcrLnalB9z9KEbg5jWssUXaEgJIePy1mBDls1UPoxYT3SJ84ycdHx+LJ6ZMAyJzPVwROamKkdwgztor3haDuLOhyiht4bG/Odo8FfHVmesNtnrEgIDQR3F0Jxq7zJNbJSbfyd660vIYfh3SIYh/oF4mxSl7YEjcVO9jJfqEV+/cNCQTHO0Qz6gMkBBY3NQOhq1BxuDSuBpZDrGuWXZo+5H4QwXN8K/WNpsNwr4L0Sn1sR5FGDVxvpgc9Q7GQUyW9uyWHUeq4n8cli0kg== X-YMail-OSG: WjkridoVM1n0B.yo9bN6nAgioDdYxG0xe.jwKS6sRFt4J9YzMO_eabFhwQVgfYa WoBJspfkC5ARNtKGBS8qDqJg4klGfCKnKNGbG_l0I7e3FJc_j364d3XA2gYQgpzMU7O8yJf6feuv jR6zla5Aq3of8hXntyH2zK23xuZ6Un4JjpvBnKKpBCb2Zqsk6NBWkJPiKSXorD0v.1vfHqjPgiDq s_anLGaPw.ndx_EWws5JK6ZdAE.my7MNPAPFK9Omjci3GRMhW9I4Nhe9S1Qtk94kVjWx2YbB4ADH JDl0cPvHVxh7YMxj1cIac1qBlAwM9aKPMMb3hnj1htKFXJJfYKfU2pQKLnyQ6e6.mquoGVvQrLgA QjSL4eIbKzjKKlfyaVbdV.CdWrFqkVx1CIkq4DxWaTv9o2uXnFgiU.nshAn_C4.IGNo_YqCypC11 Pzqhiy_nJA7HU5lRps2SKcx1ZBwT3yv7TVgOLfZtWd1g1iweq8W2Sb1VII1hRKbZej7GhnuwLQYU JGEz8UTkeqA8fgz7B10jcyBZ0bXujt2EWt00Z6wnqkPJ1GjD6Wjol5gEHt5mz5TEiiCZXKXe1Pq0 T.DoeVwTgKsQIO9tEkhpwtKX1aBKX.ZR8opY9Y4y9lnMYs33NkXSmVWl7ojeh1ddzh149RdqmuQr 8f3Vf08EMZtbj9pqfY9HArQGgJbXhQtYOAULOjW8WqeTPyfkrr7p1Q9zhOuIKD.f6HlZQTlvOoan _YbrjCQSpHe3.6WgxHKpaQ_.dcJVwpxNkFfPodb3pmHZ7GHrx5iRz924DSreMNvx30INQrGYO3Rn qU6ESobBbU3C0D0kt7EiiblssaoabCQU7J.vVjIBa1ToTxN4wfGLhA3gOHAiqs1oFV6HPbOo65Px 1tM876Hf4H_nNA9M8yRSmUeNfpUxoa9K1xyD1_wPdkABG64BQE2qMaRXWDI2txDEXoynX9TlRL0f dxtl7WqVgN7RKcEJ6.6ZBCA3Ij8Q7Xgt2o6pVaKxHBjOJt5Fc91nt0S0PLNFHdLMNLUinne3U5ho QuvoH.soaccLawf1EVL.sjDDcLYNutwDhRUZ.WQK.XLm7XqML1mVU3fahXJIb5Z5Sk0mzzKstUpk jDUVmuJqrpUP5HN8exFFD79IP2XtBeUK9TPQpgcGP.tD3cZkWyfGHkG5r4ILGf0Y7UI0woc7C3NT MM9fuNbVgZul7rDPICZdMXj35PcxRE8uuQKCuWfOh8wFL7S0Js8iXkCKnvmP9FRW5YXXUuziuWW6 4WR2Yv9AFg6hmp98ej2uQHcYtzNuLPYwWDGj.xT4PF.YTnqRdjqAAwuUWUZ2OV08.HlkC6Ug9_yJ xpyczx9cRH7grAIHA2dq0b688ZL5t9mAPBQdQgucfZgKrInhz8MoUQKp18OVM1tDhj08GzyyQ0vW blRQvTfW3piGWSMoTmAPmwd46y43hdi7Rk7J5TzFSfwyZodcJLqP7Y9yE3m5i4hlmvv3epVU62C0 qqSGc66n.LBqSekOGna2.3c2Whn6JOsbg_cbEMwYVcC_d0P7ei5gH7YD37gB7HXUt9O45FCDEL6n 9PTU.15ruFnTmTZp_evZh7avSPAwN.TwMNBqaHI5ci7fWuTXqk1wX_o3OjcRMMjO8Hnps75y0Twn 0DF8hkwE1QsKbNzVCArkdZhyKAj1YyXa59VBM2XEC_4eRekSrC.G7m6EP8_fltNGVSmoCCl0Zt.G I28Xb6OuS8K1TsakVHXVGoS9X5pwa8tcrKpArliANoLDua_I0hjzhnUOM6tUJSq1rno5rXgM3JTu L84wI0_SZ1fNyASs7nCEc.DvRQSG3zYPJJACu9ORvjakDs2cGRj.tGaD2GAWMSsSyJVT6.z6gEr3 FHyTIeiBLmRxgMcA4v3xxCdZRY4j0fy3m2kvw3ap2BqzNaLInCZA_.S80l_UwqSWsJHYGTjMduSb 5PizSKZBfzVJiMx_yreaQpY_KY6YiSdgu6IgxeG6DJmkZaDtLjj8Ofx5LX0ECGbs5LA5lvVDWXJ4 ek1djInrRuNcOFOTMw0ddpOJUwu0W.t6IghdjKewB5OwlL_7TrjwSpZXfa9IZj1e2bNH0rOGP4My g6m2FNwYtEodRc8eG1fWv.HmW_p5jpdHemXXwVH1ES_xOJx062dhWwoVqrTySSXgrDwz28ArApu3 xGu_sfDqc0xcjaipHoNzdJnnnVHjBQ75DQ8TDpzR.6pBL_q8- X-Sonic-MF: X-Sonic-ID: e988718f-097e-4045-b607-d1995a7638d5 Received: from sonic.gate.mail.ne1.yahoo.com by sonic314.consmr.mail.ne1.yahoo.com with HTTP; Sun, 19 May 2024 14:47:26 +0000 Date: Sun, 19 May 2024 14:47:22 +0000 (UTC) From: Pedro Giffuni To: Kyle Evans Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" 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> Subject: Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_1722963_885190984.1716130042180" X-Mailer: WebService/1.1.22356 YMailNorrin X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:36646, ipnet:66.163.184.0/21, country:US] X-Rspamd-Queue-Id: 4Vj3Sw0jcgz4RPK ------=_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 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 > 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 > > wrote: >=C2=A0 > >=C2=A0 > >=C2=A0 > >=C2=A0 > >=C2=A0 > On May 18, 2024 13:42, Pedro Giffuni > 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 > = > >=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
<= div>
Hmm... well.

In all honesty I understand I am doomed to lose this battle :).
<= 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.

best of lucks!

<= div dir=3D"ltr" data-setdir=3D"false">Pedro.

ps. Just for the reference, the Google guys did develop a do= cument on how they implemented this for clang on bionic:

=
=20
=20
On Sunday, May 19, 2024 at 12:13:00 AM GMT-5, Kyle = Evans <kevans@freebsd.org> wrote:


=20 =20
On 5/18/24 23:39, Pedro Giffuni wrote= :
> FWIW .. and let me be clear I haven't work= ed on this in ages and I am
> not planning to= retake this either...
>
> clang just couldn't do the static  fortify_source checks&nb= sp; due to the way
> llvm uses an intermediat= e representation; the size just couldn't be
>= handled in the preprocessor. Google did spend some time adding extra
<= /div>
> attributes to clang to improve the debugging and= you can see that
> implemented in bionic lib= c but that was it. musl didn't even try.
>

Admittedly, I have no id= ea 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 ty= pically
performance critical string/memory ops u= se __builtin___foo_chk() that do
successfully ge= t optimized away in the common case to the underlying
foo() call.  This all works very well with clang, I haven't test= ed it
under GCC but, as you've noted, would assu= me 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 t= he 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 distributi= ons
> out there indeed have built their softwa= re packages with GCC and
> fortify_source >= ;=3D2. As a consequence, when we ran an exp-run on the
> ports tree (with GCC), fortify_source didn't find anything: i= t was
> basically a waste of time.
<= div dir=3D"ltr">>
> 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 a= gain wastes energy and can have some performance
> hit. The later may seem negligible in modern processors, but why do t= hem
> if they bring no benefit? (No need to a= nswer ... just left as food for
> thought)
>
> Pedro.
>
> On Saturday, May 18= , 2024 at 09:08:52 PM GMT-5, Kyle Evans
> <= ;k= evans@freebsd.org> wrote:
>
<= div dir=3D"ltr">>
>
>
> On 5/18/24 20:09, Pedro Giffuni w= rote:
>  > (sorry for top posting .. m= y mailer just sucks)
>  > Hi;
>  >
>  >= I used to like the limited static checking FORTIFY_SOURCE provides and
=
>  > when I ran it over FreeBSD it did fi= nd a couple of minor issues. It only
>  &= gt; 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 siz= e
> while GCC did.  I'd welcome counter-e= xamples here, though -- we have
> funding to b= oth finish the project (widen the _FORTIFY_SOURCE net to
> more of libc/libsys) and add tests to demonstrate that it's b= oth
> functional and correct.  It would b= e useful to also document
> deficiencies in th= e tests.
>
>&nbs= p; > I guess it doesn't really hurt to have FORTIFY_SOURCE around and Ne= tBSD
>  > had the least intrusive impl= ementation the last time I checked but I
>&nbs= p; > would certainly request it should never be activated by default,
>  > specially with clang. The GCC versi= on 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 t= o be just fine
> (and, iirc, was also fine whe= n I checked at the beginning of working on
> t= his in 2021) and it provides useful
>
>  > What I would like to see working on Free= BSD is Safestack as a
>  > replacement= for the stack protector, which we were so very slow to adopt
>  > even when it was originally developed in FreeB= SD. 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<= br>
> playing around with SafeStack (and might hav= e enabled it? I haven't
> actually looked in a= while now).
>
>=   > This is just all my $0.02
>  = >
>  > Pedro.
>
> Thanks,
>
> Kyle Evans
= >
>  >
&= gt;  > On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans
>  > <kaevans@fastmail.com <m= ailto:kaevans@fastmail.com>> wrote:
>&nb= sp; >
>  >
>  >
>  >
>  > On May 18, 2024 13:42, Pedro Giffuni <pfg@freebsd.= org
> <mailto:pfg@freebsd.org>> = wrote:
>  >
= >  >    Oh no .. please not...
>  >
>  >    W= e went into that in a GSoC:
>  >
>  >
>  >
&g= t;  >
>  >    Ultima= tely it proved to be useless since stack-protector-strong.
>  >
>  >
>  > Respectfully, I disagree with your concl= usion here:
>  >
>  > 1.) _FORTIFY_SOURCE provides more granular detection o= f overflow; I
>  > don't have to overf= low all the way into the canary at the end of the
>  > frame to be detected, so my minor bug now can be caught be= fore something
>  > causes the stack f= rame to be rearranged and turn it into a security
>  > issue later
>  >
<= /div>
>  > 2.) __builtin_object_size doesn't wor= k on heap objects, but it actually
>  >= ; can work on subobjects from a heap allocation (e.g., &foo->name), = so the
>  > coverage extends beyond th= e stack into starting to detect other kinds of
&g= t;  > overflow
>  >
<= div dir=3D"ltr">>  > While the security value over stack-protect= or-strong may be marginal (I
>  > won'= t debate this specifically), the feature still has value in general.
>  >
>  &g= t; Thanks,
>  >
>  > Kyle Evans
> 
------=_Part_1722963_885190984.1716130042180--