From 849499988e3db08dea73f2444d91936710b91c6f Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 21 Nov 2022 11:18:15 +0100 Subject: [PATCH] LibJS+LibWeb: Make JS::ExecutionContext protect its Web::HTML::ESO owner We can't be nuking the ESO while its owned execution context is still on the VM's execution context stack, as that may lead to a use-after-free. This patch solves this by adding a `context_owner` field to each context and treating it as a GC root. --- Userland/Libraries/LibJS/Runtime/ExecutionContext.h | 3 +++ Userland/Libraries/LibJS/Runtime/VM.cpp | 2 ++ Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp | 2 ++ 3 files changed, 7 insertions(+) diff --git a/Userland/Libraries/LibJS/Runtime/ExecutionContext.h b/Userland/Libraries/LibJS/Runtime/ExecutionContext.h index c3427766191..cae575bdd1c 100644 --- a/Userland/Libraries/LibJS/Runtime/ExecutionContext.h +++ b/Userland/Libraries/LibJS/Runtime/ExecutionContext.h @@ -58,6 +58,9 @@ public: Environment* variable_environment { nullptr }; // [[VariableEnvironment]] PrivateEnvironment* private_environment { nullptr }; // [[PrivateEnvironment]] + // Non-standard: This points at something that owns this ExecutionContext, in case it needs to be protected from GC. + Cell* context_owner { nullptr }; + ASTNode const* current_node { nullptr }; FlyString function_name; Value this_value; diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index 40a17de1c1a..5a2a72cb4db 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -204,6 +204,8 @@ void VM::gather_roots(HashTable& roots) roots.set(execution_context->lexical_environment); roots.set(execution_context->variable_environment); roots.set(execution_context->private_environment); + if (auto* context_owner = execution_context->context_owner) + roots.set(context_owner); execution_context->script_or_module.visit( [](Empty) {}, [&](auto& script_or_module) { diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp index 653a38ec95a..3e34cb0f6cd 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp +++ b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp @@ -22,6 +22,8 @@ namespace Web::HTML { EnvironmentSettingsObject::EnvironmentSettingsObject(NonnullOwnPtr realm_execution_context) : m_realm_execution_context(move(realm_execution_context)) { + m_realm_execution_context->context_owner = this; + // Register with the responsible event loop so we can perform step 4 of "perform a microtask checkpoint". responsible_event_loop().register_environment_settings_object({}, *this); }