Skip to content

Add fix for memset_s issue discovered with ATFE #776

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.

Sign up for GitHub

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

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

Add fix for memset_s issue discovered with ATFE #776

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/tf_psa_crypto_platform_requirements.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
#ifndef TF_PSA_CRYPTO_TF_PSA_CRYPTO_PLATFORM_REQUIREMENTS_H
#define TF_PSA_CRYPTO_TF_PSA_CRYPTO_PLATFORM_REQUIREMENTS_H

/* Do not try to use the library extension with ATfE toolchain */
#if !defined(__GNUC__) || !defined(__clang_major__)
Copy link
Copy Markdown
Contributor

@gilles-peskine-arm gilles-peskine-arm Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is throwing the baby with the bathwater. We do want to enable __STDC_WANT_LIB_EXT1__ so that we can use Annex K functions (memset_s, gmtime_s, …) where available. That inclues Clang with Glibc.

A conforming C implementation will not define __STDC_LIB_EXT1__ in its headers if it doesn't provide the Annex K functions. This should be the case even for embedded platforms that don't have a full libc. Technically the fact that compilation fails is a bug in TF-M or in AtFE, but given that TF-M is not using an external libc, we may need to add a special case.

We need to identify why AtFE in TF-M has a problem, and either add a workaround for this specific case or tell TF-M how to fix their code or their compiler invocation.

Copy link
Copy Markdown
Contributor Author

@bjwtaylor bjwtaylor Apr 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, I thought the plan was to verify the error and apply their fix.

So the issue is caused because TF-M adds the option to the compiler -nostdlib here https://github.com/TrustedFirmware-M/trusted-firmware-m/blob/c630c687aecaba792788b3d0cec17035981e0eb9/toolchain_ATFE.cmake#L134. If we do not want to accept their patch, they will have to change their build to something like this pattern:

clang --target=arm-none-eabi -mcpu=cortex-m33 -mthumb -nostartfiles \ tf-psa-crypto_build/repro_zeroize.c \ tf-psa-crypto_build/libc_stubs.c \ f-psa-crypto_build/core/libtfpsacrypto.a \ -Wl,-e,main \ -Wl,--defsym=__heap_start=0x20000000 \ -Wl,--defsym=__heap_end=0x20000000 \ -o tf-psa-crypto_build/repro_zeroize_option1.axf
Though they may not be keen to do that, as I suspect this will increase the size of their binaries and they won't like that. Which is probably why they're using the option in the first place. We will also probably have to add a stub something like this:

void _exit(int status) { (void) status; for (;;) { } }
Let me know what you think?

Copy link
Copy Markdown
Contributor

@gilles-peskine-arm gilles-peskine-arm Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a problem only with AtFE, not with other toolchains?

Copy link
Copy Markdown
Contributor Author

@bjwtaylor bjwtaylor Apr 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we may have to work with tf-m to figure this out for sure as it appears to be a larger architectural problem with tf-m and only they will potentially know the exact reasoning behind their build config. However -nostdlib is gated on CONFIG_TFM_INCLUDE_STDLIBC , this is forced to be off when the compiler is clang here https://github.com/TrustedFirmware-M/trusted-firmware-m/blob/c630c687aecaba792788b3d0cec17035981e0eb9/config/check_config.cmake#L22 It appears based on this commit https://git.trustedfirmware.org/plugins/gitiles/TF-M/trusted-firmware-m/+/4537a2c584e1a496a215f37eecc22d37b1d9c22f that they are attempting to replace the stdlib with their own smaller more controlled implementation. This is being sourced from different places for different toolchains, which adds another level of variables and further explains the different behavior. However it looks like they are trying to add their own glibc stubs here https://github.com/TrustedFirmware-M/trusted-firmware-m/blob/c630c687aecaba792788b3d0cec17035981e0eb9/bl2/CMakeLists.txt#L162 and here https://github.com/TrustedFirmware-M/trusted-firmware-m/blob/main/bl2/src/mbedcrypto_stubs.c. However they have not added implementations for the functions that are currently causing the issue. So there's a few different ways they could fix it, though the exact solution will depend on their architectural objectives.

#ifndef __STDC_WANT_LIB_EXT1__
/* Ask for the C11 gmtime_s() and memset_s() if available */
#define __STDC_WANT_LIB_EXT1__ 1
#endif
#endif /* !__GNUC__ || !__clang_major__ */

#if !defined(_POSIX_C_SOURCE)
/* For standards-compliant access to
Expand Down