[ubsan] Fix "this == nullptr" in stack unwinding

StackHandlers form a chain, where the last element is nullptr,
so calling "handler->next()->foo()" is unsafe because "foo"
might see "this == nullptr".

Bug: v8:3770
Change-Id: Ic989384fa192e29d4d8cb76ff01b32173bf55fd9
Reviewed-on: https://chromium-review.googlesource.com/c/1400406
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58619}
This commit is contained in:
Jakob Kummerow 2019-01-08 10:27:09 +01:00 committed by Commit Bot
parent 874ae8a459
commit 40e8378f05
3 changed files with 12 additions and 7 deletions

View File

@ -51,6 +51,9 @@ inline StackHandler* StackHandler::next() const {
return FromAddress(Memory<Address>(address() + offset));
}
inline Address StackHandler::next_address() const {
return Memory<Address>(address() + StackHandlerConstants::kNextOffset);
}
inline StackHandler* StackHandler::FromAddress(Address address) {
return reinterpret_cast<StackHandler*>(address);

View File

@ -48,6 +48,10 @@ class StackHandler {
// Get the next stack handler in the chain.
inline StackHandler* next() const;
// Get the next stack handler, as an Address. This is safe to use even
// when the next handler is null.
inline Address next_address() const;
// Conversion support.
static inline StackHandler* FromAddress(Address address);
@ -158,7 +162,7 @@ class StackFrame {
// Copy constructor; it breaks the connection to host iterator
// (as an iterator usually lives on stack).
StackFrame(const StackFrame& original) {
StackFrame(const StackFrame& original) V8_NOEXCEPT {
this->state_ = original.state_;
this->iterator_ = nullptr;
this->isolate_ = original.isolate_;

View File

@ -1547,7 +1547,7 @@ Object Isolate::UnwindAndFindHandler() {
StackHandler* handler = frame->top_handler();
// Restore the next handler.
thread_local_top()->handler_ = handler->next()->address();
thread_local_top()->handler_ = handler->next_address();
// Gather information from the handler.
Code code = frame->LookupCode();
@ -1790,7 +1790,7 @@ Isolate::CatchType Isolate::PredictExceptionCatcher() {
switch (frame->type()) {
case StackFrame::ENTRY:
case StackFrame::CONSTRUCT_ENTRY: {
Address entry_handler = frame->top_handler()->next()->address();
Address entry_handler = frame->top_handler()->next_address();
// The exception has been externally caught if and only if there is an
// external handler which is on top of the top-most JS_ENTRY handler.
if (external_handler != kNullAddress &&
@ -2174,8 +2174,7 @@ void Isolate::ReportPendingMessagesFromJavaScript() {
// Get the top-most JS_ENTRY handler, cannot be on top if it doesn't exist.
Address entry_handler = Isolate::handler(thread_local_top());
DCHECK_NE(entry_handler, kNullAddress);
entry_handler =
reinterpret_cast<StackHandler*>(entry_handler)->next()->address();
entry_handler = StackHandler::FromAddress(entry_handler)->next_address();
// Get the address of the external handler so we can compare the address to
// determine which one is closer to the top of the stack.
@ -2192,8 +2191,7 @@ void Isolate::ReportPendingMessagesFromJavaScript() {
// Get the top-most JS_ENTRY handler, cannot be on top if it doesn't exist.
Address entry_handler = Isolate::handler(thread_local_top());
DCHECK_NE(entry_handler, kNullAddress);
entry_handler =
reinterpret_cast<StackHandler*>(entry_handler)->next()->address();
entry_handler = StackHandler::FromAddress(entry_handler)->next_address();
return (entry_handler > external_handler);
};