Skip to content

Commit

Permalink
bpf: manually handle the perf read state memory allocation
Browse files Browse the repository at this point in the history
When the Cilium monitor is under heavy load we see the following crash

	panic: runtime error: cgo argument has Go pointer to Go pointer

The crash is occurring near the call to `C.perf_event_read(...)`.  At
some point in time the state struct instance contains a Go pointer to Go
pointer.  According to the [Rules for passing pointers between Go and
C][0] this is not allowed.

>Go code may pass a Go pointer to C provided that the Go memory to which
it points does not contain any Go pointers.

I have not studied the Go source well enough to back up any suspicions
that something is wrong in the runtime code, but looking at workarounds
we could bypass the crasher by disabling the cgo runtime checks, but
that doesn't really make sense considering that it requires a user to
make a change. The other approach, which this patch does is instead of
relying on `struct_read_state` to do the right thing, we fallback to
malloc / free so we can reduce the chance of mismatch between Go and C
memory.

After this patch I am not able to reproduce the crasher.

[0]: https://github.com/golang/proposal/blob/master/design/12416-cgo-pointers.md
Closes: #409 (Crash in cilium monitor (ring buffer overflow?))
Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
  • Loading branch information
aalemayhu committed Aug 13, 2017
1 parent e88e1fa commit 9bd5652
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions pkg/bpf/perf.go
Expand Up @@ -23,6 +23,7 @@ package bpf
#include <linux/bpf.h>
#include <linux/perf_event.h>
#include <sys/resource.h>
#include <stdlib.h>
void create_perf_event_attr(int type, int config, int sample_type,
int wakeup_events, void *attr)
Expand Down Expand Up @@ -303,11 +304,11 @@ func (e *PerfEvent) Disable() error {

func (e *PerfEvent) Read(receive ReceiveFunc, lostFn LostFunc) error {
buf := make([]byte, 256)
state := C.struct_read_state{}
state := C.malloc(C.size_t(unsafe.Sizeof(C.struct_read_state{})))

// Prepare for reading and check if events are available
available := C.perf_event_read_init(C.int(e.npages), C.int(e.pagesize),
unsafe.Pointer(&e.data[0]), unsafe.Pointer(&state))
unsafe.Pointer(&e.data[0]), unsafe.Pointer(state))

// Poll false positive
if available == 0 {
Expand All @@ -317,7 +318,7 @@ func (e *PerfEvent) Read(receive ReceiveFunc, lostFn LostFunc) error {
for {
var msg *PerfEventHeader

if ok := C.perf_event_read(unsafe.Pointer(&state),
if ok := C.perf_event_read(unsafe.Pointer(state),
unsafe.Pointer(&buf[0]), unsafe.Pointer(&msg)); ok == 0 {
break
}
Expand All @@ -339,7 +340,8 @@ func (e *PerfEvent) Read(receive ReceiveFunc, lostFn LostFunc) error {
}

// Move ring buffer tail pointer
C.perf_event_read_finish(unsafe.Pointer(&e.data[0]), unsafe.Pointer(&state))
C.perf_event_read_finish(unsafe.Pointer(&e.data[0]), unsafe.Pointer(state))
C.free(state)

return nil
}
Expand Down

0 comments on commit 9bd5652

Please sign in to comment.