add probes on fd_install to catch accept events before kretprobe was installed #39

alban · 2017-05-10T10:41:11Z

First start a process that will block in the accept() syscall:

$ busybox nc -l -p 8181

Once the accept() syscall has started, start the tcptracer-bpf tool:

$ sudo ./tracer $(pidof busybox)
Ready

Makes the accept() syscall return:

$ busybox nc 127.0.0.1 8181

Notice that we get the "connect" event as normal. The "accept" event is missing because of #10 but we get the new event "fdinstall" that says that the process 16389 received the new file descriptor 4:

224557401988314 cpu#2 connect 16402 busybox 127.0.0.1:45488 127.0.0.1:8181 4026531969
224557402151953 cpu#3 fdinstall 16389 busybox 0.0.0.0:0 0.0.0.0:0 4

Weave Scope would need to iterate over /proc to find all the processes blocked on the accept syscall (/proc/$pid/wchan will show inet_csk_accept) and then request the tracker to monitor fd_install on them:

  t.AddFdInstallWatcher(pid)

We don't notify fd_install events for all processes because that would be too much. Instead, Scope would only request the notification for specific pids. Internally, this uses a hash map to keep track of the pids to monitor or not.

We don't need any new offset guessing for this because we don't dereference any kernel structures in the fd_install kprobes.

This is for #10

Depends on:

TODO

test on Linux 4.4
add a field for fd in the event struct

/cc @iaguis @2opremio

2opremio · 2017-05-10T11:00:14Z

Weave Scope would need iterate over /proc to find all the processes blocked on the accept syscall (/proc/$pid/wchan will show inet_csk_accept) and then requests the tracker to monitor fd_install on them

Could we avoid this by making the probe on fd_install detect whether the file descriptor comes from a socket which was on LISTEN state?

alban · 2017-05-10T11:17:08Z

Could we avoid this by making the probe on fd_install detect whether the file descriptor comes from a socket which was on LISTEN state?

Not sure how to do that, but this would involve more complicated offset guessing to inspect the struct file received as a parameter of fd_install.

alban · 2017-05-10T16:25:02Z

I rebased this on top of #37 because I needed to revendor gobpf for iovisor/gobpf#45 but this also brings other gobpf API changes that cause tcptracer-bpf changes that were already done in #37.

tests/tracer.go

