Preset _FORTIFY_SOURCE=3 should not be downgraded to 2 by Makefile

[chrfranke]

Thanks for providing this great tool. I will possibly provide this as a package for the Cygwin distro.

Recent versions of packaging support tools like Cygwin's cygport predefine -D_FORTIFY_SOURCE=3. This enables the usage of __builtin_dynamic_object_size() instead of __builtin_object_size() if supported by include files and compiler (GCC >= 12.0).

If PRESERVE_CFLAGS=1 is not set, the Makefile redefines this to _FORTIFY_SOURCE=2 and the compiler prints a related warning. A hardening feature should IMO only be downgraded if really needed.

[ColinIanKing]

Sure, I can fix that. Just to be 100% sure of what I need to do, which variables are defined with -D_FORTIFY_SOURCE=3 so that I can detect this and not modify this setting at build time?

[chrfranke]

Unfortunately a comprehensive check for _FORTIFY_SOURCE=3 requires a compile test because the value may be passed as part of CFLAGS= to make by the package build recipes or may be predefined in the compiler presets. IIRC the latter is the case with Ubuntu GCC.

Quick check for a compiler preset: defined to 3:

$ echo _FORTIFY_SOURCE | cc -E -xc - | tail -1
3

undefined:

$ echo _FORTIFY_SOURCE | cc -E -xc - | tail -1
_FORTIFY_SOURCE

Fixing this issue is not important as in this case the Makefile section could be disabled via PRESERVE_CFLAGS in this case.

In general, override should IMO only be used if the change is mandatory in all cases. Its usage prevents that maintainers could modify variables via make command line in the package build recipes. As a consequence, the file needs to be patched instead.

[ColinIanKing]

I've pushed a commit that I think fixes this. Commit:

commit 9faa5b39e1103b08ec2399945ed65655bd807e5b (HEAD -> master, origin/master, origin/HEAD)
Author: Colin Ian King <colin.i.king@gmail.com>
Date:   Fri Sep 13 17:54:51 2024 +0100

    Makefile: only define _FORTIFY_SOURCE if it's not defined

Please let me know if this addresses the issue.

[chrfranke]

Works for me:

$ uname -o
Cygwin

$ git show-ref -s HEAD
a633d359253872b7bffb25eff49d35bd00772405

$ rm -f stress-ng.o && make VERBOSE=1 stress-ng.o
readelf: Error: Not an ELF file - it has the wrong magic bytes at the start
readelf: Error: Not an ELF file - it has the wrong magic bytes at the start
readelf: Error: Not an ELF file - it has the wrong magic bytes at the start
readelf: Error: Not an ELF file - it has the wrong magic bytes at the start
CC stress-ng.c
cc -Wall -Wextra -DVERSION='"0.18.04"' -std=gnu99 -O2 -Wformat -fstack-protector-strong -Werror=format-security -D_FORTIFY_SOURCE=2 -fipa-pta -fivopts -fmodulo-sched -c -o stress-ng.o stress-ng.c

$ rm -f stress-ng.o && make VERBOSE=1 CFLAGS=-D_FORTIFY_SOURCE=3 stress-ng.o
readelf: Error: Not an ELF file - it has the wrong magic bytes at the start
readelf: Error: Not an ELF file - it has the wrong magic bytes at the start
readelf: Error: Not an ELF file - it has the wrong magic bytes at the start
readelf: Error: Not an ELF file - it has the wrong magic bytes at the start
CC stress-ng.c
cc -D_FORTIFY_SOURCE=3 -Wall -Wextra -DVERSION='"0.18.04"' -std=gnu99 -O2 -Wformat -fstack-protector-strong -Werror=format-security -fipa-pta -fivopts -fmodulo-sched -c -o stress-ng.o stress-ng.c

Thanks!

PS: Portability note: tail -1 is the syntax from ancient Unix which is interestingly no longer officially supported. At least GNU coreutils and the current FreeBSD version still support this silently. According to their man pages, only tail -n 1 would be valid. IIRC some older (Free|Net|Open)BSD required -n but I don't remember which one. 1979: https://man.cat-v.org/unix_7th/1/tail 1997: https://pubs.opengroup.org/onlinepubs/007908799/xcu/tail.html 2024: https://pubs.opengroup.org/onlinepubs/9799919799/utilities/tail.html