Invalid mem errors when using latest BCC

[gllghr]

When running with the latest version of BCC (not yet merged into our repo), we see the following error when running the io stbtrace/estat programs:

$ sudo estat backend-io -a rpool 5
bpf: Failed to load program: Permission denied
0: (79) r6 = *(u64 *)(r1 +112)
1: (7b) *(u64 *)(r10 -8) = r6
2: (b7) r7 = 0
3: (7b) *(u64 *)(r10 -16) = r7
4: (7b) *(u64 *)(r10 -24) = r7
5: (7b) *(u64 *)(r10 -48) = r7
6: (bf) r3 = r6
7: (07) r3 += 192
8: (bf) r1 = r10
9: (07) r1 += -48
10: (b7) r2 = 8
11: (85) call bpf_probe_read#4
12: (79) r8 = *(u64 *)(r10 -48)
13: (85) call bpf_ktime_get_ns#5
14: (7b) *(u64 *)(r10 -40) = r0
15: (63) *(u32 *)(r10 -48) = r7
16: (bf) r3 = r6
17: (07) r3 += 68
18: (bf) r1 = r10
19: (07) r1 += -48
20: (b7) r2 = 4
21: (85) call bpf_probe_read#4
22: (61) r1 = *(u32 *)(r10 -48)
23: (63) *(u32 *)(r10 -28) = r1
24: (07) r6 += 88
25: (63) *(u32 *)(r10 -48) = r7
26: (bf) r1 = r10
27: (07) r1 += -48
28: (b7) r2 = 4
29: (bf) r3 = r6
30: (85) call bpf_probe_read#4
31: (61) r1 = *(u32 *)(r10 -48)
32: (63) *(u32 *)(r10 -32) = r1
33: (61) r1 = *(u32 *)(r8 +12)
R8 invalid mem access 'inv'

HINT: The invalid mem access 'inv' error can happen if you try to dereference memory without first using bpf_probe_read() to copy it to the BPF stack. Sometimes the bpf_probe_read is automatic by the bcc rewriter, other times you'll need to be explicit.

Traceback (most recent call last):
  File "/usr/bin/estat", line 418, in <module>
    b.attach_kprobe(event=probe_spec[1], fn_name=probe_spec[2])
  File "/usr/lib/python3/dist-packages/bcc/__init__.py", line 656, in attach_kprobe
    fn = self.load_func(fn_name, BPF.KPROBE)
  File "/usr/lib/python3/dist-packages/bcc/__init__.py", line 397, in load_func
    (func_name, errstr))
Exception: Failed to load BPF program b'disk_io_start': Permission denied

I resolved these by switching to instances of string copying to use bpf_probe_read_str instead of __builtin_memcpy

Testing

I tested by running each of the estat programs and stbtrace collectors, and making sure they compiled and produced output that looked reasonably correct when run.

[prashks]

We should also try changing this to DISK_NAME_LEN (and corresponding field device of io_data_t) to match the io.st

[gllghr]

DEV_NAME_LEN (defined here) is 8, while DISK_NAME_LEN (defined in the kernel) is 32. I tried using DISK_NAME_LEN instead, and backend-io seems to run fine with various options enabled. I'm not sure why we chose to use a smaller value here, though.

[prashks]

Great, thanks for checking on this, we should go with the kernel defined one (DISK_NAME_LEN) then.

IIRC, during our early development there were issues with the string/buffer sizes when testing with various options and hence had ended up with the smaller size.