Skip to content

Add fix for memset_s issue discovered with ATFE#776

Open
bjwtaylor wants to merge 1 commit into
Mbed-TLS:developmentfrom
bjwtaylor:tf-m-build-issues
Open

Add fix for memset_s issue discovered with ATFE#776
bjwtaylor wants to merge 1 commit into
Mbed-TLS:developmentfrom
bjwtaylor:tf-m-build-issues

Conversation

@bjwtaylor
Copy link
Copy Markdown
Contributor

@bjwtaylor bjwtaylor commented Apr 23, 2026

Description

Add fix for memset_s issue discovered with ATFE resolves #754. Adds a patch currently being used by TF-M. Open issues are do we want to add additional testing to cover their use cases and do we want to backport it? Let me know what people think?

PR checklist

  • changelog not required because: No public changes
  • framework PR provided not required
  • TF-PSA-Crypto development PR provided #HERE
  • TF-PSA-Crypto 1.1 PR provided # | not required because: TBD
  • mbedtls development PR not required because: No changes
  • mbedtls 4.1 PR not required because: No changes
  • mbedtls 3.6 PR not required because: No changes
  • tests not required because: TBD

Signed-off-by: Ben Taylor <ben.taylor@linaro.org>
@gilles-peskine-arm gilles-peskine-arm self-requested a review April 23, 2026 13:29
@bjwtaylor bjwtaylor marked this pull request as ready for review April 24, 2026 07:10
@bjwtaylor bjwtaylor added needs-review Every commit must be reviewed by at least two team members needs-reviewer This PR needs someone to pick it up for review priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels Apr 24, 2026
Copy link
Copy Markdown
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

We need to fix an interoperability problem, not remove a feature.

#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

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

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

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

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.

@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members needs-reviewer This PR needs someone to pick it up for review labels Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-work priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)

Projects

Development

Successfully merging this pull request may close these issues.

Build broken with TF-M using ATfE in 1.1.0

2 participants