Re: [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed

On Thu, 29 Aug 2024 16:16:03 +0200
Michal Privoznik <mprivozn(a)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/edfeb8763ac56112c59248c62c9cda13e5d…

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(a)redhat.com>
 ---
 
 It may be worth exploring whether level 3 would be beneficial:
 https://developers.redhat.com/articles/2022/09/17/gccs-new-fortification-le… 
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