Merge pull request #2695 from weaveworks/2687-ebpf-init-race-segfault

fix ebpf init race segfault

Fixes #2687

probe/endpoint/ebpf.go

@@ -33,8 +33,6 @@ type eventTracker interface {
 	stop()
 }
 
-var ebpfTracker *EbpfTracker
-
 // EbpfTracker contains the sets of open and closed TCP connections.
 // Closed connections are kept in the `closedConnections` slice for one iteration of `walkConnections`.
 type EbpfTracker struct {
@@ -86,51 +84,52 @@ func newEbpfTracker() (eventTracker, error) {
 		return nil, fmt.Errorf("kernel not supported: %v", err)
 	}
 
-	t, err := tracer.NewTracer(tcpEventCbV4, tcpEventCbV6, lostCb)
+	tracker := &EbpfTracker{
+		openConnections: map[string]ebpfConnection{},
+	}
+
+	tracer, err := tracer.NewTracer(tracker.tcpEventCbV4, tracker.tcpEventCbV6, tracker.lostCb)
 	if err != nil {
 		return nil, err
 	}
 
-	tracker := &EbpfTracker{
-		openConnections: map[string]ebpfConnection{},
-		tracer:          t,
-	}
+	tracker.tracer = tracer
+	tracer.Start()
 
-	ebpfTracker = tracker
 	return tracker, nil
 }
 
 var lastTimestampV4 uint64
 
-func tcpEventCbV4(e tracer.TcpV4) {
+func (t *EbpfTracker) tcpEventCbV4(e tracer.TcpV4) {
 	if lastTimestampV4 > e.Timestamp {
 		// A kernel bug can cause the timestamps to be wrong (e.g. on Ubuntu with Linux 4.4.0-47.68)
 		// Upgrading the kernel will fix the problem. For further info see:
 		// https://github.com/iovisor/bcc/issues/790#issuecomment-263704235
 		// https://github.com/weaveworks/scope/issues/2334
 		log.Errorf("tcp tracer received event with timestamp %v even though the last timestamp was %v. Stopping the eBPF tracker.", e.Timestamp, lastTimestampV4)
-		ebpfTracker.dead = true
-		ebpfTracker.stop()
+		t.dead = true
+		t.stop()
 	}
 
 	lastTimestampV4 = e.Timestamp
 
 	if e.Type == tracer.EventFdInstall {
-		ebpfTracker.handleFdInstall(e.Type, int(e.Pid), int(e.Fd))
+		t.handleFdInstall(e.Type, int(e.Pid), int(e.Fd))
 	} else {
 		tuple := fourTuple{e.SAddr.String(), e.DAddr.String(), e.SPort, e.DPort}
-		ebpfTracker.handleConnection(e.Type, tuple, int(e.Pid), strconv.Itoa(int(e.NetNS)))
+		t.handleConnection(e.Type, tuple, int(e.Pid), strconv.Itoa(int(e.NetNS)))
 	}
 }
 
-func tcpEventCbV6(e tracer.TcpV6) {
+func (t *EbpfTracker) tcpEventCbV6(e tracer.TcpV6) {
 	// TODO: IPv6 not supported in Scope
 }
 
-func lostCb(count uint64) {
+func (t *EbpfTracker) lostCb(count uint64) {
 	log.Errorf("tcp tracer lost %d events. Stopping the eBPF tracker", count)
-	ebpfTracker.dead = true
-	ebpfTracker.stop()
+	t.dead = true
+	t.stop()
 }
 
 func tupleFromPidFd(pid int, fd int) (tuple fourTuple, netns string, ok bool) {

probe/endpoint/ebpf_test.go

@@ -209,13 +209,12 @@ func TestInvalidTimeStampDead(t *testing.T) {
 		dead:            false,
 		openConnections: map[string]ebpfConnection{},
 	}
-	ebpfTracker = mockEbpfTracker
 	event.Timestamp = 0
-	tcpEventCbV4(event)
+	mockEbpfTracker.tcpEventCbV4(event)
 	event2 := event
 	event2.SPort = 1
 	event2.Timestamp = 2
-	tcpEventCbV4(event2)
+	mockEbpfTracker.tcpEventCbV4(event2)
 	mockEbpfTracker.walkConnections(func(e ebpfConnection) {
 		cnt++
 	})
@@ -227,7 +226,7 @@ func TestInvalidTimeStampDead(t *testing.T) {
 	}
 	cnt = 0
 	event.Timestamp = 1
-	tcpEventCbV4(event)
+	mockEbpfTracker.tcpEventCbV4(event)
 	mockEbpfTracker.walkConnections(func(e ebpfConnection) {
 		cnt++
 	})

vendor/github.com/weaveworks/tcptracer-bpf/pkg/tracer/tracer.go

@@ -101,9 +101,6 @@ func NewTracer(tcpEventCbV4 func(TcpV4), tcpEventCbV6 func(TcpV6), lostCb func(l
 		}
 	}()
 
-	perfMapIPV4.PollStart()
-	perfMapIPV6.PollStart()
-
 	return &Tracer{
 		m:           m,
 		perfMapIPV4: perfMapIPV4,
@@ -112,6 +109,11 @@ func NewTracer(tcpEventCbV4 func(TcpV4), tcpEventCbV6 func(TcpV6), lostCb func(l
 	}, nil
 }
 
+func (t *Tracer) Start() {
+	t.perfMapIPV4.PollStart()
+	t.perfMapIPV6.PollStart()
+}
+
 func (t *Tracer) AddFdInstallWatcher(pid uint32) (err error) {
 	var one uint32 = 1
 	mapFdInstall := t.m.Map("fdinstall_pids")

vendor/github.com/weaveworks/tcptracer-bpf/pkg/tracer/tracer_unsupported.go

@@ -15,12 +15,13 @@ func TracerAsset() ([]byte, error) {
 func NewTracer(tcpEventCbV4 func(TcpV4), tcpEventCbV6 func(TcpV6), lostCb func(lost uint64)) (*Tracer, error) {
 	return nil, fmt.Errorf("not supported on non-Linux systems")
 }
+func (t *Tracer) Start() {
+}
 func (t *Tracer) AddFdInstallWatcher(pid uint32) (err error) {
 	return fmt.Errorf("not supported on non-Linux systems")
 }
 func (t *Tracer) RemoveFdInstallWatcher(pid uint32) (err error) {
 	return fmt.Errorf("not supported on non-Linux systems")
 }
-
 func (t *Tracer) Stop() {
 }

vendor/github.com/weaveworks/tcptracer-bpf/tcptracer-bpf.c

@@ -11,6 +11,7 @@
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wtautological-compare"
+#pragma clang diagnostic ignored "-Wgnu-variable-sized-type-not-at-end"
 #include <net/sock.h>
 #pragma clang diagnostic pop
 #include <net/inet_sock.h>

vendor/github.com/weaveworks/tcptracer-bpf/tests/tracer.go

@@ -67,6 +67,8 @@ func main() {
 		os.Exit(1)
 	}
 
+	t.Start()
+
 	for _, p := range strings.Split(watchFdInstallPids, ",") {
 		if p == "" {
 			continue

vendor/manifest

@@ -1038,7 +1038,7 @@
 			"importpath": "github.com/weaveworks/tcptracer-bpf",
 			"repository": "https://github.com/weaveworks/tcptracer-bpf",
 			"vcs": "git",
-			"revision": "a616ebc6c5ac196af743e5dfc621a52fce030f87",
+			"revision": "fc80d1659bf07dabebbb42c5507578440103d66f",
 			"branch": "master",
 			"notests": true
 		},