From nobody Sun May 19 05:12:58 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 4Vhpk45mpjz5Kllc; Sun, 19 May 2024 05:13:00 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Vhpk413Dfz4c1b; Sun, 19 May 2024 05:13:00 +0000 (UTC) (envelope-from kevans@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1716095580; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MfGtBBNaI5vw1dLcZlPqGYHdS6oG27xEPLASdUxFIOY=; b=IxItk7BZc8nDy1k//JPrCkNJfETGVq5u0OzCaxnZnE/+MS7uB3nrpgTltTr+WTQ3bR8eGX gVPsk51w8bJzInoV7TyFpCLf8D5g3rvr1bSh5qq/VRqjQe/x8aXiiC99rD8z9WT1cv4c4h TKU7xj80wlT90U0KrWQweqUf1wT1DSU0a90KImMRw1xqd3JBjPHFBCY7AOm/ZU3vr0HFvi 5EL4ohUXEbXJfptwFXLR83SwmxfLa+kSEfFwPLQIndA0TQ2BHNlGMAPrWRxZ7AJ6O2bHNn koKNBOrH47PcXRJ+XJs1ZwGMpu73c4PbyaQKaALu6EOnQaxUELDFzG5pGnelDQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1716095580; a=rsa-sha256; cv=none; b=YSb83fMiGd7EUknZLJ9TkYNqNhj6a3lWqCcbr3tTfuzOdhlz7U0H0AXNy9ZAa/Q6wgBoNj Vi8Rbd/LsfPxWxpYYefBIJnQDsBMrdK1CdHd4yfWLtIgZ9jDlsuK2guKqweNAXlkYxVezg 3Te1lKtmavcy2LacI5zpzkbIm+Oh7YP+pwHQTjx0AaQAdIsE6T7/MQ8sLRazi3VsRbP+Vn 5LxRmzZ4pnxPI+YuTdJKCAprgvMF1vUVuCUvbGt+LKNl5inmRKm1zXVzqTKERiWLemFQZ9 leYnVAjikzbCGHn/5qjYGpZL2u9nYaxF4yh/fMUJZkjBsDwxkKZZzInWUmL9nQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1716095580; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MfGtBBNaI5vw1dLcZlPqGYHdS6oG27xEPLASdUxFIOY=; b=pxouevAlPbbxXfkhQoscB5G9DLJSoMFgnK5YYn4Vo6t3BIfF70VBhVqB2tWRHb07PoqYPZ DdhPvXz+prImnF4EYcq/kQag/IUqExeVCZiDfq8AaZtSgB50ErEEC6H5KeC6LYR+hPbffI sH8PhNC1FVk2lJ7eE09gsiTqOWTUPnQtq/fgp3brh0xjOVz8SPWpZxGf74wyr1VNMxnw6B 1+nWSF5M1kqvsDfnRzBKrLG8QE809XfOs/VCUvsxOOIMmoeHpdYUeLBeLV77I+GHDobSph pHp9ubGCyjb47XN0lBHrzGXXyY8jgBU99IKc/s7s1CWhGhB6R/CIXzxyEXKTkQ== Received: from [10.9.4.95] (unknown [209.182.120.176]) (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) (Authenticated sender: kevans/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Vhpk32Bsmz1Hs3; Sun, 19 May 2024 05:12:59 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Message-ID: <216ca4a4-c0b8-4d72-bc6f-95e82a6b77da@FreeBSD.org> Date: Sun, 19 May 2024 00:12:58 -0500 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 User-Agent: Mozilla Thunderbird Subject: Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD Content-Language: en-US To: Pedro Giffuni Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@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> From: Kyle Evans In-Reply-To: <1413980952.1357400.1716093599901@mail.yahoo.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 > 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 > > > wrote: > > > > > > > > > > On May 18, 2024 13:42, Pedro Giffuni > wrote: > > > >    Oh no .. please not... > > > >    We went into that in a GSoC: > > > > > 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 >