-	if len(os.Args) != 1 {
-		fmt.Fprintf(os.Stderr, "Usage: %s\n", os.Args[0])
+	if len(os.Args) > 2 {
+		fmt.Fprintf(os.Stderr, "Usage: %s pid\n", os.Args[0])


alban · 2017-05-12T15:44:21Z

I added a test and it passes in Semaphore with Linux 4.4.45 and 4.9.6. It works as well on my laptop with Linux 4.10.10.

alban · 2017-05-16T15:07:09Z

I updated this PR:

fix compilation on Darwin (revealed by Scope tests in CircleCI)
added more shellcheck fixes (it will need a proper "git subtree" update after shellcheck: fix escaping issue build-tools#97 is merged)

960d222 Merge pull request #97 from kinvolk/alban/update-shfmt-3 1d535c7 shellcheck: fix escaping issue git-subtree-dir: tools git-subtree-split: 960d2228b3c967d28b7cb329d020f44b55429d94

…install-3

alban · 2017-05-18T12:01:42Z

Since weaveworks/build-tools#97 is merged, I rebased & updated the "git subtree". But this time, there are new errors (see weaveworks/build-tools#99)

Symptoms: ``` Step 4 : RUN go get -u gopkg.in/mvdan/sh.v1/cmd/shfmt ---> Running in 0438ed51be8b go/src/gopkg.in/mvdan/sh.v1/cmd/shfmt/main.go:26: undefined: "github.com/mvdan/sh/syntax".ParseMode go/src/gopkg.in/mvdan/sh.v1/cmd/shfmt/main.go:27: undefined: "github.com/mvdan/sh/syntax".PrintConfig go/src/gopkg.in/mvdan/sh.v1/cmd/shfmt/main.go:46: undefined: "github.com/mvdan/sh/syntax".ParseComments go/src/gopkg.in/mvdan/sh.v1/cmd/shfmt/main.go:48: undefined: "github.com/mvdan/sh/syntax".PosixConformant go/src/gopkg.in/mvdan/sh.v1/cmd/shfmt/main.go:73: undefined: "github.com/mvdan/sh/syntax".Parse go/src/gopkg.in/mvdan/sh.v1/cmd/shfmt/main.go:139: undefined: "github.com/mvdan/sh/syntax".Parse The command '/bin/sh -c go get -u gopkg.in/mvdan/sh.v1/cmd/shfmt' returned a non-zero code: 2 make: *** [build-docker-image] Error 2 ```

alban · 2017-05-18T12:47:05Z

I added a commit that fixes the error. The Dockerfile now uses shfmt with another importpath. Thanks @tomwilkie for the hint in weaveworks/build-tools#99 (comment)

@2opremio PTAL

pkg/tracer/event_common.go

-	EventConnect EventType = 1
-	EventAccept            = 2
-	EventClose             = 3
+	EventConnect   EventType = 1


tcptracer-bpf.c

+{
+	u64 pid = bpf_get_current_pid_tgid();
+	u32 tgid = pid >> 32;
+	unsigned long fd = (unsigned long) PT_REGS_PARM1(ctx);


2opremio · 2017-05-18T15:38:56Z

Minor comment, otherwise LGTM

alban · 2017-05-19T09:40:13Z

@2opremio I think I replied to the comment. Someone with merge-powers could merge this :)

2opremio · 2017-05-19T10:41:51Z

Merging, but let's continue the comment thread on LLVM please.

This includes weaveworks/tcptracer-bpf#39

Since weaveworks/tcptracer-bpf#39, tcptracer-bpf can generate "fd_install" events when a process installs a new file descriptor in its fd table. Those events must be requested explicitely on a per-pid basis with tracer.AddFdInstallWatcher(pid). This is useful to know about "accept" events that would otherwise be missed because kretprobes are not triggered for functions that were called before the installation of the kretprobe. This patch find all the processes that are currently blocked on an accept() syscall during the EbpfTracker initialization. feedInitialConnections() will use tracer.AddFdInstallWatcher() to subscribe to fd_install events. When a fd_install event is received, synthesise an accept event with the connection tuple and the network namespace (from /proc).

alban mentioned this pull request May 10, 2017

More shfmt updates weaveworks/build-tools#96

Merged

alban mentioned this pull request May 10, 2017

update build-tools git-subtree (again) #40

Merged

alban force-pushed the alban/fdinstall branch 2 times, most recently from ee23432 to 58f9bf1 Compare May 10, 2017 16:20

alban force-pushed the alban/fdinstall branch from 58f9bf1 to 87a544e Compare May 11, 2017 10:44

schu reviewed May 11, 2017

View reviewed changes

tests/tracer.go Outdated

if len(os.Args) != 1 {

fmt.Fprintf(os.Stderr, "Usage: %s\n", os.Args[0])

if len(os.Args) > 2 {

fmt.Fprintf(os.Stderr, "Usage: %s pid\n", os.Args[0])

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This was referenced May 11, 2017

handle fdinstall events from tcptracer-bpf (aka "accept before kretprobe" issue) weaveworks/scope#2518

Merged

documentation: add diagrams explaining the different kprobes & maps #41

Open

alban force-pushed the alban/fdinstall branch 3 times, most recently from dac0b87 to 3b09ef1 Compare May 12, 2017 15:31

alban force-pushed the alban/fdinstall branch 2 times, most recently from 62db57c to ac7da48 Compare May 16, 2017 12:39

alban mentioned this pull request May 16, 2017

shellcheck: fix escaping issue weaveworks/build-tools#97

Merged

alban added 4 commits May 18, 2017 13:48

Squashed 'tools/' changes from 5542498..960d222

7bfea98

960d222 Merge pull request #97 from kinvolk/alban/update-shfmt-3 1d535c7 shellcheck: fix escaping issue git-subtree-dir: tools git-subtree-split: 960d2228b3c967d28b7cb329d020f44b55429d94

Merge commit '7bfea980d81375ff23d235245ff10e4da70010b8' into alban/fd…

0b7120c

…install-3

monitor specific pids for fd_install events

a798ea1

test: fd_install events

b241d74

alban force-pushed the alban/fdinstall branch from ac7da48 to b241d74 Compare May 18, 2017 11:49

alban changed the title ~~[RFC][WIP] add probes on fd_install to catch accept events before kretprobe was installed~~ add probes on fd_install to catch accept events before kretprobe was installed May 18, 2017

2opremio reviewed May 18, 2017

View reviewed changes

pkg/tracer/event_common.go

EventConnect EventType = 1

EventAccept = 2

EventClose = 3

EventConnect EventType = 1

This comment was marked as abuse.

Sign in to view

2opremio reviewed May 18, 2017

View reviewed changes

2opremio approved these changes May 19, 2017

View reviewed changes

2opremio merged commit 783f088 into weaveworks:master May 19, 2017

alban added a commit to kinvolk-archives/scope that referenced this pull request May 19, 2017

vendor: update tcptracer-bpf

1fb4ce7

This includes weaveworks/tcptracer-bpf#39

alban mentioned this pull request May 19, 2017

does not get the "accept" event for accept() syscall started before tcptracer-bpf #10

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add probes on fd_install to catch accept events before kretprobe was installed #39

add probes on fd_install to catch accept events before kretprobe was installed #39

alban commented May 10, 2017 •

edited

Loading

2opremio commented May 10, 2017 •

edited

Loading

alban commented May 10, 2017

alban commented May 10, 2017

This comment was marked as abuse.

This comment was marked as abuse.

alban commented May 12, 2017

alban commented May 16, 2017

alban commented May 18, 2017

alban commented May 18, 2017

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

2opremio commented May 18, 2017

alban commented May 19, 2017

2opremio commented May 19, 2017

add probes on fd_install to catch accept events before kretprobe was installed #39

add probes on fd_install to catch accept events before kretprobe was installed #39

Conversation

alban commented May 10, 2017 • edited Loading

2opremio commented May 10, 2017 • edited Loading

alban commented May 10, 2017

alban commented May 10, 2017

This comment was marked as abuse.

This comment was marked as abuse.

alban commented May 12, 2017

alban commented May 16, 2017

alban commented May 18, 2017

alban commented May 18, 2017

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

2opremio commented May 18, 2017

alban commented May 19, 2017

2opremio commented May 19, 2017

alban commented May 10, 2017 •

edited

Loading

2opremio commented May 10, 2017 •

edited

Loading