Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: stack split at bad time during fuzzing #53190

Closed
catenacyber opened this issue Jun 1, 2022 · 14 comments
Closed

runtime: stack split at bad time during fuzzing #53190

catenacyber opened this issue Jun 1, 2022 · 14 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@catenacyber
Copy link
Contributor

catenacyber commented Jun 1, 2022

What version of Go are you using (go version)?

$ go version
go version devel go1.19-7ec6ef432a Fri May 20 21:32:57 2022 +0000 linux/amd64

Does this issue reproduce with the latest release?

Everything works fine with go 1.18 and go1.19 at commit e66f895

cc @kyakdan as this was introduced by your fix of #51318

It breaks also with latest commit 8a56c77

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/root/.go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/root/.go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.19-e66f895667 Fri May 20 21:03:28 2022 +0000"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build131801594=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I build ngolo-fuzzing project on oss-fuzz and run a fuzzer
cf google/oss-fuzz#7792
Let me know if you need more details

What did you expect to see?

Fuzzer running normally like

 /out/fuzz_ng_bytes 
INFO: found LLVMFuzzerCustomMutator (0x5b5e20). Disabling -len_control by default.
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 860763079
INFO: Loaded 1 modules   (15 inline 8-bit counters): 15 [0xc4b7f8, 0xc4b807), 
INFO: Loaded 1 PC tables (15 PCs): 15 [0x9ece60,0x9ecf50), 
INFO: 23479 Extra Counters
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
#2	INITED cov: 3 ft: 3 corp: 1/1b exec/s: 0 rss: 36Mb
#3	NEW    cov: 6 ft: 729 corp: 2/10b lim: 4096 exec/s: 0 rss: 40Mb L: 9/9 MS: 1 CustomCrossOver-
#4	NEW    cov: 6 ft: 753 corp: 3/19b lim: 4096 exec/s: 0 rss: 40Mb L: 9/9 MS: 1 CustomCrossOver-
#5	NEW    cov: 6 ft: 1129 corp: 4/43b lim: 4096 exec/s: 0 rss: 40Mb L: 24/24 MS: 1 CustomCrossOver-
#6	NEW    cov: 6 ft: 1153 corp: 5/80b lim: 4096 exec/s: 0 rss: 41Mb L: 37/37 MS: 1 CustomCrossOver-
#7	NEW    cov: 6 ft: 1315 corp: 6/125b lim: 4096 exec/s: 0 rss: 41Mb L: 45/45 MS: 2 InsertByte-Custom-
#8	NEW    cov: 6 ft: 1332 corp: 7/152b lim: 4096 exec/s: 0 rss: 41Mb L: 27/45 MS: 1 CustomCrossOver-

What did you see instead?

fatal error: runtime: stack split at bad time

Complete log is

/out/fuzz_ng_bytes 
INFO: found LLVMFuzzerCustomMutator (0x5b5e20). Disabling -len_control by default.
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1006414716
runtime: newstack at runtime.libfuzzerTraceConstCmp1+0x7d sp=0x10c00012b6b0 stack=[0x10c00012a000, 0x10c00012c000]
	morebuf={pc:0x5ba329 sp:0x10c00012b6b8 lr:0x0}
	sched={pc:0x5c2c5d sp:0x10c00012b6b0 lr:0x0 ctxt:0x0}
