Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: unbreak builds with _FORTIFY_SOURCE=1 #708

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ynezz
Copy link

@ynezz ynezz commented Jun 29, 2023

Some downstream build systems allow passing custom _FORTIFY_SOURCE values and this results in conflicting compiler flags and build failures:

 <command-line>: error: "_FORTIFY_SOURCE" redefined

So lets fix it by undefining any previous _FORTIFY_SOURCE values and then forcing _FORTIFY_SOURCE=2.

Some downstream build systems allow passing custom _FORTIFY_SOURCE
values and this results in conflicting compiler flags and build
failures:

 <command-line>: error: "_FORTIFY_SOURCE" redefined

So lets fix it by undefining any previous _FORTIFY_SOURCE values and
then forcing _FORTIFY_SOURCE=2.

Signed-off-by: Petr Štetiar <ynezz@true.cz>
@ynezz
Copy link
Author

ynezz commented Jun 29, 2023

Example of such failure https://github.com/openwrt/packages/actions/runs/5410066043/jobs/9831013719

[1/65] Building C object src/CMakeFiles/fido2_shared.dir/aes256.c.o
FAILED: src/CMakeFiles/fido2_shared.dir/aes256.c.o 
/builder/staging_dir/toolchain-arm_cortex-a15+neon-vfpv4_gcc-12.3.0_musl_eabi/bin/arm-openwrt-linux-muslgnueabi-gcc -DHAVE_ASPRINTF -DHAVE_CBOR_H -DHAVE_CLOCK_GETTIME -DHAVE_DEV_URANDOM -DHAVE_ENDIAN_H -DHAVE_ERR_H -DHAVE_EXPLICIT_BZERO -DHAVE_GETLINE -DHAVE_GETOPT -DHAVE_GETPAGESIZE -DHAVE_GETRANDOM -DHAVE_OPENSSLV_H -DHAVE_POSIX_IOCTL -DHAVE_SIGNAL_H -DHAVE_STRLCAT -DHAVE_STRLCPY -DHAVE_STRSEP -DHAVE_SYSCONF -DHAVE_SYS_RANDOM_H -DHAVE_UNISTD_H -DOPENSSL_API_COMPAT=0x10100000L -DTLS=__thread -DUSE_NFC -D_FIDO_INTERNAL -D_FIDO_MAJOR=1 -D_FIDO_MINOR=12 -D_FIDO_PATCH=0 -Dfido2_shared_EXPORTS -I/builder/build_dir/target-arm_cortex-a15+neon-vfpv4_musl_eabi/libfido2-1.12.0/src -D_POSIX_C_SOURCE=200809L -D_BSD_SOURCE -D_GNU_SOURCE -D_DEFAULT_SOURCE -std=c99 -Os -pipe -fno-caller-saves -fno-plt -fhonour-copts -mfloat-abi=hard -ffile-prefix-map=/builder/build_dir/target-arm_cortex-a15+neon-vfpv4_musl_eabi/libfido2-1.12.0=libfido2-1.12.0 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -DNDEBUG -D_FORTIFY_SOURCE=2 -fPIC -Wall -Wextra -Werror -Wshadow -Wcast-qual -Wwrite-strings -Wmissing-prototypes -Wbad-function-cast -Wimplicit-fallthrough -pedantic -pedantic-errors -fstack-protector-all -Wno-unused-result -Wconversion -Wsign-conversion -Wframe-larger-than=2047 -MD -MT src/CMakeFiles/fido2_shared.dir/aes256.c.o -MF src/CMakeFiles/fido2_shared.dir/aes256.c.o.d -o src/CMakeFiles/fido2_shared.dir/aes256.c.o -c /builder/build_dir/target-arm_cortex-a15+neon-vfpv4_musl_eabi/libfido2-1.12.0/src/aes256.c
<command-line>: error: "_FORTIFY_SOURCE" redefined
<command-line>: note: this is the location of the previous definition
ninja: build stopped: subcommand failed.

@LDVG
Copy link
Contributor

LDVG commented Jun 29, 2023

Hm, wouldn't it be preferred to honor the environment's wishes for a specific _FORTIFY_SOURCE level?

@ynezz
Copy link
Author

ynezz commented Jun 29, 2023

Hm, wouldn't it be preferred to honor the environment's wishes for a specific _FORTIFY_SOURCE level?

I've looked at fadcc98 introducing this option and have assumed, that its desired. Would for example _FORTIFY_SOURCE=3 work out of the box? Either way, I'm fine adjusting it as desired.

@LDVG
Copy link
Contributor

LDVG commented Jun 30, 2023

I've looked at fadcc98 introducing this option and have assumed, that its desired.

It should be seen as our desired default for release builds at this time. But I'd argue that if -D_FORTIFY_SOURCE is set by the caller, we should assume that they know what they're doing respect its value.

Would for example _FORTIFY_SOURCE=3 work out of the box?

I've only tested it briefly; it certainly should work. If it doesn't, then that's probably a bug.

@ynezz
Copy link
Author

ynezz commented Jun 30, 2023

we should assume that they know what they're doing respect its value.

Ok, I agree and I'll rework it.

resiliencetheatre added a commit to resiliencetheatre/libfido2 that referenced this pull request Oct 29, 2023
build: unbreak builds with _FORTIFY_SOURCE=1 Yubico#708

Yubico@0bd31f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants