[Webkit-unassigned] [Bug 270415] New: Stack protection race condition in "assembler" code

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=270415

            Bug ID: 270415
           Summary: Stack protection race condition in "assembler" code
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: JavaScriptCore
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: argosphil at murena.io

I found this issue while investigating why running Bun (https://bun.sh) in Valgrind produced uninitialized data warnings. My current conclusion is that it is indeed a bug in WebKit/JavaScriptCore, and my calculations indicate this could actually happen once in a while, so I thought I'd better report it here. However, it's entirely possible I'm totally wrong, and I'd like to apologize in that case.

JSC contains/generates this code:

Dump of assembler code for function op_call_slow_return_location:
   0x0000000008b2c2ad <+0>:     mov    0x10(%rbp),%rdx
   0x0000000008b2c2b1 <+4>:     mov    0x14(%rdx),%edx
   0x0000000008b2c2b4 <+7>:     shl    $0x3,%rdx
   0x0000000008b2c2b8 <+11>:    mov    %rbp,%rsp
   0x0000000008b2c2bb <+14>:    sub    %rdx,%rsp
   0x0000000008b2c2be <+17>:    mov    0x24(%rbp),%r8d
   0x0000000008b2c2c2 <+21>:    movsbq 0x1(%r13,%r8,1),%rsi
   0x0000000008b2c2c8 <+27>:    mov    %rax,0x0(%rbp,%rsi,8)
   0x0000000008b2c2cd <+32>:    movzbl 0x5(%r13,%r8,1),%edx
   0x0000000008b2c2d3 <+38>:    imul   $0xfffffffffffffff0,%rdx,%rdx
   0x0000000008b2c2d7 <+42>:    mov    %rax,-0x10(%r12,%rdx,1)
   0x0000000008b2c2dc <+47>:    add    $0x7,%r8
   0x0000000008b2c2e0 <+51>:    movzbl 0x0(%r13,%r8,1),%eax
   0x0000000008b2c2e6 <+57>:    lea    0x2396ab3(%rip),%rsi        # 0xaec2da0 <g_opcodeMap>
   0x0000000008b2c2ed <+64>:    jmp    *(%rsi,%rax,8)

The intention of the first five instructions is to change the stack pointer to a known value. However, it does so by first moving the base pointer to the stack pointer, then subtracting the new reservation from the stack pointer:

   0x0000000008b2c2b8 <+11>:    mov    %rbp,%rsp
   0x0000000008b2c2bb <+14>:    sub    %rdx,%rsp

If a signal happens between those two instructions, the signal handler will wrongly assume that the stack space below the base pointer is no longer used, and overwrite the stack. This is a highly theoretical possibility, since arithmetic operations are unlikely to be interrupted, but Valgrind caught it nonetheless and assumed, correctly, that the values in the intersection of the old and the new stack reservation could no longer be relied upon.

So how could this happen, given that for an assembly programmer, this is a beginner's mistake? The problem here is that JSC uses its own language which is meant to be something like machine-independent assembler code. The source code in this case is:

    subp cfr, t0, sp

(t0 is a temporary register, cfr is the base register, sp is the stack pointer, and the "p" suffix marks this as a pointer instruction)

That looks okay, but pre-APX x86 doesn't have a ternary subtraction instruction! It's translated into the unsafe mov + sub sequence above by Ruby code, which assumes it's performing arithmetic on GP registers.

I've fixed this locally by rewriting the "subp" instruction into a safe mov-sub sequence, which in turn gets translated into:

   0x0000000008b2c318 <+11>:    neg    %rdx
   0x0000000008b2c31b <+14>:    add    %rbp,%rdx
   0x0000000008b2c31e <+17>:    mov    %rdx,%rsp

Crisis averted! My Valgrind runs are clean now, though I've disabled GC.

What's a good way to fix this? I think a good way would be to add an additional temporary register operand to the subp instruction, maybe something like

    subp cfr, t0, sp [clobbering t0]

(of course the precise syntax here is a matter of personal taste, though I like making the clobber explicit. In practice it's likely to be identical to the second operand anyway).

The x86 subp implementation, and possibly others, should be fixed to complain loudly and refuse to assemble instructions which would involve temporary values (or clobber) the stack pointer register.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20240303/8060bed0/attachment.htm>