Skip to content

Possible memory leak: NativeLog allocates C strings on every log line and never frees them (android and darwin) #137

Open

Description

@OvOhao

Description

pkg/logutil sends every zap entry handled by the native core (logger_native.go, Write -> NativeLog) to the platform logger through cgo. Both platform implementations pass C.CString(...) results directly as call arguments. Those results are never bound to a name, so they are never passed to C.free.

pkg/logutil/logger_native_android.go:31:

C.__android_log_write(level, C.CString(namespace), C.CString(fmt.Sprintf("[%s] %s", logLevel.CapitalString(), message)))

pkg/logutil/logger_native_darwin.go:52-54:

log := C.os_log_create(C.CString(namespace), C.CString(namespace))

C.os_log_wrapper(level, log, C.CString(fmt.Sprintf("[%s] %s", logLevel.CapitalString(), message)))

C.CString allocates with malloc, and the caller owns the result. None of the callees keep these buffers:

  • __android_log_write writes tag and message synchronously through an iovec to logd (AOSP liblog/logger_write.cpp).
  • os_log_create copies subsystem and category into the runtime's own collection.
  • The os_log_* macros expand to a synchronous _os_log_impl call (os_log_with_type in os/log.h), so the string argument is consumed before the macro returns and is not retained.

So every log line leaks two C strings on Android (namespace and message) and three on darwin/iOS (namespace twice, plus the message). These allocations are never released.

Commit: 71f6981 (also identical at the weshnet version pinned by berty/berty master, 915f5df).

NewNativeLogger is used, for example, by berty/berty's mobile bridge (go/framework/bertybridge/bridge.go:82, logutil.NewNativeLogger("bertybridge")). In logger_native.go, Enabled always returns true and Check always adds the core, so every log entry of every level reaches NativeLog.

Reproduction (darwin)

I copied logger_native_darwin.go and logger_native.go unchanged into a small program. It creates NewNativeLogger("bertybridge").Named("grpc") and writes N Info entries. I built it with cgo on macOS (arm64) and ran it under leaks --atExit with N = 20000:

Process 7676: 59998 leaks for 3199888 total leaked bytes.
STACK OF 59997 INSTANCES OF 'ROOT LEAK: <malloc in _cgo_..._Cfunc__Cmalloc>':

59,997 of those blocks are malloc in _cgo_..._Cfunc__Cmalloc, i.e. C.CString, three per log line. The remaining 48-byte xpc_date_t block is unrelated: leaks --atExit reports it for any Go binary on this machine, including one that never logs. When each string is bound to a variable and freed after the call, the same loop reports no _Cfunc__Cmalloc blocks at all (1 leak for 48 total leaked bytes, the same xpc_date_t).

Suggested fix

Bind each string to a variable and free it after the call. For example, on Android (add #include <stdlib.h> to the preamble and import unsafe):

cTag := C.CString(namespace)
defer C.free(unsafe.Pointer(cTag))
cMsg := C.CString(fmt.Sprintf("[%s] %s", logLevel.CapitalString(), message))
defer C.free(unsafe.Pointer(cMsg))
C.__android_log_write(level, cTag, cMsg)

Apply the same change to the three strings in the darwin version.

If you could credit me as a reporter for my contributions to security advisory I will be thankful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions