Disallow syscalls before libc is initialised, remove all existing early boot syscalls#2273
Merged
Merged
Conversation
Disables automatic locking when static variables are initialised from constructors. To delay initialisation of some static variables until the kernel is ready they are initialised from a constructor. This can for instance be useful when waiting for BSS to be cleared, RNG to be initialised or heap to be ready. As we have thread support in libcxx, the compiler will automatically add surrounding calls to __cxa_guard_acquire and __cxa_guard_release to make sure the variable is only set by a single thread/process. While locking they call syscalls FUTEX and GETTID. As this may happen early in the boot process (pre-libc) we are not prepared to handle syscalls and the results may be unexpected. One side effect is that they set errno in libc which has not been initialised to point to a valid location yet. With thread safe statics disabled we avoid this issue, but we have to manually lock in the kernel when SMP is enabled. Alternatives: An alternative would be to set _LIBCPP_HAS_NO_THREADS=1. This would remove thread support - but also features that can be useful later for SMP support, such as std::lock_guard. We would still have to lock manually when initialising static variables in IncludeOS' SMP mode. Bonus: Use C++20 when building libraries
Since we already read the ELF symbols, this also initializes the parser so the symbols are shown in stacktrace output. This is useful when e.g. debugging chainloader crashes. Also removes unused reference to _init_bss.
Previously we used std::map in memmap, which would proceed to call libc's malloc() before libc was initialised. This uses std::pmr::map instead and sets the default PMR allocator to be the same allocator as we later use in mmap/malloc. This allocator is initialised by the kernel as part of the heap. This change also allows the kernel to transparently use other std::pmr implementations after heap initialisation to avoid malloc calls.
Now that we have a heap PMR allocator we can avoid the calls to strdup(). In init_libc it was also incorrectly linked to libc's strdup, which resulted in malloc-calls and brk syscalls before libc was initialised.
This call to printf calls ioctl before libc is initialised. Replaced with INFO2()
Adds a new "syscalls_allowed" state variable to disallow syscalls before libc has been initialised. Since the libc intialisation itself uses syscalls we can't use libc_initialised, which will be set to true when libc returns from init. This commit depends on previous commits that clean up and remove calls to malloc and other syscalls in early boot. In particular we can't use the __cxa_guard-calls or std-implementations that rely on malloc being available. Std::pmr implementations should now work by default as long as the kernel heap has been initialised.
This was referenced Sep 4, 2024
Member
|
I think we relied heavily upon brk during startup before, which was probably fine. The SMP memalign call was unfortunate. |
Member
Author
It may have worked fine (and maybe it still does), but it seems much safer to avoid all the early syscalls if we can. There's also the write to errno in syscall_ret for instance, which we know may cause crashes (from #2268). The brk calls come from |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a panic on syscalls that happen before libc has been initialised. It also removes all the syscalls that prevented the kernel from booting after this check was added.
During early boot stages (before initialising libc) there were several hidden calls into malloc, which then called into the brk syscall. As libc wasn't initialised this could write to unprepared areas of memory (such as errno). This is fixed by adding a new default std::pmr allocator that can be used transparently in places we need to allocate. E.g. strdup has been replaced with std::pmr::string and std::pmr::map is used in memmap.
The kernel was compiled with threadsafe statics on, which added a guard lock around static variables that were initialised in the constructor. This again called into syscalls FUTEX and GETTID, which were not initialised. This is fixed by adding -fno-threadsafe-statics to the kernel and library builds.
There was a call to printf in multiboot that triggered an ioctl syscall.
See commits for additional details about each change.
Bonus: