Add support to attach kprobe to offset #956

mmisono · 2019-11-08T10:29:44Z

Example: bpftrace -e 'kprobe:do_sys_open+9 { ... }'.

This is based on the support of attaching uprobe to offset (#803). The
offset is checked whether it is an instruction boundary using vmlinux.
The environment variable BPFTRACE_VMLINUX can be used to specify
vmlinux file. ALLOW_UNSAFE_UPROBE option is renamed to
ALLOW_UNSASFE_PROBE so that it supports both uprobe and kprobe.

Closes #42

mmisono · 2019-11-08T14:01:14Z

So kernel image is needed to test. Although it does not need to be a debug symbol... (it takes too much time to fetch.)

mmisono · 2019-11-09T05:50:24Z

It seems we can use extract-vmlinux. vmlinux is stripped, so we need to consult /boot/System.map to disassemble.

mmisono · 2019-11-11T14:41:16Z

Ah, I just realized that kprobe checks instruction boundaries when registering (src). I'm wondering userspace check is needed.

docs/reference_guide.md

src/attached_probe.cpp

danobi · 2019-12-24T00:35:01Z

Ah, I just realized that kprobe checks instruction boundaries when registering (src). I'm wondering userspace check is needed.

The link you posted is for the ftrace interface (/sys/kernel/debug/tracing). bpftrace uses the perf interface. That being said, it looks like the same checks are done in the perf code paths.

Can you do some tests to see if -EILSEQ is returned from perf_event_open? If so, we can probably skip the userspace checks on our side. But if the return code is something generic like EINVAL we might want to do the work ourselves so we can generate a proper diagnostic. The latter case is definitely open for discussion.

mmisono · 2019-12-28T06:19:13Z

Thanks for the review. I'll fix these points.

I confirmed that perf_event_open returns -EILSEQ if an instruction boundary is invalid, though I don't check the kernel code. Furthermore, bcc's bpf_attach_kprobe first tries to attach kprobe usingperf_event_open and if it fails, it'll uses ftrace interface.
https://github.com/iovisor/bcc/blob/4f0a887a1530c716190454b824a7f003fde8b9e8/src/cc/libbpf.c#L1023-L1032
If attaching kprobe fails, it prints an error message.
https://github.com/iovisor/bcc/blob/4f0a887a1530c716190454b824a7f003fde8b9e8/src/cc/libbpf.c#L1058

So, if bpftrace does not perform a boundary check and tries to attach kprobe to an invalid offset, the output looks like:

% sudo ./src/bpftrace -e 'kprobe:do_sys_open+2 { printf("hit\n"); }'
Attaching 1 probe...
cannot attach kprobe, Invalid or incomplete multibyte or wide character
Error attaching probe: 'kprobe:do_sys_open+2'

"cannot attach kprobe, Invalid ..." is a message from bcc.

One of the downsides of relying on kernel boundary checks is that it does not check whether the offset is within a function. For example, the following will work in my environment.

% ./src/bpftrace -e 'kprobe:do_sys_open+1002 { printf("hit\n"); }'
Attaching 1 probe...
^C

But if bpftrace checks boundary using vmlinux, it fails.

% ./src/bpftrace -e 'kprobe:do_sys_open+1002 { printf("hit\n"); }'
Attaching 1 probe...
Offset outside the function bounds ('do_sys_open' size is 598)

I'm thinking of the following:

Make boundary check with vmlinux optional.
- If --unsafe flag is given and bpftrace cannot found vmlinux, skip boundary checks in bpftrace (Or should it introduce another flag or environment variable?)
Change runtime test codes so that they test kprobe with offset without vmlinux.

mmisono · 2020-01-12T10:45:51Z

I made the following changes:

Skip userspace check if 1) bpftrace is compile with ALLOW_UNSAFE_PROBE, 2) vmlinux file is not found, and 3) --unsafe is given
- Linux kernel checks the alignment, but it does not check function boundary
Disable runtime test which requires vmlinux with debug symbols in CI
Update documents

danobi

Sorry about the delay. Everything LGTM. Just some small comments and the merge conflict.

danobi · 2020-01-17T01:27:16Z

src/attached_probe.cpp

+  // based on btf_location in btf.cpp
+  const char *vmlinux_locs[] = {
+    "/boot/vmlinux-%1$s",
+    "/lib/modules/%1$s/vmlinux-%1$s",
+    "/lib/modules/%1$s/build/vmlinux",
+    "/usr/lib/modules/%1$s/kernel/vmlinux",
+    "/usr/lib/debug/boot/vmlinux-%1$s",
+    "/usr/lib/debug/boot/vmlinux-%1$s.debug",
+    "/usr/lib/debug/lib/modules/%1$s/vmlinux",
+    nullptr,
+  };


Can you move this list to utils.cpp and have btf.cpp use it too? Would be nice to maintain it in one location

src/attached_probe.cpp

mmisono · 2020-01-17T22:57:13Z

@danobi
Is it possible to merge #1081 fast? The documentation change will conflict.

danobi · 2020-01-18T00:07:06Z

@mmisono merged #1081

Example: `bpftrace -e 'kprobe:do_sys_open+9 { ... }'`. This is based on the support of attaching uprobe to offset (bpftrace#803). The offset is checked whether it is an instruction boundary using vmlinux (with debug symbols). The environment variable `BPFTRACE_VMLINUX` can be used to specify vmlinux file. ALLOW_UNSAFE_UPROBE option is renamed to ALLOW_UNSASFE_PROBE so that it supports both uprobe and kprobe. If bpftrace is compoiled with ALLOW_UNSASFE_PROBE option and vmlinux file is not found, and --unsafe option is given, the userspace check is skipped. Note that linux kernel checks the address alignment, but it does not check whether it is within the function. Closes bpftrace#42

This test requires vmlinux with debug symbols. It is possible to download it in CI, but it takes time (6~7 mins).

vmlinux is used for both symbol resolution and BTF. Manage the location list in one place.

mmisono · 2020-01-18T06:27:10Z

@danobi
Updated. Please take a look when you have time.

ajor · 2020-01-20T23:14:34Z

* Skip userspace check if 1) bpftrace is compile with `ALLOW_UNSAFE_PROBE`, 2) vmlinux file is not found, and 3) --unsafe is given
  
  * Linux kernel checks the alignment, but it does not check function boundary

Is there anything wrong with not checking the function boundary? What does "unsafe" mean here? As far as I know BPF is always "safe" in that nothing we run in it can take down the kernel.

mmisono · 2020-01-21T10:23:08Z

Indeed, I believe not checking the boundary is totally safe in that sense. The checking just prevents us from some coding errors.

Once again, I'm wondering userspace check is needed. Having the functionality of reading vmlinux may be useful in the future though (e.g., perf-probe analyze DWARF symbols and can show information on local variable locations.)

danobi · 2020-01-21T19:12:26Z

I guess we don't have to check boundaries in userspace. But IMO it's a nice check to prevent some unintended bugs.

We can always relax these checks without breaking API but adding them in later will break API. I don't feel too strongly either way. If it's not too much of runtime overhead I'd lean towards leaving userspace checks in.

mmisono · 2020-01-21T21:28:23Z

In addition, "--unsafe" here means skipping user checking and this is the same as the option for uprobe. It is possible to attach uprobe to the middle of instructions, so it is "unsafe". On the other hand, in the case of kprobe, kernel does minimum safety checks.

danobi · 2020-01-22T01:43:06Z

Let's merge this for now. If we run into issues or change our minds, the door is always open to take out userspace checks.

danobi reviewed Dec 24, 2019

View reviewed changes

docs/reference_guide.md Outdated Show resolved Hide resolved

docs/reference_guide.md Show resolved Hide resolved

src/attached_probe.cpp Outdated Show resolved Hide resolved

src/attached_probe.cpp Outdated Show resolved Hide resolved

mmisono force-pushed the kprobe_offset branch from 9e32ec5 to e344595 Compare January 12, 2020 09:41

danobi reviewed Jan 17, 2020

View reviewed changes

mmisono force-pushed the kprobe_offset branch from e344595 to e348ede Compare January 18, 2020 05:40

mmisono added 2 commits January 18, 2020 06:02

Disable probe.kprobe_offset_fail_size runtime test in CI

76f08de

This test requires vmlinux with debug symbols. It is possible to download it in CI, but it takes time (6~7 mins).

Unify vmlinux and BTF location list

d58bf05

vmlinux is used for both symbol resolution and BTF. Manage the location list in one place.

mmisono force-pushed the kprobe_offset branch from e348ede to d58bf05 Compare January 18, 2020 06:03

danobi approved these changes Jan 21, 2020

View reviewed changes

danobi merged commit 1d39776 into bpftrace:master Jan 22, 2020

BurntBrunch mentioned this pull request Aug 15, 2022

bpftrace cannot attach kprobe to module symbol+offset #2327

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to attach kprobe to offset #956

Add support to attach kprobe to offset #956

mmisono commented Nov 8, 2019

mmisono commented Nov 8, 2019 •

edited

mmisono commented Nov 9, 2019

mmisono commented Nov 11, 2019

danobi commented Dec 24, 2019

mmisono commented Dec 28, 2019

mmisono commented Jan 12, 2020

danobi left a comment •

edited

danobi Jan 17, 2020

mmisono commented Jan 17, 2020

danobi commented Jan 18, 2020

mmisono commented Jan 18, 2020

ajor commented Jan 20, 2020

mmisono commented Jan 21, 2020

danobi commented Jan 21, 2020

mmisono commented Jan 21, 2020

danobi commented Jan 22, 2020

Add support to attach kprobe to offset #956

Add support to attach kprobe to offset #956

Conversation

mmisono commented Nov 8, 2019

mmisono commented Nov 8, 2019 • edited

mmisono commented Nov 9, 2019

mmisono commented Nov 11, 2019

danobi commented Dec 24, 2019

mmisono commented Dec 28, 2019

mmisono commented Jan 12, 2020

danobi left a comment • edited

Choose a reason for hiding this comment

danobi Jan 17, 2020

Choose a reason for hiding this comment

mmisono commented Jan 17, 2020

danobi commented Jan 18, 2020

mmisono commented Jan 18, 2020

ajor commented Jan 20, 2020

mmisono commented Jan 21, 2020

danobi commented Jan 21, 2020

mmisono commented Jan 21, 2020

danobi commented Jan 22, 2020

mmisono commented Nov 8, 2019 •

edited

danobi left a comment •

edited