Commit af6a35c8 authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[runtime] Check for proper stack checks

When calling the {Isolate::StackOverflow} method, we should not have
overflown the stack limit by too much. Otherwise there might not be
enough space on the stack for handling the stack overflow exception.

This DCHECK would have failed before landing https://crrev.com/c/3059074
and https://crrev.com/c/3059075. If it fails, we might need to add more
special stack checks also in other places. Such failures should not be
considered security issues per se, but we should try to fix them to
avoid potential issues.

R=jkummerow@chromium.org
CC=ahaas@chromium.org

Bug: v8:12017
Change-Id: I25e42a20d3fcc981c266ae998f52b3f090237297
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3059076Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75998}
parent 580508f8
...@@ -644,8 +644,8 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source, ...@@ -644,8 +644,8 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
Local<Value> name, PrintResult print_result, Local<Value> name, PrintResult print_result,
ReportExceptions report_exceptions, ReportExceptions report_exceptions,
ProcessMessageQueue process_message_queue) { ProcessMessageQueue process_message_queue) {
if (i::FLAG_parse_only) {
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate); i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
if (i::FLAG_parse_only) {
i::VMState<PARSER> state(i_isolate); i::VMState<PARSER> state(i_isolate);
i::Handle<i::String> str = Utils::OpenHandle(*(source)); i::Handle<i::String> str = Utils::OpenHandle(*(source));
...@@ -681,6 +681,15 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source, ...@@ -681,6 +681,15 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
TryCatch try_catch(isolate); TryCatch try_catch(isolate);
try_catch.SetVerbose(report_exceptions == kReportExceptions); try_catch.SetVerbose(report_exceptions == kReportExceptions);
// Explicitly check for stack overflows. This method can be called
// recursively, and since we consume quite some stack space for the C++
// frames, the stack check in the called frame might be too late.
if (i::StackLimitCheck{i_isolate}.HasOverflowed()) {
i_isolate->StackOverflow();
i_isolate->OptionalRescheduleException(false);
return false;
}
MaybeLocal<Value> maybe_result; MaybeLocal<Value> maybe_result;
bool success = true; bool success = true;
{ {
......
...@@ -1392,6 +1392,15 @@ bool Isolate::MayAccess(Handle<Context> accessing_context, ...@@ -1392,6 +1392,15 @@ bool Isolate::MayAccess(Handle<Context> accessing_context,
} }
Object Isolate::StackOverflow() { Object Isolate::StackOverflow() {
// Whoever calls this method should not have overflown the stack limit by too
// much. Otherwise we risk actually running out of stack space.
// We allow for up to 8kB overflow, because we typically allow up to 4KB
// overflow per frame in generated code, but might call through more smaller
// frames until we reach this method.
// If this DCHECK fails, one of the frames on the stack should be augmented by
// an additional stack check.
DCHECK_GE(GetCurrentStackPosition(), stack_guard()->real_climit() - 8 * KB);
if (FLAG_correctness_fuzzer_suppressions) { if (FLAG_correctness_fuzzer_suppressions) {
FATAL("Aborting on stack overflow"); FATAL("Aborting on stack overflow");
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment