Fix random alpha-numeric string affixed to binary path uprobe keys that are too long.#4
Closed
etep wants to merge 4 commits intopixie-io:pixie5from
Closed
Fix random alpha-numeric string affixed to binary path uprobe keys that are too long.#4etep wants to merge 4 commits intopixie-io:pixie5from
etep wants to merge 4 commits intopixie-io:pixie5from
Conversation
Signed-off-by: Pete Stevenson <jps@pixielabs.ai>
etep
changed the title
Signed-off-by: Pete Stevenson <jps@pixielabs.ai>
oazizi000
reviewed
Dec 2, 2022
Collaborator
oazizi000
left a comment
oazizi000
left a comment
There was a problem hiding this comment.
Looks good. A couple of small comments, but nothing fundamental.
src/cc/api/BPF.cc
Outdated
Comment on lines
861
to
862
| static std::random_device rd; | ||
| std::uniform_int_distribution<size_t> dist(0, sizeof(kDict)-1); | ||
| static std::mt19937 gen(rd()); |
Collaborator
There was a problem hiding this comment.
Do we really need a random device for the seed, or can we just use a fixed seed? I think all we need is uniqueness, not true run-to-run randomness.
src/cc/api/BPF.cc
Outdated
| static std::random_device rd; | ||
| std::uniform_int_distribution<size_t> dist(0, sizeof(kDict)-1); | ||
| static std::mt19937 gen(rd()); | ||
| std::uniform_int_distribution<size_t> dist(0, sizeof(kDict)-2); |
Collaborator
There was a problem hiding this comment.
minor: should we make this static as well? I know it'll work either way, but it'll save some work on every invocation. And we're already going down the static path, so could be more consistent.
oazizi000
reviewed
Dec 5, 2022
src/cc/api/BPF.cc
Outdated
Comment on lines
860
to
862
| static constexpr char kDict[] = "0123456789abcdefghijklmnopqrstuvwxyz"; | ||
| static std::random_device rd; | ||
| std::uniform_int_distribution<size_t> dist(0, sizeof(kDict)-1); | ||
| static std::mt19937 gen; | ||
| static std::uniform_int_distribution<size_t> dist(0, sizeof(kDict)-2); |
Collaborator
There was a problem hiding this comment.
Offline discussion: Why -2? Turns out it is for the null character.
Instead we should consider using:
static constexpr std::string_view kDict = "0123456789abcdefghijklmnopqrstuvwxyz"
And then we can do kDict.size() - 1
Signed-off-by: Pete Stevenson <jps@pixielabs.ai>
oazizi000
approved these changes
Dec 5, 2022
Collaborator
|
Merged in b276d10. |
JamesMBartlett
pushed a commit
that referenced
this pull request
Dec 8, 2022
…for -v option
Add additional information and change format of backtrace
- add symbol base offset, dso name, dso base offset
- symbol and dso info is included if it's available in target binary
- changed format:
INDEX ADDR [SYMBOL+OFFSET] (MODULE+OFFSET)
Print backtrace of ip if it failed to get syms.
Before:
# offcputime -v
psiginfo
vscanf
__snprintf_chk
[unknown]
[unknown]
[unknown]
[unknown]
[unknown]
sd_event_exit
sd_event_dispatch
sd_event_run
[unknown]
__libc_start_main
[unknown]
- systemd-journal (204)
1
xas_load
xas_find
filemap_map_pages
__handle_mm_fault
handle_mm_fault
do_page_fault
do_translation_fault
do_mem_abort
do_el0_ia_bp_hardening
el0_ia
xas_load
--
failed to get syms
- PmLogCtl (138757)
1
After:
# offcputime -v
#0 0xffffffc01018b7e8 __arm64_sys_clock_nanosleep+0x0
#1 0xffffffc01009a93c el0_svc_handler+0x34
#2 0xffffffc010084a08 el0_svc+0x8
#3 0xffffffc01018b7e8 __arm64_sys_clock_nanosleep+0x0
--
#4 0x0000007fa0bffd14 clock_nanosleep+0x94 (/usr/lib/libc-2.31.so+0x9ed14)
#5 0x0000007fa0c0530c nanosleep+0x1c (/usr/lib/libc-2.31.so+0xa430c)
#6 0x0000007fa0c051e4 sleep+0x34 (/usr/lib/libc-2.31.so+0xa41e4)
#7 0x000000558a5a9608 flb_loop+0x28 (/usr/bin/fluent-bit+0x52608)
#8 0x000000558a59f1c4 flb_main+0xa84 (/usr/bin/fluent-bit+0x481c4)
#9 0x0000007fa0b85124 __libc_start_main+0xe4 (/usr/lib/libc-2.31.so+0x24124)
iovisor#10 0x000000558a59d828 _start+0x34 (/usr/bin/fluent-bit+0x46828)
- fluent-bit (1238)
1
#0 0xffffffc01027daa4 generic_copy_file_checks+0x334
#1 0xffffffc0102ba634 __handle_mm_fault+0x8dc
#2 0xffffffc0102baa20 handle_mm_fault+0x168
#3 0xffffffc010ad23c0 do_page_fault+0x148
#4 0xffffffc010ad27c0 do_translation_fault+0xb0
#5 0xffffffc0100816b0 do_mem_abort+0x50
#6 0xffffffc0100843b0 el0_da+0x1c
#7 0xffffffc01027daa4 generic_copy_file_checks+0x334
--
#8 0x0000007f8dc12648 [unknown]
#9 0x0000007f8dc0aef8 [unknown]
iovisor#10 0x0000007f8dc1c990 [unknown]
iovisor#11 0x0000007f8dc08b0c [unknown]
iovisor#12 0x0000007f8dc08e48 [unknown]
iovisor#13 0x0000007f8dc081c8 [unknown]
- PmLogCtl (2412)
1
Fixed: iovisor#3884
Signed-off-by: Eunseon Lee <es.lee@lge.com>
ddelnano
pushed a commit
that referenced
this pull request
Oct 9, 2024
…option
Add additional information and change format of backtrace
- add symbol base offset, dso name, dso base offset
- symbol and dso info is included if it's available in target binary
- changed format:
INDEX ADDR [SYMBOL+OFFSET] (MODULE+OFFSET)
before:
# ./capable -UK
TIME UID PID COMM CAP NAME AUDIT VER DICT
01:59:17 0 730 irqbalance 21 CAP_SYS_ADMIN 0 deny
cap_vm_enough_memory
security_vm_enough_memory_mm
mmap_region
do_mmap
vm_mmap_pgoff
do_syscall_64
entry_SYSCALL_64_after_hwframe
mmap64
- irqbalance (730)
After:
# ./capable -UKv
TIME UID PID COMM CAP NAME AUDIT VERDICT
01:56:37 0 730 irqbalance 21 CAP_SYS_ADMIN 0 deny
#0 0xffffffff81447dc6 cap_vm_enough_memory+0x26
#1 0xffffffff8144a94f security_vm_enough_memory_mm+0x2f
#2 0xffffffff812576e3 mmap_region+0x103
#3 0xffffffff8125837e do_mmap+0x3de
#4 0xffffffff8122c41c vm_mmap_pgoff+0xdc
#5 0xffffffff81dc3be0 do_syscall_64+0x50
#6 0xffffffff81e0011b entry_SYSCALL_64_after_hwframe+0x63
#7 0x00007f3036e9e9ca mmap64+0xa (/lib/x86_64-linux-gnu/libc-2.19.so+0xf49ca)
- irqbalance (730)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For binary paths that form a lookup key for uprobes, we truncate and affix a random alphanumeric string, such that the key "works" in upstream code (where keys that are too long fail) and remains discoverable in the BCC wrapper by mapped lookup. This patch fixes a bug where the alphanumeric suffix was not being generated.