String loaded using BPF_probe_read being overridden by constant string in some cases
- Dominant language
- C
- Stars
- 22.7k
- Forks
- 4.1k
- Avg merge
- 10d 4h
- Merged PRs (30d)
- 3
Description
LLVM: 6, 7 and 8
BCC: 0.9
Kernel: 4.15.0-47-generic
In some cases, strings loaded with BPF_probe_read are being overridden with constant strings. This seems like an LLVM optimization issue.
The code below works as expeced:
```python
#!/usr/bin/env python
from bcc import BPF
from time import sleep
text = """
#include
#include
#define NODENAME_LEN __NEW_UTS_LEN
typedef struct Foo {
char loaded[NODENAME_LEN];
char constant[10];
} Foo_t;
BPF_HASH(foos, Foo_t);
int bug_example(struct pt_regs *ctx) {
char constant[] = "mystr";
struct task_struct *task = (struct task_struct *)bpf_get_current_task();
char loaded[NODENAME_LEN] = "";
__builtin_memcpy(loaded, task->nsproxy->uts_ns->name.nodename, NODENAME_LEN);
Foo_t foo;
__builtin_memcpy(&foo.loaded, loaded, sizeof(foo.loaded));
__builtin_memcpy(&foo.constant, constant, sizeof(foo.constant));
u64 ts = 1;
foos.update(&foo, &ts);
return 0;
}
"""
b = BPF(text=text, debug=0)
b.attach_kprobe(event="do_nanosleep", fn_name="bug_example")
sleep(2)
counts = b.get_table("foos")
for k, v in counts.items():
print("%10d \"%s\" \"%s\"" % (v.value, k.loaded, k.constant))
```
Output:
```
# python bug.py
1 "laul-mmarchini.corp.netflix.com" "mystr"
1 "laul-mmarchini" "mystr"
```
But if we swap `loaded` and `constant` in the struct `Foo`, the `loaded` string will be overriden with the `constant` string, leading to a erroneous result.
```diff
@@ -12,8 +12,8 @@
typedef struct Foo {
- char loaded[NODENAME_LEN];
char constant[10];
+ char loaded[NODENAME_LEN];
} Foo_t;
BPF_HASH(foos, Foo_t);
```
Output:
```
1 "mystr" "mystr"
1 "mystr" "mystr"
1 "mystr" "mystr"
```
This is the correct BPF program:
```
0: (85) call bpf_get_current_task#35
1: (b7) r6 = 0
2: (73) *(u8 *)(r10 -32) = r6
3: (7b) *(u64 *)(r10 -40) = r6
4: (7b) *(u64 *)(r10 -48) = r6
5: (7b) *(u64 *)(r10 -56) = r6
6: (7b) *(u64 *)(r10 -64) = r6
7: (7b) *(u64 *)(r10 -72) = r6
8: (7b) *(u64 *)(r10 -80) = r6
9: (7b) *(u64 *)(r10 -88) = r6
10: (7b) *(u64 *)(r10 -96) = r6
11: (7b) *(u64 *)(r10 -8) = r6
12: (7b) *(u64 *)(r10 -16) = r6
13: (07) r0 += 2712
14: (bf) r1 = r10
15: (07) r1 += -16
16: (b7) r2 = 8
17: (bf) r3 = r0
18: (85) call bpf_probe_read#4
19: (79) r3 = *(u64 *)(r10 -16)
20: (07) r3 += 8
21: (bf) r1 = r10
22: (07) r1 += -8
23: (b7) r2 = 8
24: (85) call bpf_probe_read#4
25: (79) r3 = *(u64 *)(r10 -8)
26: (07) r3 += 69
27: (bf) r1 = r10
28: (07) r1 += -96
29: (b7) r2 = 65
30: (85) call bpf_probe_read#4
31: (18) r1 = 0x727473796d
33: (7b) *(u64 *)(r10 -32) = r1
34: (79) r1 = *(u64 *)(r10 -40)
35: (7b) *(u64 *)(r10 -40) = r1
36: (79) r1 = *(u64 *)(r10 -48)
37: (7b) *(u64 *)(r10 -48) = r1
38: (79) r1 = *(u64 *)(r10 -56)
39: (7b) *(u64 *)(r10 -56) = r1
40: (79) r1 = *(u64 *)(r10 -64)
41: (7b) *(u64 *)(r10 -64) = r1
42: (79) r1 = *(u64 *)(r10 -72)
43: (7b) *(u64 *)(r10 -72) = r1
44: (79) r1 = *(u64 *)(r10 -80)
45: (7b) *(u64 *)(r10 -80) = r1
46: (79) r1 = *(u64 *)(r10 -88)
47: (7b) *(u64 *)(r10 -88) = r1
48: (79) r1 = *(u64 *)(r10 -96)
49: (7b) *(u64 *)(r10 -96) = r1
50: (6b) *(u16 *)(r10 -24) = r6
51: (b7) r1 = 1
52: (7b) *(u64 *)(r10 -8) = r1
53: (18) r1 = 0xffff97f3091f1600
55: (bf) r2 = r10
56: (07) r2 += -96
57: (bf) r3 = r10
58: (07) r3 += -8
59: (b7) r4 = 0
60: (85) call bpf_map_update_elem#2
61: (b7) r0 = 0
62: (95) exit
processed 61 insns, stack depth 96
```
And this is the faulty BPF program:
```
0: (85) call bpf_get_current_task#35
1: (b7) r6 = 0
2: (73) *(u8 *)(r10 -32) = r6
3: (7b) *(u64 *)(r10 -40) = r6
4: (7b) *(u64 *)(r10 -48) = r6
5: (7b) *(u64 *)(r10 -56) = r6
6: (7b) *(u64 *)(r10 -64) = r6
7: (7b) *(u64 *)(r10 -72) = r6
8: (7b) *(u64 *)(r10 -80) = r6
9: (7b) *(u64 *)(r10 -88) = r6
10: (7b) *(u64 *)(r10 -96) = r6
11: (7b) *(u64 *)(r10 -8) = r6
12: (7b) *(u64 *)(r10 -16) = r6
13: (07) r0 += 2712
14: (bf) r1 = r10
15: (07) r1 += -16
16: (b7) r2 = 8
17: (bf) r3 = r0
18: (85) call bpf_probe_read#4
19: (79) r3 = *(u64 *)(r10 -16)
20: (07) r3 += 8
21: (bf) r1 = r10
22: (07) r1 += -8
23: (b7) r2 = 8
24: (85) call bpf_probe_read#4
25: (79) r3 = *(u64 *)(r10 -8)
26: (07) r3 += 69
27: (bf) r1 = r10
28: (07) r1 += -96
29: (b7) r2 = 65
30: (85) call bpf_probe_read#4
31: (18) r1 = 0x727473796d
33: (7b) *(u64 *)(r10 -96) = r1
34: (79) r1 = *(u64 *)(r10 -40)
35: (bf) r2 = r1
36: (77) r2 >>= 48
37: (6b) *(u16 *)(r10 -24) = r2
38: (bf) r2 = r1
39: (77) r2 >>= 32
40: (6b) *(u16 *)(r10 -26) = r2
41: (6b) *(u16 *)(r10 -30) = r1
42: (77) r1 >>= 16
43: (6b) *(u16 *)(r10 -28) = r1
44: (79) r1 = *(u64 *)(r10 -48)
45: (bf) r2 = r1
46: (77) r2 >>= 48
47: (6b) *(u16 *)(r10 -32) = r2
48: (bf) r2 = r1
49: (77) r2 >>= 32
50: (6b) *(u16 *)(r10 -34) = r2
51: (6b) *(u16 *)(r10 -38) = r1
52: (77) r1 >>= 16
53: (6b) *(u16 *)(r10 -36) = r1
54: (79) r1 = *(u64 *)(r10 -56)
55: (bf) r2 = r1
56: (77) r2 >>= 48
57: (6b) *(u16 *)(r10 -40) = r2
58: (bf) r2 = r1
59: (77) r2 >>= 32
60: (6b) *(u16 *)(r10 -42) = r2
61: (6b) *(u16 *)(r10 -46) = r1
62: (77) r1 >>= 16
63: (6b) *(u16 *)(r10 -44) = r1
64: (79) r1 = *(u64 *)(r10 -64)
65: (bf) r2 = r1
66: (77) r2 >>= 48
67: (6b) *(u16 *)(r10 -48) = r2
68: (bf) r2 = r1
69: (77) r2 >>= 32
70: (6b) *(u16 *)(r10 -50) = r2
71: (6b) *(u16 *)(r10 -54) = r1
72: (77) r1 >>= 16
73: (6b) *(u16 *)(r10 -52) = r1
74: (79) r1 = *(u64 *)(r10 -72)
75: (bf) r2 = r1
76: (77) r2 >>= 48
77: (6b) *(u16 *)(r10 -56) = r2
78: (bf) r2 = r1
79: (77) r2 >>= 32
80: (6b) *(u16 *)(r10 -58) = r2
81: (6b) *(u16 *)(r10 -62) = r1
82: (77) r1 >>= 16
83: (6b) *(u16 *)(r10 -60) = r1
84: (79) r1 = *(u64 *)(r10 -80)
85: (bf) r2 = r1
86: (77) r2 >>= 48
87: (6b) *(u16 *)(r10 -64) = r2
88: (bf) r2 = r1
89: (77) r2 >>= 32
90: (6b) *(u16 *)(r10 -66) = r2
91: (6b) *(u16 *)(r10 -70) = r1
92: (77) r1 >>= 16
93: (6b) *(u16 *)(r10 -68) = r1
94: (79) r1 = *(u64 *)(r10 -88)
95: (bf) r2 = r1
96: (77) r2 >>= 48
97: (6b) *(u16 *)(r10 -72) = r2
98: (bf) r2 = r1
99: (77) r2 >>= 32
100: (6b) *(u16 *)(r10 -74) = r2
101: (6b) *(u16 *)(r10 -78) = r1
102: (77) r1 >>= 16
103: (6b) *(u16 *)(r10 -76) = r1
104: (79) r1 = *(u64 *)(r10 -96)
105: (bf) r2 = r1
106: (77) r2 >>= 48
107: (6b) *(u16 *)(r10 -80) = r2
108: (bf) r2 = r1
109: (77) r2 >>= 32
110: (6b) *(u16 *)(r10 -82) = r2
111: (6b) *(u16 *)(r10 -86) = r1
112: (77) r1 >>= 16
113: (6b) *(u16 *)(r10 -84) = r1
114: (6b) *(u16 *)(r10 -88) = r6
115: (b7) r1 = 1
116: (7b) *(u64 *)(r10 -8) = r1
117: (18) r1 = 0xffff97f337e71600
119: (bf) r2 = r10
120: (07) r2 += -96
121: (bf) r3 = r10
122: (07) r3 += -8
123: (b7) r4 = 0
124: (85) call bpf_map_update_elem#2
125: (b7) r0 = 0
126: (95) exit
processed 125 insns, stack depth 96
```
In both cases, `task->nsproxy->uts_ns->name.nodename` will be loaded at `(r10 --96)` on instruction 30, but in the second case `(r10 --96)` is overridden by instruction 33, so the problem is here:
```
25: (79) r3 = *(u64 *)(r10 -8)
26: (07) r3 += 69
27: (bf) r1 = r10
28: (07) r1 += -96
29: (b7) r2 = 65
30: (85) call bpf_probe_read#4
31: (18) r1 = 0x727473796d
33: (7b) *(u64 *)(r10 -96) = r1
```
LLVM is not respecting the lifetime of the `loaded` variable and is overriding it's location on the BPF stack.
A quick workaround for users is "be careful with the order of string members in structures", but the second program is completely valid and should work, so we should probably fix it upstream.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reproducing the Python BCC example with the listed LLVM, BCC, and kernel versions, then compare the correct and faulty BPF instruction listings when the struct members are swapped. Investigate LLVM's BPF stack allocation and variable-lifetime handling around BPF_probe_read and the constant string. Done means the loaded buffer is not overwritten and both struct layouts produce the expected output.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, linux
- Domain
- compilers, operating-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100