* [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed
@ 2024-08-29 14:16 Michal Privoznik
2024-08-29 17:03 ` Stefano Brivio
2024-08-30 7:07 ` Stefano Brivio
0 siblings, 2 replies; 4+ messages in thread
From: Michal Privoznik @ 2024-08-29 14:16 UTC (permalink / raw)
To: passt-dev
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed
2024-08-29 14:16 [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed Michal Privoznik
@ 2024-08-29 17:03 ` Stefano Brivio
2024-08-30 7:54 ` Michal Prívozník
2024-08-30 7:07 ` Stefano Brivio
1 sibling, 1 reply; 4+ messages in thread
From: Stefano Brivio @ 2024-08-29 17:03 UTC (permalink / raw)
To: Michal Privoznik; +Cc: passt-dev, Rahil Bhimjiani
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed
2024-08-29 14:16 [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed Michal Privoznik
2024-08-29 17:03 ` Stefano Brivio
@ 2024-08-30 7:07 ` Stefano Brivio
1 sibling, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2024-08-30 7:07 UTC (permalink / raw)
To: Michal Privoznik; +Cc: passt-dev
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed
2024-08-29 17:03 ` Stefano Brivio
@ 2024-08-30 7:54 ` Michal Prívozník
0 siblings, 0 replies; 4+ messages in thread
From: Michal Prívozník @ 2024-08-30 7:54 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, Rahil Bhimjiani
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-08-30 7:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-29 14:16 [PATCH] Makefile: Enable _FORTIFY_SOURCE iff needed Michal Privoznik
2024-08-29 17:03 ` Stefano Brivio
2024-08-30 7:54 ` Michal Prívozník
2024-08-30 7:07 ` Stefano Brivio
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).