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

check-config.h blocks implementation of timing on Mbed OS #4633

Closed
Patater opened this issue Jun 9, 2021 · 0 comments · Fixed by #4634
Closed

check-config.h blocks implementation of timing on Mbed OS #4633

Patater opened this issue Jun 9, 2021 · 0 comments · Fixed by #4634
Labels
bug component-platform Portability layer and build scripts

Comments

@Patater
Copy link
Contributor

Patater commented Jun 9, 2021

Summary

check-config.h is too restrictive about which platforms are permitted to implement the timing module. Specifically, it disallows Mbed OS to implement the timing module. It is very much possible to implement timing on Mbed OS, so we shouldn't explicitly disallow it.

System information

Mbed TLS version (number or commit id): Mbed TLS 2.25.0
Operating system and version: Mbed OS
Configuration (if not default, please attach config.h): https://github.com/ARMmbed/mbed-os/blob/master/connectivity/mbedtls/include/mbedtls/config.h

Expected behavior

check-config.h does not block someone from implementing and using MBEDTLS_TIMING_ALT on Mbed OS

Actual behavior

check-config.h blocks enabling MBEDTLS_TIMING_C, even if MBEDTLS_TIMING_ALT is used.

Steps to reproduce

Enable MBEDTLS_TIMING_C on Mbed OS

Additional information

Long long ago, Mbed didn't have threads and back then it maybe made more sense to have this check. However, Mbed OS has changed a lot and the check has become a barrier to progress rather than anything helpful.

Patater added a commit to Patater/mbedtls that referenced this issue Jun 9, 2021
Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.

Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7eba ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.

Fixes Mbed-TLS#4633

Signed-off-by: Jaeden Amero <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added bug component-platform Portability layer and build scripts fix available labels Jun 9, 2021
Patater added a commit to Patater/mbedtls that referenced this issue Jun 9, 2021
Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.

Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7eba ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.

Fixes Mbed-TLS#4633

Signed-off-by: Jaeden Amero <[email protected]>
Kazuyuki-Kimura pushed a commit to Kazuyuki-Kimura/mbedtls that referenced this issue Jun 20, 2021
Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.

Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7eba ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.

Fixes Mbed-TLS#4633

Signed-off-by: Jaeden Amero <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-platform Portability layer and build scripts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants