Skip to content

Commit

Permalink
Kernel: Make sure we free the thread stack on thread exit
Browse files Browse the repository at this point in the history
This adds two new arguments to the thread_exit system call which let
a thread unmap an arbitrary VM range on thread exit. LibPthread
uses this functionality to unmap the thread stack.

Fixes SerenityOS#7267.
  • Loading branch information
gunnarbeutner authored and awesomekling committed May 29, 2021
1 parent 95c2166 commit 42d6676
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Kernel/Process.h
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ class Process
KResultOr<int> sys$sched_setparam(pid_t pid, Userspace<const struct sched_param*>);
KResultOr<int> sys$sched_getparam(pid_t pid, Userspace<struct sched_param*>);
KResultOr<int> sys$create_thread(void* (*)(void*), Userspace<const Syscall::SC_create_thread_params*>);
[[noreturn]] void sys$exit_thread(Userspace<void*>);
[[noreturn]] void sys$exit_thread(Userspace<void*>, Userspace<void*>, size_t);
KResultOr<int> sys$join_thread(pid_t tid, Userspace<void**> exit_value);
KResultOr<int> sys$detach_thread(pid_t tid);
KResultOr<int> sys$set_thread_name(pid_t tid, Userspace<const char*> buffer, size_t buffer_size);
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Syscall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ KResultOr<FlatPtr> handle(RegisterState& regs, FlatPtr function, FlatPtr arg1, F
process.sys$exit(arg1);
break;
case SC_exit_thread:
process.sys$exit_thread(arg1);
process.sys$exit_thread(arg1, arg2, arg3);
break;
default:
VERIFY_NOT_REACHED();
Expand Down
8 changes: 7 additions & 1 deletion Kernel/Syscalls/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ KResultOr<int> Process::sys$create_thread(void* (*entry)(void*), Userspace<const
return thread->tid().value();
}

void Process::sys$exit_thread(Userspace<void*> exit_value)
void Process::sys$exit_thread(Userspace<void*> exit_value, Userspace<void*> stack_location, size_t stack_size)
{
REQUIRE_PROMISE(thread);

Expand All @@ -90,6 +90,12 @@ void Process::sys$exit_thread(Userspace<void*> exit_value)
auto current_thread = Thread::current();
PerformanceManager::add_thread_exit_event(*current_thread);

if (stack_location) {
auto unmap_result = space().unmap_mmap_range(VirtualAddress { stack_location }, stack_size);
if (unmap_result.is_error())
dbgln("Failed to unmap thread stack, terminating thread anyway. Error code: {}", unmap_result.error());
}

current_thread->exit(reinterpret_cast<void*>(exit_value.ptr()));
VERIFY_NOT_REACHED();
}
Expand Down
19 changes: 13 additions & 6 deletions Userland/Libraries/LibPthread/pthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,18 @@ static constexpr size_t required_stack_alignment = 4 * MiB;
static constexpr size_t highest_reasonable_guard_size = 32 * PAGE_SIZE;
static constexpr size_t highest_reasonable_stack_size = 8 * MiB; // That's the default in Ubuntu?

__thread void* s_stack_location;
__thread size_t s_stack_size;

#define __RETURN_PTHREAD_ERROR(rc) \
return ((rc) < 0 ? -(rc) : 0)

extern "C" {

static void* pthread_create_helper(void* (*routine)(void*), void* argument)
static void* pthread_create_helper(void* (*routine)(void*), void* argument, void* stack_location, size_t stack_size)
{
s_stack_location = stack_location;
s_stack_size = stack_size;
void* ret_val = routine(argument);
pthread_exit(ret_val);
return nullptr;
Expand All @@ -57,10 +62,12 @@ static int create_thread(pthread_t* thread, void* (*entry)(void*), void* argumen

// We set up the stack for pthread_create_helper.
// Note that we need to align the stack to 16B, accounting for
// the fact that we also push 8 bytes.
while (((uintptr_t)stack - 8) % 16 != 0)
// the fact that we also push 16 bytes.
while (((uintptr_t)stack - 16) % 16 != 0)
push_on_stack(nullptr);

push_on_stack((void*)thread_params->m_stack_size);
push_on_stack(thread_params->m_stack_location);
push_on_stack(argument);
push_on_stack((void*)entry);
VERIFY((uintptr_t)stack % 16 == 0);
Expand All @@ -74,10 +81,10 @@ static int create_thread(pthread_t* thread, void* (*entry)(void*), void* argumen
__RETURN_PTHREAD_ERROR(rc);
}

[[noreturn]] static void exit_thread(void* code)
[[noreturn]] static void exit_thread(void* code, void* stack_location, size_t stack_size)
{
__pthread_key_destroy_for_current_thread();
syscall(SC_exit_thread, code);
syscall(SC_exit_thread, code, stack_location, stack_size);
VERIFY_NOT_REACHED();
}

Expand Down Expand Up @@ -119,7 +126,7 @@ int pthread_create(pthread_t* thread, pthread_attr_t* attributes, void* (*start_

void pthread_exit(void* value_ptr)
{
exit_thread(value_ptr);
exit_thread(value_ptr, s_stack_location, s_stack_size);
}

void pthread_cleanup_push([[maybe_unused]] void (*routine)(void*), [[maybe_unused]] void* arg)
Expand Down

0 comments on commit 42d6676

Please sign in to comment.