From bd792030e194a0bcce4defbf2298041244b54121 Mon Sep 17 00:00:00 2001 From: Michal Klocek Date: Thu, 6 Jun 2019 14:46:56 +0200 Subject: [Backport] Fix for security issue 951322 [builtins] Check for stack overflow in JSConstructStub Bug: chromium:951322 Change-Id: I9d17fbfc8e1bd95dab5607dc7758b10313dcfd26 Reviewed-by: Allan Sandfeld Jensen --- chromium/v8/src/builtins/arm/builtins-arm.cc | 35 ++++++---- chromium/v8/src/builtins/arm64/builtins-arm64.cc | 84 +++++++++++++----------- chromium/v8/src/builtins/ia32/builtins-ia32.cc | 56 +++++++++------- chromium/v8/src/builtins/x64/builtins-x64.cc | 48 ++++++++------ 4 files changed, 131 insertions(+), 92 deletions(-) diff --git a/chromium/v8/src/builtins/arm/builtins-arm.cc b/chromium/v8/src/builtins/arm/builtins-arm.cc index e30100452b8..3e98402fc35 100644 --- a/chromium/v8/src/builtins/arm/builtins-arm.cc +++ b/chromium/v8/src/builtins/arm/builtins-arm.cc @@ -98,6 +98,20 @@ static void GenerateTailCallToReturnedCode(MacroAssembler* masm, namespace { +void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args, + Register scratch, Label* stack_overflow) { + // Check the stack for overflow. We are not trying to catch + // interruptions (e.g. debug break and preemption) here, so the "real stack + // limit" is checked. + __ LoadRoot(scratch, Heap::kRealStackLimitRootIndex); + // Make scratch the space we have left. The stack might already be overflowed + // here which will cause scratch to become negative. + __ sub(scratch, sp, scratch); + // Check if the arguments will overflow the stack. + __ cmp(scratch, Operand(num_args, LSL, kPointerSizeLog2)); + __ b(le, stack_overflow); // Signed comparison. +} + void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- r0 : number of arguments @@ -109,6 +123,8 @@ void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { // ----------------------------------- Register scratch = r2; + Label stack_overflow; + Generate_StackOverflowCheck(masm, r0, scratch, &stack_overflow); // Enter a construct frame. { @@ -165,21 +181,14 @@ void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { __ add(sp, sp, Operand(scratch, LSL, kPointerSizeLog2 - kSmiTagSize)); __ add(sp, sp, Operand(kPointerSize)); __ Jump(lr); + __ bind(&stack_overflow); + { + FrameScope scope(masm, StackFrame::INTERNAL); + __ CallRuntime(Runtime::kThrowStackOverflow); + __ bkpt(0); // Unreachable code. + } } -void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args, - Register scratch, Label* stack_overflow) { - // Check the stack for overflow. We are not trying to catch - // interruptions (e.g. debug break and preemption) here, so the "real stack - // limit" is checked. - __ LoadRoot(scratch, Heap::kRealStackLimitRootIndex); - // Make scratch the space we have left. The stack might already be overflowed - // here which will cause scratch to become negative. - __ sub(scratch, sp, scratch); - // Check if the arguments will overflow the stack. - __ cmp(scratch, Operand(num_args, LSL, kPointerSizeLog2)); - __ b(le, stack_overflow); // Signed comparison. -} } // namespace diff --git a/chromium/v8/src/builtins/arm64/builtins-arm64.cc b/chromium/v8/src/builtins/arm64/builtins-arm64.cc index 6bad8b2849c..eb9cc734c6a 100644 --- a/chromium/v8/src/builtins/arm64/builtins-arm64.cc +++ b/chromium/v8/src/builtins/arm64/builtins-arm64.cc @@ -88,6 +88,44 @@ static void GenerateTailCallToReturnedCode(MacroAssembler* masm, namespace { +void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args, + Label* stack_overflow) { + UseScratchRegisterScope temps(masm); + Register scratch = temps.AcquireX(); + + // Check the stack for overflow. + // We are not trying to catch interruptions (e.g. debug break and + // preemption) here, so the "real stack limit" is checked. + Label enough_stack_space; + __ LoadRoot(scratch, Heap::kRealStackLimitRootIndex); + // Make scratch the space we have left. The stack might already be overflowed + // here which will cause scratch to become negative. + __ Sub(scratch, sp, scratch); + // Check if the arguments will overflow the stack. + __ Cmp(scratch, Operand(num_args, LSL, kPointerSizeLog2)); + __ B(le, stack_overflow); + +#if defined(V8_OS_WIN) + // Simulate _chkstk to extend stack guard page on Windows ARM64. + const int kPageSize = 4096; + Label chkstk, chkstk_done; + Register probe = temps.AcquireX(); + + __ Sub(scratch, sp, Operand(num_args, LSL, kPointerSizeLog2)); + __ Mov(probe, sp); + + // Loop start of stack probe. + __ Bind(&chkstk); + __ Sub(probe, probe, kPageSize); + __ Cmp(probe, scratch); + __ B(lo, &chkstk_done); + __ Ldrb(xzr, MemOperand(probe)); + __ B(&chkstk); + + __ Bind(&chkstk_done); +#endif +} + void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { Label post_instantiation_deopt_entry; @@ -101,6 +139,8 @@ void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { // ----------------------------------- ASM_LOCATION("Builtins::Generate_JSConstructStubHelper"); + Label stack_overflow; + Generate_StackOverflowCheck(masm, x0, &stack_overflow); // Enter a construct frame. { @@ -191,46 +231,16 @@ void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { // Remove caller arguments from the stack and return. __ DropArguments(x1, TurboAssembler::kCountExcludesReceiver); __ Ret(); -} -void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args, - Label* stack_overflow) { - UseScratchRegisterScope temps(masm); - Register scratch = temps.AcquireX(); - - // Check the stack for overflow. - // We are not trying to catch interruptions (e.g. debug break and - // preemption) here, so the "real stack limit" is checked. - Label enough_stack_space; - __ LoadRoot(scratch, Heap::kRealStackLimitRootIndex); - // Make scratch the space we have left. The stack might already be overflowed - // here which will cause scratch to become negative. - __ Sub(scratch, sp, scratch); - // Check if the arguments will overflow the stack. - __ Cmp(scratch, Operand(num_args, LSL, kPointerSizeLog2)); - __ B(le, stack_overflow); - -#if defined(V8_OS_WIN) - // Simulate _chkstk to extend stack guard page on Windows ARM64. - const int kPageSize = 4096; - Label chkstk, chkstk_done; - Register probe = temps.AcquireX(); - - __ Sub(scratch, sp, Operand(num_args, LSL, kPointerSizeLog2)); - __ Mov(probe, sp); - - // Loop start of stack probe. - __ Bind(&chkstk); - __ Sub(probe, probe, kPageSize); - __ Cmp(probe, scratch); - __ B(lo, &chkstk_done); - __ Ldrb(xzr, MemOperand(probe)); - __ B(&chkstk); - - __ Bind(&chkstk_done); -#endif + __ Bind(&stack_overflow); + { + FrameScope scope(masm, StackFrame::INTERNAL); + __ CallRuntime(Runtime::kThrowStackOverflow); + __ Unreachable(); + } } + } // namespace // The construct stub for ES5 constructor functions and ES6 class constructors. diff --git a/chromium/v8/src/builtins/ia32/builtins-ia32.cc b/chromium/v8/src/builtins/ia32/builtins-ia32.cc index 69ddc00d0e8..9bccfd46f15 100644 --- a/chromium/v8/src/builtins/ia32/builtins-ia32.cc +++ b/chromium/v8/src/builtins/ia32/builtins-ia32.cc @@ -67,6 +67,30 @@ static void GenerateTailCallToReturnedCode(MacroAssembler* masm, namespace { +void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args, + Register scratch, Label* stack_overflow, + bool include_receiver = false) { + // Check the stack for overflow. We are not trying to catch + // interruptions (e.g. debug break and preemption) here, so the "real stack + // limit" is checked. + ExternalReference real_stack_limit = + ExternalReference::address_of_real_stack_limit(masm->isolate()); + // Compute the space that is left as a negative number in scratch. If + // we already overflowed, this will be a positive number. + __ mov(scratch, Operand::StaticVariable(real_stack_limit)); + __ sub(scratch, esp); + // Add the size of the arguments. + static_assert(kPointerSize == 4, + "The next instruction assumes kPointerSize == 4"); + __ lea(scratch, Operand(scratch, num_args, times_4, 0)); + if (include_receiver) { + __ add(scratch, Immediate(kPointerSize)); + } + // See if we overflowed, i.e. scratch is positive. + __ cmp(scratch, Immediate(0)); + __ j(greater, stack_overflow); // Signed comparison. +} + void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- eax: number of arguments @@ -75,6 +99,9 @@ void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { // -- esi: context // ----------------------------------- + Label stack_overflow; + Generate_StackOverflowCheck(masm, eax, ecx, &stack_overflow); + // Enter a construct frame. { FrameScope scope(masm, StackFrame::CONSTRUCT); @@ -131,32 +158,15 @@ void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { __ lea(esp, Operand(esp, ebx, times_2, 1 * kPointerSize)); // 1 ~ receiver __ push(ecx); __ ret(0); -} - -void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args, - Register scratch, Label* stack_overflow, - bool include_receiver = false) { - // Check the stack for overflow. We are not trying to catch - // interruptions (e.g. debug break and preemption) here, so the "real stack - // limit" is checked. - ExternalReference real_stack_limit = - ExternalReference::address_of_real_stack_limit(masm->isolate()); - // Compute the space that is left as a negative number in scratch. If - // we already overflowed, this will be a positive number. - __ mov(scratch, Operand::StaticVariable(real_stack_limit)); - __ sub(scratch, esp); - // Add the size of the arguments. - static_assert(kPointerSize == 4, - "The next instruction assumes kPointerSize == 4"); - __ lea(scratch, Operand(scratch, num_args, times_4, 0)); - if (include_receiver) { - __ add(scratch, Immediate(kPointerSize)); + __ bind(&stack_overflow); + { + FrameScope scope(masm, StackFrame::INTERNAL); + __ CallRuntime(Runtime::kThrowStackOverflow); + __ int3(); // This should be unreachable. } - // See if we overflowed, i.e. scratch is positive. - __ cmp(scratch, Immediate(0)); - __ j(greater, stack_overflow); // Signed comparison. } + } // namespace // The construct stub for ES5 constructor functions and ES6 class constructors. diff --git a/chromium/v8/src/builtins/x64/builtins-x64.cc b/chromium/v8/src/builtins/x64/builtins-x64.cc index 1c7413722b7..cc23ce38230 100644 --- a/chromium/v8/src/builtins/x64/builtins-x64.cc +++ b/chromium/v8/src/builtins/x64/builtins-x64.cc @@ -67,6 +67,26 @@ static void GenerateTailCallToReturnedCode(MacroAssembler* masm, namespace { +void Generate_StackOverflowCheck( + MacroAssembler* masm, Register num_args, Register scratch, + Label* stack_overflow, + Label::Distance stack_overflow_distance = Label::kFar) { + // Check the stack for overflow. We are not trying to catch + // interruptions (e.g. debug break and preemption) here, so the "real stack + // limit" is checked. + __ LoadRoot(kScratchRegister, Heap::kRealStackLimitRootIndex); + __ movp(scratch, rsp); + // Make scratch the space we have left. The stack might already be overflowed + // here which will cause scratch to become negative. + __ subp(scratch, kScratchRegister); + __ sarp(scratch, Immediate(kPointerSizeLog2)); + // Check if the arguments will overflow the stack. + __ cmpp(scratch, num_args); + // Signed comparison. + __ j(less_equal, stack_overflow, stack_overflow_distance); +} + + void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- rax: number of arguments @@ -75,6 +95,9 @@ void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { // -- rsi: context // ----------------------------------- + Label stack_overflow; + Generate_StackOverflowCheck(masm, rax, rcx, &stack_overflow, Label::kFar); + // Enter a construct frame. { FrameScope scope(masm, StackFrame::CONSTRUCT); @@ -132,25 +155,12 @@ void Generate_JSBuiltinsConstructStubHelper(MacroAssembler* masm) { __ PushReturnAddressFrom(rcx); __ ret(0); -} - -void Generate_StackOverflowCheck( - MacroAssembler* masm, Register num_args, Register scratch, - Label* stack_overflow, - Label::Distance stack_overflow_distance = Label::kFar) { - // Check the stack for overflow. We are not trying to catch - // interruptions (e.g. debug break and preemption) here, so the "real stack - // limit" is checked. - __ LoadRoot(kScratchRegister, Heap::kRealStackLimitRootIndex); - __ movp(scratch, rsp); - // Make scratch the space we have left. The stack might already be overflowed - // here which will cause scratch to become negative. - __ subp(scratch, kScratchRegister); - __ sarp(scratch, Immediate(kPointerSizeLog2)); - // Check if the arguments will overflow the stack. - __ cmpp(scratch, num_args); - // Signed comparison. - __ j(less_equal, stack_overflow, stack_overflow_distance); + __ bind(&stack_overflow); + { + FrameScope scope(masm, StackFrame::INTERNAL); + __ CallRuntime(Runtime::kThrowStackOverflow); + __ int3(); // This should be unreachable. + } } } // namespace -- cgit v1.2.3