Re: [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 29 Aug 2024 16:16:03 +0200
Michal Privoznik <mprivozn@redhat.com> wrote:

> On some systems source fortification is enabled whenever code
> optimization is enabled (e.g. with -O2). Since code fortification
> is explicitly enabled too (with possibly different value than the
> system wants, there are three levels [1]), distros are required
> to patch our Makefile, e.g. [2].

Hah, thanks for the patch, I would have never guessed. I just tried
this on Alpine and, also there, gcc enables -D_FORTIFY_SOURCE=2 by
default, while it's not the case on Debian and Fedora.

> Detect whether fortification is not already enabled and enable it
> explicitly only if really needed.
> 
> 1: https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html
> 2: https://github.com/gentoo/gentoo/commit/edfeb8763ac56112c59248c62c9cda13e5d01c97

Rahil, I'm going to apply this in a bit, once it's released you can
drop Makefile-2024.03.20.patch (I didn't understand why you needed that
patch and I forgot to ask, but Michal just explained).

> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> 
> It may be worth exploring whether level 3 would be beneficial:
> https://developers.redhat.com/articles/2022/09/17/gccs-new-fortification-level#

I tried it a while ago, but at least in my quick test back then it
seemed to have a significant effect on TCP throughput.

On the other hand it might be worth understanding where that comes from
an if it's fixable somehow.

>  Makefile | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 01fada4..74a9513 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -33,9 +33,16 @@ AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/MIPS64EL/MIPSEL64/')
>  AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/HPPA/PARISC/')
>  AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/SH4/SH/')
>  
> +# On some systems enabling optimization also enables source fortification,
> +# automagically. Do not override it.
> +FORTIFY_FLAG :=
> +ifeq ($(shell $(CC) -O2 -dM -E - < /dev/null 2>&1 | grep ' _FORTIFY_SOURCE ' > /dev/null; echo $$?),1)
> +FORTIFY_FLAG := -D_FORTIFY_SOURCE=2
> +endif
> +
>  FLAGS := -Wall -Wextra -Wno-format-zero-length
>  FLAGS += -pedantic -std=c11 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE
> -FLAGS += -D_FORTIFY_SOURCE=2 -O2 -pie -fPIE
> +FLAGS +=  $(FORTIFY_FLAG) -O2 -pie -fPIE
>  FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
>  FLAGS += -DNETNS_RUN_DIR=\"/run/netns\"
>  FLAGS += -DPASST_AUDIT_ARCH=AUDIT_ARCH_$(AUDIT_ARCH)

-- 
Stefano