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

cmd/cgo: Writing to C pointer causes Go heap corruption #26288

Closed
chfast opened this issue Jul 9, 2018 · 8 comments
Closed

cmd/cgo: Writing to C pointer causes Go heap corruption #26288

chfast opened this issue Jul 9, 2018 · 8 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@chfast
Copy link

chfast commented Jul 9, 2018

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

go version go1.10.1 linux/amd64

Does this issue reproduce with the latest release?

I have not checked 1.10.3 nor 1.11 beta.

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/chfast/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/chfast/go"
GORACE=""
GOROOT="/usr/lib/go-1.10"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.10/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build613522550=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I have an Go exported function that takes a pointer to a C struct as an argument. It is so-called output
argument, so the Go function is writing data to this C struct via the provided pointer.
Depending how exactly this is done the Go heap might get corrupted.
I have not minimized the example fully.

The C struct is:

type _Ctype_struct_evmc_result struct {
	status_code	int32
	_		[4]byte
	gas_left	_Ctype_int64_t
	output_data	*_Ctype_uint8_t
	output_size	_Ctype_size_t
	release		_Ctype_evmc_release_result_fn
	create_address	_Ctype_struct_evmc_address
	padding		[4]_Ctype_uint8_t
}

The variant of the exported function working correctly:

//export call
func call(pResult *C.struct_evmc_result, pCtx unsafe.Pointer, msg *C.struct_evmc_message) {
	result := C.struct_evmc_result{}
	result.status_code = C.EVMC_FAILURE
	*pResult = result
}

Go assembly:

"".call STEXT size=158 args=0x18 locals=0x60
TEXT	"".call(SB), $96-24
MOVQ	(TLS), CX
CMPQ	SP, 16(CX)
JLS	148
SUBQ	$96, SP
MOVQ	BP, 88(SP)
LEAQ	88(SP), BP
FUNCDATA	$0, gclocals·e6397a44f8e1b6e77d0f200b4fba5269(SB)
FUNCDATA	$1, gclocals·6c80e7c7d827a5ab4457c7bd9e35d7d4(SB)
XORPS	X0, X0
MOVUPS	X0, "".result+24(SP)
MOVUPS	X0, "".result+40(SP)
MOVUPS	X0, "".result+56(SP)
MOVUPS	X0, "".result+72(SP)
MOVL	$1, "".result+24(SP)
MOVQ	"".pResult+104(SP), DI
TESTB	AL, (DI)
MOVL	runtime.writeBarrier(SB), AX
TESTL	AX, AX
JNE	115
LEAQ	"".result+24(SP), SI
DUFFCOPY	$840
MOVQ	88(SP), BP
ADDQ	$96, SP
RET
LEAQ	type."".C.struct_evmc_result(SB), AX
MOVQ	AX, (SP)
MOVQ	DI, 8(SP)
LEAQ	"".result+24(SP), AX
MOVQ	AX, 16(SP)
PCDATA	$0, $1
CALL	runtime.typedmemmove(SB)
JMP	105
NOP
PCDATA	$0, $-1
CALL	runtime.morestack_noctxt(SB)
JMP	0

The variant of the exported function causing the following Go heap corruption:

runtime: pointer 0xc420cc3000 to unallocated span idx=0x661 span.base()=0xc420cbe000 span.limit=0xc420cc6000 span.state=3
fatal error: found bad pointer in Go heap (incorrect use of unsafe or cgo?)

runtime stack:
runtime.throw(0x89ff13, 0x3e)
	/usr/lib/go-1.10/src/runtime/panic.go:616 +0x81 fp=0x7fbf767fb748 sp=0x7fbf767fb728 pc=0x42dca1
runtime.heapBitsForObject(0xc420cc3000, 0x0, 0x0, 0xc4202a8d40, 0xc41ffeab95, 0x7fbf00000000, 0x7fbf90765ce8, 0x35)
	/usr/lib/go-1.10/src/runtime/mbitmap.go:425 +0x473 fp=0x7fbf767fb7a0 sp=0x7fbf767fb748 pc=0x415223
runtime.wbBufFlush1(0xc420038500)
	/usr/lib/go-1.10/src/runtime/mwbbuf.go:234 +0xde fp=0x7fbf767fb820 sp=0x7fbf767fb7a0 pc=0x429bce
runtime.wbBufFlush.func1()
	/usr/lib/go-1.10/src/runtime/mwbbuf.go:180 +0x3a fp=0x7fbf767fb838 sp=0x7fbf767fb820 pc=0x45811a
runtime.systemstack(0x7fbf767fbe80)
	/usr/lib/go-1.10/src/runtime/asm_amd64.s:409 +0x79 fp=0x7fbf767fb840 sp=0x7fbf767fb838 pc=0x459b89
runtime.mstart()
	/usr/lib/go-1.10/src/runtime/proc.go:1175 fp=0x7fbf767fb848 sp=0x7fbf767fb840 pc=0x4322e0

goroutine 3888 [running, locked to thread]:
runtime.systemstack_switch()
	/usr/lib/go-1.10/src/runtime/asm_amd64.s:363 fp=0xc420afae50 sp=0xc420afae48 pc=0x459b00
runtime.wbBufFlush(0xc420a6ec28, 0xc4202a8d40)
	/usr/lib/go-1.10/src/runtime/mwbbuf.go:179 +0x4e fp=0xc420afae70 sp=0xc420afae50 pc=0x429a5e
runtime.gcWriteBarrier(0xc420afaf18, 0x20, 0xc420afaf38, 0x459eee, 0x8a6750, 0xc420b1a510, 0x7fbf767fbb90, 0x7fbf767fbc30, 0xc420afafb8, 0x404687, ...)
	/usr/lib/go-1.10/src/runtime/asm_amd64.s:2442 +0xb4 fp=0xc420afaef8 sp=0xc420afae70 pc=0x45c734
runtime.deferreturn(0xc420afaf8f)
	/usr/lib/go-1.10/src/runtime/panic.go:343 +0xb3 fp=0xc420afaf48 sp=0xc420afaef8 pc=0x42cee3
runtime.cgocallbackg1(0x0)
	/usr/lib/go-1.10/src/runtime/cgocall.go:332 +0x1a7 fp=0xc420afafc8 sp=0xc420afaf48 pc=0x404687
runtime.cgocallbackg(0x0)
	/usr/lib/go-1.10/src/runtime/cgocall.go:194 +0xda fp=0xc420afb030 sp=0xc420afafc8 pc=0x40444a
runtime.cgocallback_gofunc(0x4042f8, 0x7902a0, 0xc420afb0e0, 0x7a3843)
	/usr/lib/go-1.10/src/runtime/asm_amd64.s:826 +0x9b fp=0xc420afb050 sp=0xc420afb030 pc=0x45b49b
runtime.asmcgocall(0x7902a0, 0xc420afb0e0)
	/usr/lib/go-1.10/src/runtime/asm_amd64.s:673 +0x42 fp=0xc420afb058 sp=0xc420afb050 pc=0x45b332
runtime.cgocall(0x7902a0, 0xc420afb0e0, 0xc420af0001)
	/usr/lib/go-1.10/src/runtime/cgocall.go:131 +0x88 fp=0xc420afb090 sp=0xc420afb058 pc=0x4042f8
github.com/ethereum/go-ethereum/core/vm._Cfunc_execute(0x7fbf8c2c93a0, 0x896, 0xc400000001, 0xc420b1a510, 0x7fbf6c003560, 0x84, 0x0, 0x0, 0x0, 0x0, ...)
//export call
func call(pResult *C.struct_evmc_result, pCtx unsafe.Pointer, msg *C.struct_evmc_message) {
	pResult.status_code = C.EVMC_FAILURE
	pResult.output_size = 0
	pResult.release = nil
}

Go assembly:

"".call STEXT size=93 args=0x18 locals=0x8
TEXT	"".call(SB), $8-24
MOVQ	(TLS), CX
CMPQ	SP, 16(CX)
JLS	86
SUBQ	$8, SP
MOVQ	BP, (SP)
LEAQ	(SP), BP
FUNCDATA	$0, gclocals·d4dc2f11db048877dbc0f60a22b4adb3(SB)
FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
MOVQ	"".pResult+16(SP), CX
MOVL	$1, (CX)
MOVQ	$0, 24(CX)
MOVL	runtime.writeBarrier(SB), DX
LEAQ	32(CX), DI
TESTL	DX, DX
JNE	77
MOVQ	$0, 32(CX)
MOVQ	(SP), BP
ADDQ	$8, SP
RET
XORL	AX, AX
CALL	runtime.gcWriteBarrier(SB)
JMP	68
NOP
PCDATA	$0, $-1
CALL	runtime.morestack_noctxt(SB)
JMP	0
@randall77
Copy link
Contributor

