[PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed

[PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed

[Michal Privoznik]

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].

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

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#

 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)
-- 
@@ -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)
-- 
2.44.2

Re: [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed

[Stefano Brivio]

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

Re: [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed

[Michal Prívozník]

On 8/29/24 19:03, Stefano Brivio wrote:
> 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).

Gentoo actually have so called live ebuilds - recipes to install an app
from its git. And seeing a patch applied on top of git made me write
this patch.

Anyway, PR posted here:

https://github.com/gentoo/gentoo/pull/38342

Michal

Re: [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed

[Stefano Brivio]

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].
> 
> 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
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Applied, thanks!

-- 
Stefano