_GNU_SOURCE redefined

[mu578]

Note: This is just a template, so feel free to use/remove the unnecessary things

Description

• Type: Bug
• Priority: Minor

---

Bug

OS
|linux|

mbed TLS build:
Version: 2.16.6 OS version: generic behaviors

entropy_poll.c line 24

- #if defined(__linux__)
- /* Ensure that syscall() is available even when compiling with -std=c99 */
- #define _GNU_SOURCE
- #endif

+ #if defined(__GLIBC__) && !defined(_GNU_SOURCE)
+ /* Ensure that syscall() is available even when compiling with -std=c99 */
+ #define _GNU_SOURCE
+ #endif

[gilles-peskine-arm]

Thanks! I see two issues here: that the redefinition of _GNU_SOURCE causes an error if _GNU_SOURCE was already defined with a non-empty expansion, and a change in the conditions under which _GNU_SOURCE is defined. Adding && !defined(_GNU_SOURCE) looks correct, but I'm less sure about the other change.

Systems where __linux__ is defined but not __GLIBC__: Linux kernel with a different libc. I'm not well-versed enough in the Linux libc ecosystem: would we want to define _GNU_SOURCE here? Does it matter?

Systems where __GLIBC__ is defined but not __linux__: Glibc on a different kernel (e.g. FreeBSD, Hurd). We don't want to invoke syscall(SYS_getrandom) there. So should it be defined(__GLIBC__) && defined(__linux__)?

[mu578]

_GNU_SOURCE is a GLIBC messy thingy; these days, there are linux'es without glibc. For the rest, you need a better detection contract; yes coupling is an option; as GLIBC is something different from the target OS; for instance, Android is linux based however its default libc is Bionic, so in your case:

+ #if defined(__linux__) &&  defined(__GLIBC__) && !defined(_GNU_SOURCE)
+ /* Ensure that syscall() is available even when compiling with -std=c99 */
+ #define _GNU_SOURCE
+ #endif

update: I will take a look where you make use of syscall(SYS_getrandom) to have the necessary context, the reasons; maybe there is a better way like using an extern call hence dropping out the need of symbol visibility; unfortunately, you have third party libs out there doing the same thing: playing with private definitions; avoiding doing it is always a better solution than attending a game of try catch.

[mpg]

I fully agree that we should not redefine _GNU_SOURCE if it's already defined, that is, we should add !defined(_GNU_SOURCE) to the condition.

Regarding the other part (adding defined(__GLIBC__) to the condition), the glibc is not the only libc out there that provides syscall() if _GNU_SOURCE is defined, for example uClibc looks at _GNU_SOURCE as well: https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/include/features.h - but it also defines __GLIBC__. So I don't think I have an objection to adding defined(__GLIBC__) to the condition.

Just out of curiosity: does defining _GNU_SOURCE actually cause problems with other libc implementation? I would assume when a non-GNU-compatible libc is in use, _GNU_SOURCE would be useless but harmless.

[mu578]

better including for not relaying on any GLIBC assumption and forwarding .

#include <sys/syscall.h>
extern long syscall(long number, ...);

#ifdef SYS_getrandom

...

// x86_64  318
// x86     355
// aarch64 278
// arm     384
// ppc64le 359

From manual:
      syscall():
           Since glibc 2.19:
               _DEFAULT_SOURCE
           Before glibc 2.19:
               _BSD_SOURCE || _SVID_SOURCE

• from repo history : 2020-08-05: glibc 2.32 released. 2.19 has been released on 2014-02-07.

[gilles-peskine-arm]

I think we can in fact remove defined(__GLIBC__) altogether and call the system call. The only question is whether __linux_ implies the availability of <sys/syscall.h> and syscall(). If you have some experience with the libc ecosystem on Linux, I'd appreciate comments on https://github.com/ARMmbed/mbedtls/pull/3731.

[mpg]

I think we can in fact remove defined(__GLIBC__) altogether and call the system call. The only question is whether __linux__ implies the availability of <sys/syscall.h> and syscall().

I don't understand your reasoning here. For me things go the other way: the first question is whether __linux__ implies the availability of <sys/syscall.h> and syscall(). After we've answered this question, we'll know whether we can in fact remove && defined(__GLIBC__) and just call the system call. Not before.

[gilles-peskine-arm]

@mpg Aren't we saying the same thing? I'm saying that if __linux__ implies <sys/syscall.h> and syscall(), then we can and should remove defined(__GLIBC__).

[mu578]

Good morning, the best solution would be to move this concern within a configure step. __linux__ macro is not enough you must exclude Android and certainly other linux-based OSes even if they declare __linux__. Their layouts are so different + it's not guaranteed they expose public syscalls and if even if this syscall is avalaible. For instance, Bionic has an arc4random/arc4random_buf implementation which could be used for bootstrapping instead; it's not optimal as it doesn't guarantee a variety of sources. However, you could immediatly apply a cipher tea pass on its output; if well implemented it should be re-seed from kernel-random.

[gilles-peskine-arm]

move this concern within a configure step

I personally agree, but Mbed TLS doesn't currently have a configure script (in the sense of platform autodetection), and there is a sensible argument that platform autodetection is complicated in brittle. (A counter-argument is that configure doesn't have to mean autoconf, which is the source of most of the backlash against platform autodetection.)

Without autodetection, we're left with manual configuration. Which is possible, but painful.

If we can't find a good generic solution, is there a specific platform that you care about and that we could make work reliably?

[mu578]

yes, first for __linux__ detection the rational would be #if __linux__ && !defined(ANDROID) and any excluded ones in the future then treating android separately. After that if you enter __linux__ you iterate LIBC types and versions;

# if __GLIBC__ > 2 || __GLIBC_MINOR__ > 24
#  include <sys/random.h>
# elif ...

if not identifiable like musl you fallback on _POSIX_VERSION macro then from there you should have at least:

       long int random(void);    
       void srandom(unsigned int seed);
       4.3BSD, POSIX.1-2001.

       long int lrand48(void);
       void srand48(long int seedval);    
       SVr4, POSIX.1-2001.

you may "hybriding" something with the two calls. Seeding could done based on hardware-clock availability __asm__ and so on. Giving an example on arm32:

#  if (defined(__arm__) && !(defined(__arm64__) || defined(__aarch64__)))
...
    __asm__ __volatile__ ("0:" "subs %[count], 1;" "bne 0b;" :[count]"+r"(C)); \
...

I am not a big fan either, I think people care about current popular embedded systems; iOS is clean-posix/BSD ; Android is barely-okish on some concerns. Any code availability by macro detection is painful, that's never easy-peasy; but I would say if you follow the following nested rule you mostly always get back on your feet:

OS/PLATFORM - then - C-RT LIB - then VERSION - then STANDARD - then HARDWARE.