"Heap corruption" isn't really the right term here, or at least that's not a conclusion I can draw yet. Go is finding a pointer in its heap that is bad somehow (not a valid Go or C pointer). Presumably it's the result of evmcAddress.

So where does the result of evmcAddress come from? It looks like a pointer into the Go heap. Does your program use package unsafe to compute that address? Is it computed in C from a Go pointer?

The reason I ask is that the bad pointer is to the very start of an object span. That makes me think the pointer points just past the end of the last object in the previous span. That can happen if you do pointer arithmetic with unsafe or in C (which allows such pointers).

@ianlancetaylor
Copy link
Contributor

Go code is never permitted to store a Go pointer into memory allocated by C. See https://golang.org/cmd/cgo/#hdr-Passing_pointers . Try running your program with the environment variable GODEBUG=cgocheck=2. I would expect that you will get an error.

@ianlancetaylor ianlancetaylor added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 9, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.12 milestone Jul 9, 2018
@chfast
Copy link
Author

chfast commented Jul 10, 2018

So where does the result of evmcAddress come from? It looks like a pointer into the Go heap. Does your program use package unsafe to compute that address? Is it computed in C from a Go pointer?

Sorry for the distraction with evmcAddress() function. It was copying the [20]byte byte-by-byte to a C struct. But it is not needed to reproduce the problem. I updated the description with a smaller example.

Go code is never permitted to store a Go pointer into memory allocated by C.

I know that and really try not to. I even started copying byte arrays before passing to C. The GODEBUG=cgocheck=2 does not report anything.

I discovered one thing. If I modify the function by adding pResult.create_address = C.struct_evmc_address{}

//export call
func call(pResult *C.struct_evmc_result, pCtx unsafe.Pointer, msg *C.struct_evmc_message) {
	pResult.create_address = C.struct_evmc_address{}
	pResult.status_code = C.EVMC_FAILURE
	pResult.output_size = 0
	pResult.release = nil
}

I sometimes get different panic error:

runtime: nelems=128 nalloc=107 previous allocCount=106 nfreed=65535
fatal error: sweep increased allocation count

runtime stack:
runtime.throw(0x8953ab, 0x20)
	/usr/lib/go-1.10/src/runtime/panic.go:616 +0x81
runtime.(*mspan).sweep(0x7f2b5c33a708, 0x7f2b5c33a701, 0x7ffe3ae0b600)
	/usr/lib/go-1.10/src/runtime/mgcsweep.go:298 +0x89c
runtime.(*mcentral).cacheSpan(0xd2c870, 0x7f2b4274cf68)
	/usr/lib/go-1.10/src/runtime/mcentral.go:58 +0x277
runtime.(*mcache).refill(0x7f2b5c376000, 0xd1b80a)
	/usr/lib/go-1.10/src/runtime/mcache.go:123 +0x9c
runtime.(*mcache).nextFree.func1()
	/usr/lib/go-1.10/src/runtime/malloc.go:556 +0x32
runtime.systemstack(0x0)
	/usr/lib/go-1.10/src/runtime/asm_amd64.s:409 +0x79
runtime.mstart()
	/usr/lib/go-1.10/src/runtime/proc.go:1175

@ianlancetaylor
Copy link
Contributor

Can you show us a complete program that we can use to recreate the problem ourselves?

@chfast
Copy link
Author

chfast commented Jul 11, 2018

Can you show us a complete program that we can use to recreate the problem ourselves?

The program is too big probably for that, but the commit that fixed the issue is here: ethereum/go-ethereum@8a99766.

I can provide more detailed instructions how to run it if you want to.

@ianlancetaylor
Copy link
Contributor

Thanks. Is it possible that the C memory passed to the exported call function is uninitialized? Perhaps this is another instance of #19928.

@chfast
Copy link
Author

chfast commented Jul 11, 2018

Yes, it's uninitiated struct allocated on C stack.

@chfast
Copy link
Author

chfast commented Jul 11, 2018

Duplicate of #19928.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants