OP-TEE / optee_os

Trusted side of the TEE
Other
1.6k stars 1.07k forks source link

Remove libgcc linkage in optee-os #5856

Closed rossburton closed 10 months ago

rossburton commented 1 year ago

I was surprised when I noticed that optee-os needs to link against libgcc, as other pieces of firmware (such as TF-A) don't.

libgcc is typically used by only userspace software as it is tied to the libc being used, and has interesting behaviour like calling __getauxval() to determine platform capabilities, expecting that the kernel ELF loader has set that up. This is worked around in 36e784f621bf5d5be9183beba35f39426277c110 which results in LSE atomics not being used in C++ TAs even when the hardware supports it.

We did a little digging (read: deleted all linkage to libgcc) and the optee-os build we did failed with just one missing symbol, __udivti3, via mbedtls. This can either be implemented in optee-os directly, or mbedtls can be told to not use those intrinsics with -DMBEDTLS_NO_UDBL_DIVISION. I can’t see any other reason to use libgcc, it has just always been there since the initial commit, and the use of libgcc brings considerable concerns about code which expects to be running in userspace actually running in firmware.

jenswi-linaro commented 1 year ago

the use of libgcc brings considerable concerns about code which expects to be running in userspace actually running in firmware.

Hear, hear! We touched on this at the last LOC (https://www.trustedfirmware.org/meetings/ Feb 28). I see two ways forward:

  1. replace libgcc.a with something provided by libutils
  2. compile a special version of libgcc.a suitable for OP-TEE

Keep in mind that this should work for both AArch32 and AArch64.

CJKay commented 1 year ago

libgcc is typically used by only userspace software as it is tied to the libc being used, and has interesting behaviour like calling __getauxval() to determine platform capabilities, expecting that the kernel ELF loader has set that up.

This appears to be specific to aarch64-linux-* distributions of GCC. There's only one instance of __getauxval being called in libgcc, and it's gated on glibc being available:

https://github.com/gcc-mirror/gcc/blob/4ee2f419fe42222dbb21dfb0622cc7c0e46a2261/libgcc/config/aarch64/lse-init.c#L34C21-L50

libgcc has very little concept of "user-space" in the Linux sense - it knows only what can be derived by the target triple and compiler/linker flags and what's bundled with the toolchain. On aarch64-linux-* targets that's generally a full-blown hosted libc, but on aarch64-none-* targets that should be little more than a minimal freestanding libc. If you're using the former and expecting it to work like the latter, that's probably the real problem!

gyuri-szing commented 1 year ago

about code which expects to be running in userspace

Let me challenge this. IMHO if libgcc.a does things not suitable for freestanding/bare-metal environments that is a bug in the implementation. libgcc.a is a way for the compiler to add compiler specific code to the executable if and when this is needed. The design and dependency between the compiler and this library is completely implementation defined, and it will be hard and error prone to rewrite it's functionality in an independent project.

rossburton commented 1 year ago

So https://github.com/OP-TEE/optee_os/commit/36e784f621bf5d5be9183beba35f39426277c110 should be reverted and instead the build fail if a compiler that isn't built for -none- is used?

CJKay commented 1 year ago

That'd be my suggestion - you can use GCC's -dumpmachine flag to determine the target triple. You can, in theory, use a Linux toolchain and strip it of all of its runtime components, but the runtime libraries move with the requirements of the compiler and are designed around GCC, so there's no guarantee (that my searching has yielded at least) that they'll work with anything other than the version of GCC that they were distributed with.

gyuri-szing commented 1 year ago

Looking at the C99 standard chapter 4.6, in freestanding environment only a set of "macro definitions only" header files are required to be available. That means if -ffreestanding is used GCC shall not emit any code depending on libgcc (or libc). The GCC documentation sys GCC is not fully compliant and has the following:

Most of the compiler support routines used by GCC are present in ‘libgcc’, but there are a few exceptions. GCC requires the freestanding environment provide memcpy, memmove, memset and memcmp. Finally, if __builtin_trap is used, and the target does not implement the trap pattern, then GCC emits a call to abort.

This might mean two things:

Depending on the answer linking libgcc might be wrong. Or if it is needed libgcc depending on any libc implementation when -ffreestanding is used is an error.

Thoughts?

1: https://www.dii.uchile.cl/~daespino/files/Iso_C_1999_definition.pdf 2: https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc.pdf

CJKay commented 1 year ago

libgcc is definitely expected to still be present in freestanding programs, as is a freestanding C standard library.

GCC is not a conforming freestanding C implementation precisely because it relies on some functions from string.h, which is not required to be a part of a conforming freestanding C standard library. Whether that reliance is because of a compiler-generated function call or a function call in libgcc is not specified, because libgcc is canonically a part of the compiler implementation.

gyuri-szing commented 1 year ago

libgcc is canonically a part of the compiler implementation.

IMHO if that is true, then libgcc depending on glibc is a bug and restricting the compiler to -none- triplets should be compiler version specific.

CJKay commented 1 year ago

libgcc is canonically a part of the compiler implementation.

IMHO if that is true, then libgcc depending on glibc is a bug and restricting the compiler to -none- triplets should be compiler version specific.

GCC only relies on glibc in glibc distributions of GCC. To what extent the compiler relies on its libc is an implementation detail, but it's exceedingly difficult to separate the compiler from its libc in all environments, particularly hosted ones like Linux. The standard refers to "a conforming implementation" rather than "a conforming compiler" or "a conforming standard library" for that reason.

IIRC GCC distributions like the Arm GNU Toolchain are configured with inhibit_libc to prevent the runtimes from relying on libc implementation details, so these should be fine to use.

github-actions[bot] commented 1 year ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

jenswi-linaro commented 1 year ago

So if I understand this correctly we should use (and note a dependency on) a GCC distribution configured with inhibit_libc. The alternative is to use a compiler with -none- in the triplet when building OP-TEE Core and the TAs.

github-actions[bot] commented 1 year ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

rossburton commented 1 year ago

..

github-actions[bot] commented 1 year ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

rossburton commented 1 year ago

Still an issue.

jforissier commented 1 year ago

Still an issue.

From what I understand (your first message), you managed to get things to build without libgcc. So how about creating a PR with that and we'll see if it passes CI and review ;)

github-actions[bot] commented 1 year ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

rossburton commented 1 year ago

Failed to ping this in time.

I still think something needs to happen.

If optee-os is good with linking to libgcc (as appears to be essential for C++ parts) then it shouldn't implement a stub __getauxval that just returns 0. This would likely mean that people can't use a "desktop" GCC to build optee-os. Alternatively, the use of libgcc should be removed, but it appears that would break C++ TAs.

Disclaimer: I'm very much not an expert on this so may well be very wrong.

github-actions[bot] commented 1 year ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

jforissier commented 1 year ago

@rossburton as a first step, would you create a PR to remove the use of libgcc in the OP-TEE OS kernel? (core/). If I understand correctly there is no difficulty in this, except maybe for implementing __udivti3(). The case of TAs is more problematic due to C++ support.

github-actions[bot] commented 1 year ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

github-actions[bot] commented 1 year ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

rossburton commented 11 months ago

Bah, can someone reopen this please.

github-actions[bot] commented 10 months ago

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.