Commit 555c888
authored
Address PEM crash caused by parsing certain older Go application binaries (#1976)
Summary: Address PEM crash caused by parsing certain older Go
application binaries
This PR fixes a crash caused by certain older Go application binaries.
In addition, this change includes the
`//src/stirling/binaries:go_binary_parse_profiling` cli tool. This tool
was helpful for debugging the previous 32 bit issue and aided in
debugging this problem (see Background section for more details). This
change is best reviewed commit by commit.
**Background**
Our Golang binary parsing was revamped in
#1605 to support Go 1.20.4
applications and later (#1318)
in addition to fixing a PEM crash caused by 32 bit go binaries
(#1300). While this solved the
aforementioned issues, it resulted in a new crash that we weren't able
to reproduce (#1646).
I was able to work with a Pixie Community Slack user to track down where
one of these issues originate from. The overview is that Go embeds
virtual addresses within the `.go.buildinfo` ELF section. These virtual
addresses are used in certain cases to read the build settings used when
the binary was created (toolchain version, go experiments, etc). In
order to properly read these strings, these virtual addresses need to be
converted into file offsets (binary addresses).
This bug presents itself when the `LOAD` ELF segments in the binary are
not contiguous or ordered by increasing virtual memory address. Meaning
if there are LOAD segments for segments 1, 2 and 3, this bug occurs if
those segments aren't adjacent to each other or don't have increasing
virtual memory addresses (vaddr of segment 1 < vaddr of segment 2 <
vaddr of segment 3). Instead the virtual address that needs to be looked
up, should be matched against the relevant segment and that segment's
virtual address offset should be used.
Relevant Issues: Partially addresses #1646 -- there is one more known
case, which must be investigated further
Type of change: /kind bug
Test Plan: Verified this change through the following
- [x] User from the community slack
[verified](https://pixie-community.slack.com/archives/CQ63KEVFY/p1722271309767939?thread_ts=1721315312.198319&cid=CQ63KEVFY)
that the issue was fixed.
- [x] New ElfReader function is covered with a test
- [x] go 1.17 test case ("[little
endian](https://github.com/pixie-io/pixie/blob/50ddcd32eb217e1aa5e87124883ee284a36052a1/src/stirling/obj_tools/go_syms_test.cc#L51)"
case) still works despite it not triggering this bug
- I was unable to recreate a binary that had the segments in an
unordered fashion.
Changelog Message: Fixed an issue with Go uprobe attachment that
previously caused crashes for a subset of older Go applications (Go 1.17
and earlier)
---------
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>1 parent c6e18a9 commit 555c888
File tree
8 files changed
+177
-9
lines changed- src/stirling
- binaries
- obj_tools
- source_connectors/socket_tracer
8 files changed
+177
-9
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
79 | 87 | | |
80 | 88 | | |
81 | 89 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
605 | 605 | | |
606 | 606 | | |
607 | 607 | | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
608 | 621 | | |
609 | 622 | | |
610 | 623 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
160 | 160 | | |
161 | 161 | | |
162 | 162 | | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
163 | 175 | | |
164 | 176 | | |
165 | 177 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
43 | 92 | | |
44 | 93 | | |
45 | 94 | | |
| |||
133 | 182 | | |
134 | 183 | | |
135 | 184 | | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
136 | 196 | | |
137 | 197 | | |
138 | 198 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
58 | | - | |
| 58 | + | |
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
65 | | - | |
66 | | - | |
| 65 | + | |
67 | 66 | | |
68 | 67 | | |
69 | 68 | | |
| |||
136 | 135 | | |
137 | 136 | | |
138 | 137 | | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
143 | 143 | | |
144 | 144 | | |
145 | 145 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
464 | 464 | | |
465 | 465 | | |
466 | 466 | | |
| 467 | + | |
| 468 | + | |
467 | 469 | | |
468 | 470 | | |
469 | 471 | | |
| |||
510 | 512 | | |
511 | 513 | | |
512 | 514 | | |
513 | | - | |
514 | | - | |
515 | 515 | | |
516 | 516 | | |
517 | 517 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
76 | 80 | | |
77 | 81 | | |
0 commit comments