Fix getrandom detection with musl libc - #843
Conversation
Detect sys/random.h before selecting the Linux entropy path. This lets musl builds use their libc getrandom wrapper instead of falling back to blocking /dev/random. Signed-off-by: Ning Xia <hi@nosec.me>
gilles-peskine-arm
left a comment
There was a problem hiding this comment.
I'm all for improving the library on musl, but not if it breaks the build elsewhere.
The right thing to do would be to guard the non-portable code with #if __MUSL__ or some such. But since this does not and will not exist, I'm afraid we're stuck. We could add a way to declare “I swear I have getrandom()” at compile time. But that's even more complexity in code that's already more complex than I'd like. So I'd want to set up some testing if we do that, and that's complicated.
| * libcs such as musl which provide getrandom() but not __GLIBC__. | ||
| */ | ||
| #if ((defined(__linux__) && defined(__GLIBC__)) || defined(__midipix__)) | ||
| #if defined(__linux__) && defined(__has_include) |
There was a problem hiding this comment.
This breaks the build with dietlibc.
CC source/platform/platform_util.c
In file included from source/platform/platform_util.c:305:
/usr/include/x86_64-linux-gnu/sys/random.h: In function ‘getrandom’:
/usr/include/x86_64-linux-gnu/sys/random.h:35:42: error: unknown type name ‘__wur’; did you mean ‘__wsum’?
35 | unsigned int __flags) __wur
| ^~~~~
| __wsum
/usr/include/x86_64-linux-gnu/sys/random.h:36:35: error: expected declaration specifiers or ‘...’ before ‘(’ token
36 | __attr_access ((__write_only__, 1, 2));
| ^
/usr/include/x86_64-linux-gnu/sys/random.h:40:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘__wur’
40 | int getentropy (void *__buffer, size_t __length) __wur
| ^~~~~
source/platform/platform_util.c:309:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or __attribute__’ before ‘{’ token
309 | {
| ^
source/platform/platform_util.c:387:1: error: parameter ‘mbedtls_platform_dev_random’ is initialized
387 | const char *mbedtls_platform_dev_random = MBEDTLS_PLATFORM_DEV_RANDOM;
| ^~~~~
source/platform/platform_util.c:392:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or __attribute__’ before ‘{’ token
392 | {
| ^
/usr/include/x86_64-linux-gnu/sys/random.h:34:9: error: old-style parameter declarations in prototyped function definition
34 | ssize_t getrandom (void *__buffer, size_t __length,
| ^~~~~~~~~
source/platform/platform_util.c:447: error: expected ‘{’ at end of input
source/platform/platform_util.c:447: error: control reaches end of non-void function [-Werror=return-type]
cc1: all warnings being treated as errors
make: *** [Makefile:11984: platform/platform_util.o] Error 1
(on Ubuntu 24.04)
(I'm pretty sure that's why I didn't do that when I overhauled our random-on-Linux code recently.)
| @@ -0,0 +1,3 @@ | |||
| Bugfix | |||
There was a problem hiding this comment.
I wouldn't consider this a bug in TF-PSA-Crypto (but a bug in older Linux kernels). We're just not taking advantage of a nicer feature exposed by musl.
|
Thanks, you're right — checking only for the presence of Before updating the implementation, would you be open to an explicit opt-in configuration macro, for example |
|
We prefer to keep the number of compile-time options down, but if that's the only way, we'll accept it. But there's one thing I'm not sure I've tried: maybe the block that uses Ideally we'd like this to be covered in our CI, but that would require more work on our CI images than we have time for at the moment. So we'll settle for manual testing for now. |
|
That's a great idea—thanks for pointing it out. I replaced the
I also reclassified the changelog entry as a feature. Validation performed:
|
Signed-off-by: Ning Xia <hi@nosec.me>
7828d13 to
c615026
Compare
| @@ -0,0 +1,5 @@ | |||
| Features | |||
| * Use the `getrandom` system call for PSA crypto initialization on Linux | |||
There was a problem hiding this comment.
Cosmetic: changelog entries are text, not markdown, so they shouldn't use backticks. Also, it's conventional to write getrandom() with parentheses.
There was a problem hiding this comment.
I've removed the Markdown backticks and changed it to the conventional getrandom() spelling.
Signed-off-by: Ning Xia <hi@nosec.me>
Description
On Linux, the platform entropy implementation only enables
getrandom()forglibc and Midipix. musl provides a
getrandom()wrapper in<sys/random.h>butdoes not define
__GLIBC__, so affected builds fall back to/dev/random.This can make a second PSA crypto initialization block after the first process
has consumed the available entropy on Linux 5.6 and earlier.
Prefer the libc wrapper when
<sys/random.h>is available, while preservingthe existing raw-syscall path for older glibc/toolchain combinations and the
existing fallback when
getrandom()is unavailable at runtime.Validation:
PSA crypto initialization and reached the connection attempt without
blocking.
PR checklist