Changeset 211479 in webkit


Ignore:
Timestamp:
Feb 1, 2017 3:29:25 AM (6 years ago)
Author:
Yusuke Suzuki
Message:

ArityFixup should adjust SP first
https://bugs.webkit.org/show_bug.cgi?id=167239

Reviewed by Michael Saboff.

JSTests:

Significantly large arity fixup reliably causes this crash.

  • stress/arity-fixup-should-not-touch-stack-area-below-sp.js: Added.

Source/JavaScriptCore:

Arity fixup extends the stack and copy/fill the stack with
the values. At that time, we accidentally read/write stack
space below the stack pointer. As a result, we touch the area
of the stack space below the x64 red zone. These areas are unsafe.
OS may corrupt this space when constructing a signal stack.
The Linux kernel could not populate the pages for this space
and causes segmentation fault. This patch changes the stack
pointer before performing the arity fixup.

  • jit/ThunkGenerators.cpp:

(JSC::arityFixupGenerator):

  • llint/LowLevelInterpreter32_64.asm:
  • llint/LowLevelInterpreter64.asm:
Location:
trunk
Files:
1 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r211464 r211479  
     12017-02-01  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        ArityFixup should adjust SP first
     4        https://bugs.webkit.org/show_bug.cgi?id=167239
     5
     6        Reviewed by Michael Saboff.
     7
     8        Significantly large arity fixup reliably causes this crash.
     9
     10        * stress/arity-fixup-should-not-touch-stack-area-below-sp.js: Added.
     11
    1122017-01-31  Filip Pizlo  <fpizlo@apple.com>
    213
  • trunk/Source/JavaScriptCore/ChangeLog

    r211463 r211479  
     12017-02-01  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        ArityFixup should adjust SP first
     4        https://bugs.webkit.org/show_bug.cgi?id=167239
     5
     6        Reviewed by Michael Saboff.
     7
     8        Arity fixup extends the stack and copy/fill the stack with
     9        the values. At that time, we accidentally read/write stack
     10        space below the stack pointer. As a result, we touch the area
     11        of the stack space below the x64 red zone. These areas are unsafe.
     12        OS may corrupt this space when constructing a signal stack.
     13        The Linux kernel could not populate the pages for this space
     14        and causes segmentation fault. This patch changes the stack
     15        pointer before performing the arity fixup.
     16
     17        * jit/ThunkGenerators.cpp:
     18        (JSC::arityFixupGenerator):
     19        * llint/LowLevelInterpreter32_64.asm:
     20        * llint/LowLevelInterpreter64.asm:
     21
    1222017-01-31  Filip Pizlo  <fpizlo@apple.com>
    223
  • trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp

    r210232 r211479  
    441441    jit.neg64(JSInterfaceJIT::argumentGPR0);
    442442
     443    // Adjust call frame register and stack pointer to account for missing args.
     444    // We need to change the stack pointer first before performing copy/fill loops.
     445    // This stack space below the stack pointer is considered unsed by OS. Therefore,
     446    // OS may corrupt this space when constructing a signal stack.
     447    jit.move(JSInterfaceJIT::argumentGPR0, extraTemp);
     448    jit.lshift64(JSInterfaceJIT::TrustedImm32(3), extraTemp);
     449    jit.addPtr(extraTemp, JSInterfaceJIT::callFrameRegister);
     450    jit.addPtr(extraTemp, JSInterfaceJIT::stackPointerRegister);
     451
    443452    // Move current frame down argumentGPR0 number of slots
    444453    JSInterfaceJIT::Label copyLoop(jit.label());
     
    456465    jit.branchAdd32(MacroAssembler::NonZero, JSInterfaceJIT::TrustedImm32(1), JSInterfaceJIT::argumentGPR2).linkTo(fillUndefinedLoop, &jit);
    457466   
    458     // Adjust call frame register and stack pointer to account for missing args
    459     jit.move(JSInterfaceJIT::argumentGPR0, extraTemp);
    460     jit.lshift64(JSInterfaceJIT::TrustedImm32(3), extraTemp);
    461     jit.addPtr(extraTemp, JSInterfaceJIT::callFrameRegister);
    462     jit.addPtr(extraTemp, JSInterfaceJIT::stackPointerRegister);
    463 
    464467    done.link(&jit);
    465468
  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

    r210276 r211479  
    614614    negi t1
    615615    move cfr, t3
     616    move t1, t0
     617    lshiftp 3, t0
     618    addp t0, cfr
     619    addp t0, sp
    616620.copyLoop:
    617621    loadi PayloadOffset[t3], t0
     
    632636    baddinz 1, t2, .fillLoop
    633637
    634     lshiftp 3, t1
    635     addp t1, cfr
    636     addp t1, sp
    637638.continue:
    638639    # Reload CodeBlock and PC, since the slow_path clobbered it.
  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

    r210276 r211479  
    526526    subp CalleeSaveSpaceAsVirtualRegisters * 8, t3
    527527    addi CalleeSaveSpaceAsVirtualRegisters, t2
     528    move t1, t0
     529    lshiftp 3, t0
     530    addp t0, cfr
     531    addp t0, sp
    528532.copyLoop:
    529533    loadq [t3], t0
     
    539543    addp 8, t3
    540544    baddinz 1, t2, .fillLoop
    541 
    542     lshiftp 3, t1
    543     addp t1, cfr
    544     addp t1, sp
    545545
    546546.continue:
Note: See TracChangeset for help on using the changeset viewer.