spacemonkeygo / openssl

OpenSSL bindings for Go
http://godoc.org/github.com/spacemonkeygo/openssl
Apache License 2.0
474 stars 237 forks source link

Crash when freeing Certificates #27

Closed isaldana closed 7 years ago

isaldana commented 9 years ago

I created multiple *openssl.Ctx. When one goes out of scope, the finalizer tries to free a bunch of stuff including the *openssl.Certificate associated with that context (https://github.com/spacemonkeygo/openssl/blob/master/cert.go#L326). My program keeps crashing and it seems like the finalizer is being called twice. The workaround is to never let the openssl.Ctx go out of scope. The following is the relevant crash error:

main(97262,0xb0104000) malloc: *** error for object 0x5216a20: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
SIGABRT: abort
PC=0x7fff8da09286
signal arrived during cgo execution

goroutine 19 [syscall]:
runtime.cgocall(0x4002560, 0x46f4f10)
    /usr/local/Cellar/go/1.3.1/libexec/src/pkg/runtime/cgocall.c:143 +0xe5 fp=0x46f4ef8 sp=0x46f4eb0
github.com/spacemonkeygo/openssl._Cfunc_X509_free(0x52152d0)
    github.com/spacemonkeygo/openssl/_obj/_cgo_defun.c:1460 +0x31 fp=0x46f4f10 sp=0x46f4ef8
github.com/spacemonkeygo/openssl.func·007(0xc208085980)
    /Users/i/proj/src/github.com/spacemonkeygo/openssl/cert.go:327 +0x2a fp=0x46f4f20 sp=0x46f4f10
runtime.call16(0x446ade8, 0xc2080003d0, 0x1000000010)
    /usr/local/Cellar/go/1.3.1/libexec/src/pkg/runtime/asm_amd64.s:360 +0x32 fp=0x46f4f38 sp=0x46f4f20
runfinq()
    /usr/local/Cellar/go/1.3.1/libexec/src/pkg/runtime/mgc0.c:2682 +0x207 fp=0x46f4fa8 sp=0x46f4f38
runtime.goexit()
    /usr/local/Cellar/go/1.3.1/libexec/src/pkg/runtime/proc.c:1445 fp=0x46f4fb0 sp=0x46f4fa8
created by runtime.gc
    /usr/local/Cellar/go/1.3.1/libexec/src/pkg/runtime/mgc0.c:2268
lunixbochs commented 9 years ago

OpenSSL is probably calling X509_free() internally on the cert when it frees the context, so we need to adjust our GC strategy so that doesn't happen twice.

(See https://github.com/spacemonkeygo/openssl/blob/master/ctx.go#L125)

It also needs to be resilient to changing the certificate of a context multiple times without leaking or crashing.

This is basically the problem I'm talking about in #19

zeebo commented 7 years ago

This might be fixed by now. Closing due to inactivity.