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: Buffer overflow trying to use C.malloc #69098

Closed
IoIxD opened this issue Aug 28, 2024 · 8 comments
Closed

cmd/cgo: Buffer overflow trying to use C.malloc #69098

IoIxD opened this issue Aug 28, 2024 · 8 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@IoIxD
Copy link

IoIxD commented Aug 28, 2024

Go version

go version go1.22.6 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/gavin/.cache/go-build'
GOENV='/home/gavin/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/gavin/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/gavin/go'
GOPRIVATE=''
GOPROXY='direct'
GOROOT='/usr/lib/golang'
GOSUMDB='off'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.6'
GCCGO='/usr/bin/gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3841798468=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Tried to copy the contents of a Golang pointer into a C struct that has a C.void in it.

/*
typedef struct TransparentStruct {
	void * inner;
} TransparentStruct;

typedef TransparentStruct ResponseWriter;
*/
import "C"

func ResponseWriterIntoRaw(raw http.ResponseWriter) C.ResponseWriter {
	fmt.Printf("ResponseWriter size: %v\n", unsafe.Sizeof(raw))
	ptr := (*http.ResponseWriter)(C.malloc(C.ulong(unsafe.Sizeof(raw))))
	*ptr = raw
	return C.ResponseWriter{unsafe.Pointer(ptr)}
}

What did you see happen?

Before I could even see my code fail in any other way, I saw C.malloc throwing an error that implies a buffer overflow, even though it's 16 bytes.

ResponseWriter size: 16
malloc(): corrupted top size
[1]    277712 IOT instruction (core dumped)  cargo run

This only happens 50% of the time, the other 50% I see unexpected fault address 0x0 which is an error I was getting before I was told that you need to use malloc to pass Go types to C.

What did you expect to see?

I honestly would be amazed if I didn't encounter another memory bug after this but I at least expected malloc to be able to function and not complain about a buffer overflow.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 28, 2024
@randall77
Copy link
Contributor

You're violating quite a few unsafe.Pointer rules here. Particularly, casting C-allocated memory to a Go type that contains pointers, and particularly an interface, is very bad. The write barriers implicit in the *ptr = raw statement will be very confused.

Not sure why malloc itself is failing though. All the invalid code I see is post-malloc. A complete reproducible example would help.

I was told that you need to use malloc to pass Go types to C

You can do this for pointer-free types. You cannot do it for types containing pointers (which includes interfaces).

A better strategy is to use the new runtime.Pinner api. Then you do not need to call malloc at all.

@IoIxD
Copy link
Author

IoIxD commented Aug 28, 2024

I figured all that, but yeah, malloc itself failkng is weird.
Does it not just call gcc's malloc?

@randall77
Copy link
Contributor

Does it not just call gcc's malloc?

It calls libc's malloc, yes.
That error (corrupted top size) makes it sound like the malloc heap itself got corrupted somehow. Does this error occur on the first call to your ResponseWriterIntoRaw function, or a subsequent one? I could believe it might happen on a second or subsequent call, as the code at the end of the call would put you squarely in undefined behavior territory.

@IoIxD
Copy link
Author

IoIxD commented Aug 28, 2024

Does this error occur on the first call to your ResponseWriterIntoRaw function, or a subsequent one?

First call.

@w3nl1ng
Copy link

w3nl1ng commented Aug 28, 2024

It looks like C.malloc finally reached the code below
When the size of top chunk exceeds system_mem, this error will occur
I suggest you debug to this place to see if av(arena) is initialized correctly

      victim = av->top;
      size = chunksize (victim);

      if (__glibc_unlikely (size > av->system_mem))
        malloc_printerr ("malloc(): corrupted top size");

@ianlancetaylor
Copy link
Member

We can't do anything here without a complete, standalone, example that demonstrates the problem.

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 28, 2024
@IoIxD
Copy link
Author

IoIxD commented Aug 28, 2024

I actually can't.
I suppose then, the comment about the heap being corrupted is right.

A better strategy is to use the new runtime.Pinner api

Which seems to be poorly documented, and also I'm still getting that fault address error.
Am I not using this right?

func ResponseIntoRaw(raw *http.Response) C.Response {
	pinner := new(runtime.Pinner)
	pinner.Pin(raw)
	return C.Response{(unsafe.Pointer)(raw)}
}
func ResponseWriterIntoRaw(raw http.ResponseWriter) C.ResponseWriter {
	pinner := new(runtime.Pinner)
	pinner.Pin(raw)
	return C.ResponseWriter{(unsafe.Pointer)(&raw)}
}

@IoIxD IoIxD closed this as completed Aug 28, 2024
@ianlancetaylor
Copy link
Member

As the runtime.Pinner docs say, "If the pinned object itself contains pointers to Go objects, these objects must be pinned separately if they are going to be accessed from C code." Just pinning the top level is not enough.

I suggest that you bring your actual problem to a forum. You will get better and faster answers there than you will on the issue tracker. See https://go.dev/wiki/Questions. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

6 participants
@ianlancetaylor @randall77 @gopherbot @IoIxD @w3nl1ng and others