tinygo-org / tinygo

Go compiler for small places. Microcontrollers, WebAssembly (WASM/WASI), and command-line tools. Based on LLVM.
https://tinygo.org
Other
15.38k stars 906 forks source link

Memory corruption in WebAssembly #2876

Closed aykevl closed 2 years ago

aykevl commented 2 years ago

While trying to enable ThinLTO on WebAssembly, I've found an issue with memory corruption. I managed to reproduce it with a small patch that should be harmless but triggers this bug.

diff --git a/src/runtime/gc_conservative.go b/src/runtime/gc_conservative.go
index 00cadc30..f2a0b268 100644
--- a/src/runtime/gc_conservative.go
+++ b/src/runtime/gc_conservative.go
@@ -168,6 +168,11 @@ func (b gcBlock) markFree() {
        if gcAsserts && b.state() != blockStateFree {
                runtimePanic("gc: markFree() was not successful")
        }
+       addr := b.address()
+       *(*uintptr)(unsafe.Pointer(addr + 0)) = 0
+       *(*uintptr)(unsafe.Pointer(addr + 4)) = 0
+       *(*uintptr)(unsafe.Pointer(addr + 8)) = 0
+       *(*uintptr)(unsafe.Pointer(addr + 12)) = 0
 }

 // unmark changes the state of the block from mark to head. It must be marked
@@ -264,6 +269,7 @@ func alloc(size uintptr, layout unsafe.Pointer) unsafe.Pointer {
        if size == 0 {
                return unsafe.Pointer(&zeroSizedAlloc)
        }
+       GC()

        neededBlocks := (size + (bytesPerBlock - 1)) / bytesPerBlock

What it does, is:

  1. Zero memory freed by the GC (it's free memory, so should be safe).
  2. Run the GC before each allocation.

And then I run any program like this:

$ tinygo run -target=wasi -opt=1 ./testdata/alias.go
Error: failed to run main module `/tmp/tinygo2191174038/main`

Caused by:
    0: failed to invoke command default
    1: wasm trap: uninitialized element
       wasm backtrace:
           0: 0x2701 - <unknown>!tinygo_launch
           1:  0x537 - (*internal/task.Task).Resume
                           at /home/ayke/src/github.com/tinygo-org/tinygo/src/internal/task/task_asyncify.go:110:17
           2: 0x23a4 - runtime.scheduler
                           at /home/ayke/src/github.com/tinygo-org/tinygo/src/runtime/scheduler.go:167:11
           3: 0x22fc - runtime.run
                           at /home/ayke/src/github.com/tinygo-org/tinygo/src/runtime/scheduler_any.go:28:11
           4: 0x2283 - _start
                           at /home/ayke/src/github.com/tinygo-org/tinygo/src/runtime/runtime_wasm_wasi.go:21:5

(The -opt=1 is important, otherwise it won't reproduce).

Apparently t.state.entry is zero, which it shouldn't be.

aykevl commented 2 years ago

Apparently what happens is this:

  1. internal/task.start allocates some memory
  2. (*internal/task.state).initialize allocates some more memory
  3. runtime.markStack only marks the objects in (*internal/task.state).initialize, not in internal/task.start.
  4. why???
aykevl commented 2 years ago

@niaow any idea what might be going on? I'm suspecting a problem with wasm-opt, probably Asyncify. With a quick comparison, I found that (*internal/task.state).initialize is a lot bigger after wasm-opt, so it looks like it had the Asyncify pass applied to it.

niaow commented 2 years ago

From my initial look at the IR, the internal/task.start function seems to be removing its element of the stack chain early (before calling initialize), which creates a gap when the task is not tracked.

niaow commented 2 years ago

I think it is a bug with this part of the stack slots pass, which places the stack chain restore after the last time a pointer is created locally. However, when an allocation is done in a function after the last time a local pointer is created, this can cause a use-after-free: https://github.com/tinygo-org/tinygo/blob/cf0b7edc78e42038a0bb522b3f1a5b76928e730e/transform/gc.go#L275-L303

niaow commented 2 years ago

In order to fix this, I think we need to strip the tail flags and put this immediately before the return.

niaow commented 2 years ago

Another interesting bug is that the MakeGCStackSlots pass does not treat the runtime.GC function as a garbage collection trigger.

niaow commented 2 years ago

Currently working on some fixes, although it looks like we may have some other bugs too. . .

deadprogram commented 2 years ago

This fix was released in TinyGo v0.24.0 so now closing.