Add fix for memset_s issue discovered with ATFE #776

Open
bjwtaylor wants to merge 1 commit from bjwtaylor/tf-m-build-issues into development
bjwtaylor commented 2026-04-23 08:13:54 -05:00 (Migrated from github.com)

Description

Add fix for memset_s issue discovered with ATFE resolves https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/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
## Description Add fix for memset_s issue discovered with ATFE resolves https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/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
gilles-peskine-arm (Migrated from github.com) requested changes 2026-04-27 08:18:27 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

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

We need to fix an interoperability problem, not remove a feature.
@ -16,2 +16,4 @@
#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__)
gilles-peskine-arm (Migrated from github.com) commented 2026-04-27 08:18:00 -05:00

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.

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.
bjwtaylor (Migrated from github.com) reviewed 2026-04-27 09:27:05 -05:00
@ -16,2 +16,4 @@
#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__)
bjwtaylor (Migrated from github.com) commented 2026-04-27 09:27:05 -05:00

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 github.com/TrustedFirmware-M/trusted-firmware-m@c630c687ae/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?

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?
gilles-peskine-arm (Migrated from github.com) reviewed 2026-04-27 09:54:44 -05:00
@ -16,2 +16,4 @@
#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__)
gilles-peskine-arm (Migrated from github.com) commented 2026-04-27 09:54:44 -05:00

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

Why is there a problem only with AtFE, not with other toolchains?
bjwtaylor (Migrated from github.com) reviewed 2026-04-28 02:28:46 -05:00
@ -16,2 +16,4 @@
#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__)
bjwtaylor (Migrated from github.com) commented 2026-04-28 02:28:46 -05:00

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 github.com/TrustedFirmware-M/trusted-firmware-m@c630c687ae/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 github.com/TrustedFirmware-M/trusted-firmware-m@c630c687ae/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.

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.
bjwtaylor (Migrated from github.com) reviewed 2026-06-30 08:05:22 -05:00
@ -16,2 +16,4 @@
#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__)
bjwtaylor (Migrated from github.com) commented 2026-06-30 08:05:22 -05:00

@gilles-peskine-arm, I've spoken to TF-M today and their preference is for use to disable the use of the ANNEX K functions in tf-psa-crypto. Though they would also reluctantly add further stubs to the existing github.com/TrustedFirmware-M/trusted-firmware-m@c630c687ae/bl2/CMakeLists.txt (L162) and here https://github.com/TrustedFirmware-M/trusted-firmware-m/blob/main/bl2/src/mbedcrypto_stubs.c. So I think it comes down to what we want to do, I guess we could have a conditional disable of the ANNEX K functions based on a config? Or we could just say the library requires ANNEX K and tf-m have to stub it, what do you think?

@gilles-peskine-arm, I've spoken to TF-M today and their preference is for use to disable the use of the ANNEX K functions in tf-psa-crypto. Though they would also reluctantly add further stubs to the existing 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. So I think it comes down to what we want to do, I guess we could have a conditional disable of the ANNEX K functions based on a config? Or we could just say the library requires ANNEX K and tf-m have to stub it, what do you think?
gilles-peskine-arm (Migrated from github.com) reviewed 2026-06-30 13:31:05 -05:00
@ -16,2 +16,4 @@
#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__)
gilles-peskine-arm (Migrated from github.com) commented 2026-06-30 13:31:05 -05:00

TF-PSA-Crypto does not require annex K functions. It uses annex K functions if the system headers declare that annex K is available.

It's perfectly reasonable for TF-M to prefer not to implement memset_s(). But TF-PSA-Crypto already supports that. The problem is that in one specific build configuration, the TF-M platform (consisting of the TF-M build scripts, the TF-M code and the toolchain code) declares that memset_() is available, but does not provide it.

Why does this specific combination of project and compiler cause a discrepancy between system headers and actual function availability?

TF-PSA-Crypto does not require annex K functions. It uses annex K functions if the system headers declare that annex K is available. It's perfectly reasonable for TF-M to prefer not to implement `memset_s()`. But TF-PSA-Crypto already supports that. The problem is that in one specific build configuration, the TF-M platform (consisting of the TF-M build scripts, the TF-M code and the toolchain code) declares that `memset_()` is available, but does not provide it. Why does this specific combination of project and compiler cause a discrepancy between system headers and actual function availability?
bjwtaylor (Migrated from github.com) reviewed 2026-07-01 07:28:38 -05:00
@ -16,2 +16,4 @@
#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__)
bjwtaylor (Migrated from github.com) commented 2026-07-01 07:28:38 -05:00

So, thanks of pointing this out, I hadn't realised we had this check. As far as I can tell the reason it fails because the check is a compile time check and the change that tf-m made keeps the compile time logic from atfe, which passes this check. They then change the linker to link in an implementation of libc which doesn't provide this function. So I guess they have further options of fixing the mismatch between compile time and link. Or we could just propose they use MBEDTLS_PLATFORM_ZEROIZE_ALT? Or we could maybe add a link based test, though this would probably not be an ideal for us. What do you think?

So, thanks of pointing this out, I hadn't realised we had this check. As far as I can tell the reason it fails because the check is a compile time check and the change that tf-m made keeps the compile time logic from atfe, which passes this check. They then change the linker to link in an implementation of libc which doesn't provide this function. So I guess they have further options of fixing the mismatch between compile time and link. Or we could just propose they use MBEDTLS_PLATFORM_ZEROIZE_ALT? Or we could maybe add a link based test, though this would probably not be an ideal for us. What do you think?
This pull request is broken due to missing fork information.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 'bjwtaylor/tf-m-build-issues:bjwtaylor/tf-m-build-issues'
git switch 'bjwtaylor/tf-m-build-issues'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'development'
git merge --no-ff 'bjwtaylor/tf-m-build-issues'
git switch 'bjwtaylor/tf-m-build-issues'
git rebase 'development'
git switch 'development'
git merge --ff-only 'bjwtaylor/tf-m-build-issues'
git switch 'bjwtaylor/tf-m-build-issues'
git rebase 'development'
git switch 'development'
git merge --no-ff 'bjwtaylor/tf-m-build-issues'
git switch 'development'
git merge --squash 'bjwtaylor/tf-m-build-issues'
git switch 'development'
git merge --ff-only 'bjwtaylor/tf-m-build-issues'
git switch 'development'
git merge 'bjwtaylor/tf-m-build-issues'
git push origin 'development'
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Mbed-TLS/TF-PSA-Crypto!776
No description provided.