syscall.RawSyscall6(0x0?, 0x0?, 0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
	runtime/internal/syscall/syscall_linux.go:38 +0x49 fp=0x10c00012b738 sp=0x10c00012b6b8 pc=0x5ba329
syscall.Syscall(0x0?, 0x0?, 0x0?, 0x5c2c32?)
	syscall/syscall_linux.go:81 +0x48 fp=0x10c00012b7a8 sp=0x10c00012b738 pc=0x635988
internal/syscall/unix.IsNonblock(0x0?)
	internal/syscall/unix/nonblocking.go:16 +0x68 fp=0x10c00012b7f8 sp=0x10c00012b7a8 pc=0x6f3c88
os.NewFile(0x6b8711?, {0xa989ed, 0xa})
	os/file_unix.go:103 +0x67 fp=0x10c00012b838 sp=0x10c00012b7f8 pc=0x717967
os.init()
	os/file.go:65 +0x213 fp=0x10c00012b860 sp=0x10c00012b838 pc=0x71bbd3
runtime.doInit(0xd68060)
	runtime/proc.go:6303 +0x128 fp=0x10c00012b990 sp=0x10c00012b860 pc=0x5fc348
runtime.doInit(0xd673c0)
	runtime/proc.go:6280 +0x71 fp=0x10c00012bac0 sp=0x10c00012b990 pc=0x5fc291
runtime.doInit(0xd65d20)
	runtime/proc.go:6280 +0x71 fp=0x10c00012bbf0 sp=0x10c00012bac0 pc=0x5fc291
runtime.doInit(0xd67fc0)
	runtime/proc.go:6280 +0x71 fp=0x10c00012bd20 sp=0x10c00012bbf0 pc=0x5fc291
runtime.doInit(0xd681a0)
	runtime/proc.go:6280 +0x71 fp=0x10c00012be50 sp=0x10c00012bd20 pc=0x5fc291
runtime.doInit(0xd65820)
	runtime/proc.go:6280 +0x71 fp=0x10c00012bf80 sp=0x10c00012be50 pc=0x5fc291
runtime.main()
	runtime/proc.go:233 +0x1d4 fp=0x10c00012bfe0 sp=0x10c00012bf80 pc=0x5ef254
runtime.goexit()
	runtime/asm_amd64.s:1594 +0x1 fp=0x10c00012bfe8 sp=0x10c00012bfe0 pc=0x61a161
fatal error: runtime: stack split at bad time

runtime stack:
runtime.throw({0xa9ee83?, 0xd6ad60?})
	runtime/panic.go:1047 +0x5f fp=0x7fab341fec20 sp=0x7fab341febf0 pc=0x5eca3f
runtime.newstack()
	runtime/stack.go:995 +0xa56 fp=0x7fab341fedd8 sp=0x7fab341fec20 pc=0x604516
runtime.morestack()
	runtime/asm_amd64.s:570 +0x80 fp=0x7fab341fede0 sp=0x7fab341fedd8 pc=0x617fe0

goroutine 1 [syscall, locked to thread]:
syscall.RawSyscall6(0x0?, 0x0?, 0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
	runtime/internal/syscall/syscall_linux.go:38 +0x49 fp=0x10c00012b738 sp=0x10c00012b6b8 pc=0x5ba329
syscall.Syscall(0x0?, 0x0?, 0x0?, 0x5c2c32?)
	syscall/syscall_linux.go:81 +0x48 fp=0x10c00012b7a8 sp=0x10c00012b738 pc=0x635988
internal/syscall/unix.IsNonblock(0x0?)
	internal/syscall/unix/nonblocking.go:16 +0x68 fp=0x10c00012b7f8 sp=0x10c00012b7a8 pc=0x6f3c88
os.NewFile(0x6b8711?, {0xa989ed, 0xa})
	os/file_unix.go:103 +0x67 fp=0x10c00012b838 sp=0x10c00012b7f8 pc=0x717967
os.init()
	os/file.go:65 +0x213 fp=0x10c00012b860 sp=0x10c00012b838 pc=0x71bbd3
runtime.doInit(0xd68060)
	runtime/proc.go:6303 +0x128 fp=0x10c00012b990 sp=0x10c00012b860 pc=0x5fc348
runtime.doInit(0xd673c0)
	runtime/proc.go:6280 +0x71 fp=0x10c00012bac0 sp=0x10c00012b990 pc=0x5fc291
runtime.doInit(0xd65d20)
	runtime/proc.go:6280 +0x71 fp=0x10c00012bbf0 sp=0x10c00012bac0 pc=0x5fc291
runtime.doInit(0xd67fc0)
	runtime/proc.go:6280 +0x71 fp=0x10c00012bd20 sp=0x10c00012bbf0 pc=0x5fc291
runtime.doInit(0xd681a0)
	runtime/proc.go:6280 +0x71 fp=0x10c00012be50 sp=0x10c00012bd20 pc=0x5fc291
runtime.doInit(0xd65820)
	runtime/proc.go:6280 +0x71 fp=0x10c00012bf80 sp=0x10c00012be50 pc=0x5fc291
runtime.main()
	runtime/proc.go:233 +0x1d4 fp=0x10c00012bfe0 sp=0x10c00012bf80 pc=0x5ef254
runtime.goexit()
	runtime/asm_amd64.s:1594 +0x1 fp=0x10c00012bfe8 sp=0x10c00012bfe0 pc=0x61a161

goroutine 2 [force gc (idle)]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
	runtime/proc.go:363 +0xd6 fp=0x10c000062fb0 sp=0x10c000062f90 pc=0x5ef656
runtime.goparkunlock(...)
	runtime/proc.go:369
runtime.forcegchelper()
	runtime/proc.go:302 +0xad fp=0x10c000062fe0 sp=0x10c000062fb0 pc=0x5ef4ed
runtime.goexit()
	runtime/asm_amd64.s:1594 +0x1 fp=0x10c000062fe8 sp=0x10c000062fe0 pc=0x61a161
created by runtime.init.7
	runtime/proc.go:290 +0x25

goroutine 18 [GC sweep wait]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
	runtime/proc.go:363 +0xd6 fp=0x10c00005e790 sp=0x10c00005e770 pc=0x5ef656
runtime.goparkunlock(...)
	runtime/proc.go:369
runtime.bgsweep(0x0?)
	runtime/mgcsweep.go:278 +0x8e fp=0x10c00005e7c8 sp=0x10c00005e790 pc=0x5dc04e
runtime.gcenable.func1()
	runtime/mgc.go:178 +0x26 fp=0x10c00005e7e0 sp=0x10c00005e7c8 pc=0x5d1186
runtime.goexit()
	runtime/asm_amd64.s:1594 +0x1 fp=0x10c00005e7e8 sp=0x10c00005e7e0 pc=0x61a161
created by runtime.gcenable
	runtime/mgc.go:178 +0x6b

goroutine 19 [GC scavenge wait]:
runtime.gopark(0x10c00010e000?, 0xae0630?, 0x1?, 0x0?, 0x0?)
	runtime/proc.go:363 +0xd6 fp=0x10c00005ef70 sp=0x10c00005ef50 pc=0x5ef656
runtime.goparkunlock(...)
	runtime/proc.go:369
runtime.(*scavengerState).park(0x17771c0)
	runtime/mgcscavenge.go:389 +0x53 fp=0x10c00005efa0 sp=0x10c00005ef70 pc=0x5da133
runtime.bgscavenge(0x0?)
	runtime/mgcscavenge.go:617 +0x45 fp=0x10c00005efc8 sp=0x10c00005efa0 pc=0x5da705
runtime.gcenable.func2()
	runtime/mgc.go:179 +0x26 fp=0x10c00005efe0 sp=0x10c00005efc8 pc=0x5d1126
runtime.goexit()
	runtime/asm_amd64.s:1594 +0x1 fp=0x10c00005efe8 sp=0x10c00005efe0 pc=0x61a161
created by runtime.gcenable
	runtime/mgc.go:179 +0xaa
Aborted

cc @ianlancetaylor as you have been fixing bugs found by ngolo-fuzzing

cc @kyakdan as this was introduced by your fix of #51318

@catenacyber
Copy link
Contributor Author

catenacyber commented Jun 1, 2022

hint: ngolo-fuzzing uses libprotobufmutator

@catenacyber
Copy link
Contributor Author

catenacyber commented Jun 1, 2022

I have no idea what it is doing but here is a one line fix :

diff --git a/src/runtime/libfuzzer.go b/src/runtime/libfuzzer.go
index 920ac575f5..2857519275 100644
--- a/src/runtime/libfuzzer.go
+++ b/src/runtime/libfuzzer.go
@@ -27,6 +27,7 @@ func libfuzzerTraceCmp8(arg0, arg1 uint64) {
        libfuzzerCall(&__sanitizer_cov_trace_cmp8, uintptr(arg0), uintptr(arg1))
 }
 
+//go:nosplit
 func libfuzzerTraceConstCmp1(arg0, arg1 uint8) {
        libfuzzerCall(&__sanitizer_cov_trace_const_cmp1, uintptr(arg0), uintptr(arg1))
 }

@catenacyber
Copy link
Contributor Author

catenacyber commented Jun 1, 2022

@kyakdan
Copy link
Contributor

kyakdan commented Jun 1, 2022

@catenacyber Thanks for reporting the issue and a potential fix. I'll have a look.

@prattmic
Copy link
Member

prattmic commented Jun 1, 2022

cc @randall77

@gopherbot
Copy link

gopherbot commented Jun 1, 2022

Change https://go.dev/cl/410034 mentions this issue: runtime: fix stack split at bad time when fuzzing

@kyakdan
Copy link
Contributor

kyakdan commented Jun 1, 2022

I've just created a pull request with a fix: https://go-review.googlesource.com/c/go/+/410034

@catenacyber
Copy link
Contributor Author

catenacyber commented Jun 2, 2022

Thanks Khaled

@dmitshur dmitshur changed the title go command: fatal error: runtime: stack split at bad time runtime: stack split at bad time during fuzzing Jun 3, 2022
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 3, 2022
@dmitshur dmitshur added this to the Go1.19 milestone Jun 3, 2022
catenacyber added a commit to catenacyber/oss-fuzz that referenced this issue Jun 10, 2022
@kyakdan
Copy link
Contributor

kyakdan commented Jun 12, 2022

@catenacyber The fix is now merged into master and you can give it another try.

@catenacyber
Copy link
Contributor Author

catenacyber commented Jun 13, 2022

Thanks Khaled, google/oss-fuzz#7831 should remove the workaround.

But you can see that this workaround was only applied if your patch was not applied yet, so everything looks good :-)

catenacyber added a commit to catenacyber/oss-fuzz that referenced this issue Jun 15, 2022
@prattmic
Copy link
Member

prattmic commented Jun 24, 2022

I dug into this a bit. The libfuzzerTraceConstCmp1 call came from instrumentation in runtime/internal/syscall.syscall_RawSyscall6.

Though adding //go:nosplit has fixed the immediate issue, I don't think runtime/internal/syscall should be getting instrumented at all (runtime is excluded, and runtime/internal/syscall is effectively part of runtime).

@gopherbot
Copy link

gopherbot commented Jun 24, 2022

Change https://go.dev/cl/413818 mentions this issue: cmd/go: compile runtime/internal/syscall as a runtime package

@kyakdan
Copy link
Contributor

kyakdan commented Jun 24, 2022

@prattmic You are right. We don't need to instrument this package. However, the hooks should be marked as nosplit. The reason is that the compiler inserts them into the code to be instrumented which means they may be inserted inside functions that are marked as nosplit.

kyakdan added a commit to CodeIntelligenceTesting/go that referenced this issue Jun 24, 2022
These functions can be inserted by the compiler into the code to be
instrumented. This may result in these functions having callers that
are nosplit. That is why they must be nosplit.
This is a followup for CL 410034 in order to fix golang#53190.
@gopherbot
Copy link

gopherbot commented Jun 25, 2022

Change https://go.dev/cl/413978 mentions this issue: runtime: mark string comparison hooks as no split

gopherbot pushed a commit that referenced this issue Jun 25, 2022
These functions can be inserted by the compiler into the code to be
instrumented. This may result in these functions having callers that
are nosplit. That is why they must be nosplit.

This is a followup for CL 410034 in order to fix #53190.

Change-Id: I03746208a2a302a581a1eaad6c9d0672bb1e949a
GitHub-Last-Rev: 6506d86
GitHub-Pull-Request: #53544
Reviewed-on: https://go-review.googlesource.com/c/go/+/413978
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
gopherbot pushed a commit that referenced this issue Jun 27, 2022
runtime/internal/syscall is a runtime package, so it should be built
with -+.

Specifically, we don't want libfuzzer instrumentation in Go functions
defined in runtime/internal/syscall, which is disabled with -+.

For #53190.

Change-Id: I9f16f5c7c7ce10b98371e9de82fcea6da854e163
Reviewed-on: https://go-review.googlesource.com/c/go/+/413818
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
catenacyber added a commit to catenacyber/oss-fuzz that referenced this issue Jun 27, 2022
catenacyber added a commit to catenacyber/oss-fuzz that referenced this issue Jun 28, 2022
jonathanmetzman pushed a commit to google/oss-fuzz that referenced this issue Jul 7, 2022
* infra: have timeout per fuzz target for coverage

As is done for other languages

* ngolo-fuzzing: remove temporary workaround

now that golang/go#53190 is closed

* ngolo-fuzzing: use built go toolchain in its directory

without copying it to /root/.go/

in order to get coverage for std lib in the end

* infra: ability to get coverage for additional golang package

And uses it with ngolo-fuzzing :
ngolo-fuzzing fuzz targets live in a different repository than
the code being fuzzed, and we we want to get the coverage, for
both the fuzz target and the package being fuzzed

* fixup bash unbound

* fixup ngolo-fuzzing only match at beginning for std package

* stricter check for every additional go package
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
These functions can be inserted by the compiler into the code to be
instrumented. This may result in these functions having callers that
are nosplit. That is why they must be nosplit.

This is a followup for CL 410034 in order to fix golang#53190.

Change-Id: I03746208a2a302a581a1eaad6c9d0672bb1e949a
GitHub-Last-Rev: 6506d86
GitHub-Pull-Request: golang#53544
Reviewed-on: https://go-review.googlesource.com/c/go/+/413978
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
runtime/internal/syscall is a runtime package, so it should be built
with -+.

Specifically, we don't want libfuzzer instrumentation in Go functions
defined in runtime/internal/syscall, which is disabled with -+.

For golang#53190.

Change-Id: I9f16f5c7c7ce10b98371e9de82fcea6da854e163
Reviewed-on: https://go-review.googlesource.com/c/go/+/413818
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
MartinPetkov pushed a commit to MartinPetkov/oss-fuzz that referenced this issue Aug 15, 2022
* infra: have timeout per fuzz target for coverage

As is done for other languages

* ngolo-fuzzing: remove temporary workaround

now that golang/go#53190 is closed

* ngolo-fuzzing: use built go toolchain in its directory

without copying it to /root/.go/

in order to get coverage for std lib in the end

* infra: ability to get coverage for additional golang package

And uses it with ngolo-fuzzing :
ngolo-fuzzing fuzz targets live in a different repository than
the code being fuzzed, and we we want to get the coverage, for
both the fuzz target and the package being fuzzed

* fixup bash unbound

* fixup ngolo-fuzzing only match at beginning for std package

* stricter check for every additional go package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
5 participants