mirror of
https://github.com/RGBCube/serenity
synced 2025-06-01 10:08:10 +00:00
LibC: Avoid unnecessary mprotect during program finalization
In particular, we track separately whether each AtExitEntry has already been called, through a separate Bitmap. This has several side-effects: - We now call malloc() during __cxa_finalize(). I believe this is fine, and at that point during program execution memory pressure should be low anyway. - An attacker could prevent arbitrary entries from executing by writing to atexit_called_entries. However, this already was possible (by setting atexit_entry_count to zero), and this path is even more troublesome (the attacker needs to overwrite atexit_called_entries, and a region serving as *atexit_called_entries.m_data, and magically know exactly how many entries already exist.) - This reduces the size of AtExitEntry from 16 to 12 (on i686). As such, we can reduce the initial memory allocation from two to one page, reducing the initial capacity from 512 to 341 entries (or 256 to 170, on x86_64). It seems that most programs only use 36-47 entries anyway. For 'true', this shaves off about 69 syscalls, as measured by strace.
This commit is contained in:
parent
26b647f303
commit
fb003d71c2
1 changed files with 14 additions and 7 deletions
|
@ -4,9 +4,11 @@
|
|||
* SPDX-License-Identifier: BSD-2-Clause
|
||||
*/
|
||||
|
||||
#include <AK/Bitmap.h>
|
||||
#include <AK/Checked.h>
|
||||
#include <AK/Debug.h>
|
||||
#include <AK/Format.h>
|
||||
#include <AK/NeverDestroyed.h>
|
||||
#include <LibC/bits/pthread_integration.h>
|
||||
#include <LibC/mallocdefs.h>
|
||||
#include <assert.h>
|
||||
|
@ -23,12 +25,11 @@ struct AtExitEntry {
|
|||
AtExitFunction method { nullptr };
|
||||
void* parameter { nullptr };
|
||||
void* dso_handle { nullptr };
|
||||
bool has_been_called { false };
|
||||
};
|
||||
|
||||
// We'll re-allocate the region if it ends up being too small at runtime.
|
||||
// Invariant: atexit_entry_region_capacity * sizeof(AtExitEntry) does not overflow.
|
||||
static size_t atexit_entry_region_capacity = (2 * PAGE_SIZE) / sizeof(AtExitEntry);
|
||||
static size_t atexit_entry_region_capacity = PAGE_SIZE / sizeof(AtExitEntry);
|
||||
|
||||
static size_t atexit_region_bytes(size_t capacity = atexit_entry_region_capacity)
|
||||
{
|
||||
|
@ -46,6 +47,11 @@ static AtExitEntry* atexit_entries;
|
|||
static size_t atexit_entry_count = 0;
|
||||
static pthread_mutex_t atexit_mutex = __PTHREAD_MUTEX_INITIALIZER;
|
||||
|
||||
// The C++ compiler automagically registers the destructor of this object with __cxa_atexit.
|
||||
// However, we can't control the order in which these destructors are run, so we might still want to access this data after the registered entry.
|
||||
// Hence, we will call the destructor manually, when we know it is safe to do so.
|
||||
static NeverDestroyed<Bitmap> atexit_called_entries;
|
||||
|
||||
// During startup, it is sufficiently unlikely that the attacker can exploit any write primitive.
|
||||
// We use this to avoid unnecessary syscalls to mprotect.
|
||||
static bool atexit_region_should_lock = false;
|
||||
|
@ -109,7 +115,7 @@ int __cxa_atexit(AtExitFunction exit_function, void* parameter, void* dso_handle
|
|||
}
|
||||
|
||||
unlock_atexit_handlers();
|
||||
atexit_entries[atexit_entry_count++] = { exit_function, parameter, dso_handle, false };
|
||||
atexit_entries[atexit_entry_count++] = { exit_function, parameter, dso_handle };
|
||||
lock_atexit_handlers();
|
||||
|
||||
__pthread_mutex_unlock(&atexit_mutex);
|
||||
|
@ -128,18 +134,19 @@ void __cxa_finalize(void* dso_handle)
|
|||
|
||||
__pthread_mutex_lock(&atexit_mutex);
|
||||
|
||||
if (atexit_entry_count > atexit_called_entries->size())
|
||||
atexit_called_entries->grow(atexit_entry_count, false);
|
||||
|
||||
ssize_t entry_index = atexit_entry_count;
|
||||
|
||||
dbgln_if(GLOBAL_DTORS_DEBUG, "__cxa_finalize: {} entries in the finalizer list", entry_index);
|
||||
|
||||
while (--entry_index >= 0) {
|
||||
auto& exit_entry = atexit_entries[entry_index];
|
||||
bool needs_calling = !exit_entry.has_been_called && (!dso_handle || dso_handle == exit_entry.dso_handle);
|
||||
bool needs_calling = !atexit_called_entries->get(entry_index) && (!dso_handle || dso_handle == exit_entry.dso_handle);
|
||||
if (needs_calling) {
|
||||
dbgln_if(GLOBAL_DTORS_DEBUG, "__cxa_finalize: calling entry[{}] {:p}({:p}) dso: {:p}", entry_index, exit_entry.method, exit_entry.parameter, exit_entry.dso_handle);
|
||||
unlock_atexit_handlers();
|
||||
exit_entry.has_been_called = true;
|
||||
lock_atexit_handlers();
|
||||
atexit_called_entries->set(entry_index, true);
|
||||
__pthread_mutex_unlock(&atexit_mutex);
|
||||
exit_entry.method(exit_entry.parameter);
|
||||
__pthread_mutex_lock(&atexit_mutex);
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue