| Message ID | 1648654000-21758-1-git-send-email-alan.maguire@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | libbpf: name-based u[ret]probe attach | expand |
On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > This patch series focuses on supporting name-based attach - similar > to that supported for kprobes - for uprobe BPF programs. > > Currently attach for such probes is done by determining the offset > manually, so the aim is to try and mimic the simplicity of kprobe > attach, making use of uprobe opts to specify a name string. > Patch 1 supports expansion of the binary_path argument used for > bpf_program__attach_uprobe_opts(), allowing it to determine paths > for programs and shared objects automatically, allowing for > specification of "libc.so.6" rather than the full path > "/usr/lib64/libc.so.6". > > Patch 2 adds the "func_name" option to allow uprobe attach by > name; the mechanics are described there. > > Having name-based support allows us to support auto-attach for > uprobes; patch 3 adds auto-attach support while attempting > to handle backwards-compatibility issues that arise. The format > supported is > > u[ret]probe/binary_path:[raw_offset|function[+offset]] > > For example, to attach to libc malloc: > > SEC("uprobe//usr/lib64/libc.so.6:malloc") > > ..or, making use of the path computation mechanisms introduced in patch 1 > > SEC("uprobe/libc.so.6:malloc") > > Finally patch 4 add tests to the attach_probe selftests covering > attach by name, with patch 5 covering skeleton auto-attach. > > Changes since v4 [1]: > - replaced strtok_r() usage with copying segments from static char *; avoids > unneeded string allocation (Andrii, patch 1) > - switched to using access() instead of stat() when checking path-resolved > binary (Andrii, patch 1) > - removed computation of .plt offset for instrumenting shared library calls > within binaries. Firstly it proved too brittle, and secondly it was somewhat > unintuitive in that this form of instrumentation did not support function+offset > as the "local function in binary" and "shared library function in shared library" > cases did. We can still instrument library calls, just need to do it in the > library .so (patch 2) ah, that's too bad, it seemed like a nice and clever idea. What was brittle? Curious to learn for the future. The fact that function+offset isn't supported int this "mode" seems totally fine to me, we can provide a nice descriptive error in this case anyways. Anyways, all the added functionality makes sense and we can further improve on it if necessary and possible. I've found a few small issues with your patches and fixed some of them while applying, please do a follow up for the rest. Thanks, great work and great addition to libbpf! > - added binary path logging in cases where it was missing (Andrii, patch 2) > - avoid strlen() calcuation in checking name match (Andrii, patch 2) > - reword comments for func_name option (Andrii, patch 2) > - tightened SEC() name validation to support "u[ret]probe" and fail on other > permutations that do not support auto-attach (i.e. have u[ret]probe/binary_path:func > format (Andrii, patch 3) > - fixed selftests to fail independently rather than skip remainder on failure > (Andrii, patches 4,5) > Changes since v3 [2]: > - reworked variable naming to fit better with libbpf conventions > (Andrii, patch 2) > - use quoted binary path in log messages (Andrii, patch 2) > - added path determination mechanisms using LD_LIBRARY_PATH/PATH and > standard locations (patch 1, Andrii) > - changed section lookup to be type+name (if name is specified) to > simplify use cases (patch 2, Andrii) > - fixed .plt lookup scheme to match symbol table entries with .plt > index via the .rela.plt table; also fix the incorrect assumption > that the code in the .plt that does library linking is the same > size as .plt entries (it just happens to be on x86_64) > - aligned with pluggable section support such that uprobe SEC() names > that do not conform to auto-attach format do not cause skeleton load > failure (patch 3, Andrii) > - no longer need to look up absolute path to libraries used by test_progs > since we have mechanism to determine path automatically > - replaced CHECK()s with ASSERT*()s for attach_probe test (Andrii, patch 4) > - added auto-attach selftests also (Andrii, patch 5) > Changes since RFC [3]: > - used "long" for addresses instead of ssize_t (Andrii, patch 1). > - used gelf_ interfaces to avoid assumptions about 64-bit > binaries (Andrii, patch 1) > - clarified string matching in symbol table lookups > (Andrii, patch 1) > - added support for specification of shared object functions > in a non-shared object binary. This approach instruments > the Procedure Linking Table (PLT) - malloc@PLT. > - changed logic in symbol search to check dynamic symbol table > first, then fall back to symbol table (Andrii, patch 1). > - modified auto-attach string to require "/" separator prior > to path prefix i.e. uprobe//path/to/binary (Andrii, patch 2) > - modified auto-attach string to use ':' separator (Andrii, > patch 2) > - modified auto-attach to support raw offset (Andrii, patch 2) > - modified skeleton attach to interpret -ESRCH errors as > a non-fatal "unable to auto-attach" (Andrii suggested > -EOPNOTSUPP but my concern was it might collide with other > instances where that value is returned and reflects a > failure to attach a to-be-expected attachment rather than > skip a program that does not present an auto-attachable > section name. Admittedly -EOPNOTSUPP seems a more natural > value here). > - moved library path retrieval code to trace_helpers (Andrii, > patch 3) > > [1] https://lore.kernel.org/bpf/1647000658-16149-1-git-send-email-alan.maguire@oracle.com/ > [2] https://lore.kernel.org/bpf/1643645554-28723-1-git-send-email-alan.maguire@oracle.com/ > [3] https://lore.kernel.org/bpf/1642678950-19584-1-git-send-email-alan.maguire@oracle.com/ > > Alan Maguire (5): > libbpf: bpf_program__attach_uprobe_opts() should determine paths for > programs/libraries where possible > libbpf: support function name-based attach uprobes > libbpf: add auto-attach for uprobes based on section name > selftests/bpf: add tests for u[ret]probe attach by name > selftests/bpf: add tests for uprobe auto-attach via skeleton > > tools/lib/bpf/libbpf.c | 330 ++++++++++++++++++++- > tools/lib/bpf/libbpf.h | 10 +- > .../selftests/bpf/prog_tests/attach_probe.c | 85 +++++- > .../selftests/bpf/prog_tests/uprobe_autoattach.c | 38 +++ > .../selftests/bpf/progs/test_attach_probe.c | 41 ++- > .../selftests/bpf/progs/test_uprobe_autoattach.c | 52 ++++ > 6 files changed, 535 insertions(+), 21 deletions(-) > create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c > create mode 100644 tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c > > -- > 1.8.3.1 >
Hello: This series was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Wed, 30 Mar 2022 16:26:35 +0100 you wrote: > This patch series focuses on supporting name-based attach - similar > to that supported for kprobes - for uprobe BPF programs. > > Currently attach for such probes is done by determining the offset > manually, so the aim is to try and mimic the simplicity of kprobe > attach, making use of uprobe opts to specify a name string. > Patch 1 supports expansion of the binary_path argument used for > bpf_program__attach_uprobe_opts(), allowing it to determine paths > for programs and shared objects automatically, allowing for > specification of "libc.so.6" rather than the full path > "/usr/lib64/libc.so.6". > > [...] Here is the summary with links: - [v5,bpf-next,1/5] libbpf: bpf_program__attach_uprobe_opts() should determine paths for programs/libraries where possible https://git.kernel.org/bpf/bpf-next/c/1ce3a60e3c28 - [v5,bpf-next,2/5] libbpf: support function name-based attach uprobes https://git.kernel.org/bpf/bpf-next/c/433966e3ae04 - [v5,bpf-next,3/5] libbpf: add auto-attach for uprobes based on section name https://git.kernel.org/bpf/bpf-next/c/7825470420d1 - [v5,bpf-next,4/5] selftests/bpf: add tests for u[ret]probe attach by name https://git.kernel.org/bpf/bpf-next/c/5bc2f0c51181 - [v5,bpf-next,5/5] selftests/bpf: add tests for uprobe auto-attach via skeleton https://git.kernel.org/bpf/bpf-next/c/948ef31c4cd9 You are awesome, thank you!
On Mon, 4 Apr 2022, Andrii Nakryiko wrote: > On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > This patch series focuses on supporting name-based attach - similar > > to that supported for kprobes - for uprobe BPF programs. > > > > Currently attach for such probes is done by determining the offset > > manually, so the aim is to try and mimic the simplicity of kprobe > > attach, making use of uprobe opts to specify a name string. > > Patch 1 supports expansion of the binary_path argument used for > > bpf_program__attach_uprobe_opts(), allowing it to determine paths > > for programs and shared objects automatically, allowing for > > specification of "libc.so.6" rather than the full path > > "/usr/lib64/libc.so.6". > > > > Patch 2 adds the "func_name" option to allow uprobe attach by > > name; the mechanics are described there. > > > > Having name-based support allows us to support auto-attach for > > uprobes; patch 3 adds auto-attach support while attempting > > to handle backwards-compatibility issues that arise. The format > > supported is > > > > u[ret]probe/binary_path:[raw_offset|function[+offset]] > > > > For example, to attach to libc malloc: > > > > SEC("uprobe//usr/lib64/libc.so.6:malloc") > > > > ..or, making use of the path computation mechanisms introduced in patch 1 > > > > SEC("uprobe/libc.so.6:malloc") > > > > Finally patch 4 add tests to the attach_probe selftests covering > > attach by name, with patch 5 covering skeleton auto-attach. > > > > Changes since v4 [1]: > > - replaced strtok_r() usage with copying segments from static char *; avoids > > unneeded string allocation (Andrii, patch 1) > > - switched to using access() instead of stat() when checking path-resolved > > binary (Andrii, patch 1) > > - removed computation of .plt offset for instrumenting shared library calls > > within binaries. Firstly it proved too brittle, and secondly it was somewhat > > unintuitive in that this form of instrumentation did not support function+offset > > as the "local function in binary" and "shared library function in shared library" > > cases did. We can still instrument library calls, just need to do it in the > > library .so (patch 2) > > ah, that's too bad, it seemed like a nice and clever idea. What was > brittle? Curious to learn for the future. > On Ubuntu, Daniel reported selftest failures which corresponded to the cases where we attached to a library function in a non-library - i.e. used the .plt computations. I reproduced this failure myself, and it seemed that although we were correctly computing the size of the .plt initial code - 16 bytes - and each .plt entry - again 16 bytes - finding the .rela.plt entry and using its index as the basis for figuring out which .plt entry to instrument wasn't working. Specifically, the .rela.plt entry for "free" was 146, but the actual .plt location of free was the 372 entry in the .plt table. I'm not clear on why, but the the correspondence betweeen .rela.plt and .plt order isn't present on Ubuntu. > The fact that function+offset isn't supported int this "mode" seems > totally fine to me, we can provide a nice descriptive error in this > case anyways. > I'll try and figure out exactly what's going on above; would be nice if we can still add this in the future. > Anyways, all the added functionality makes sense and we can further > improve on it if necessary and possible. I've found a few small issues > with your patches and fixed some of them while applying, please do a > follow up for the rest. Yep, will do, thanks for the fix-ups! Ilya has fixed a few of the issues too, so I'll have some follow-ups for the rest shortly. I'll take a look at adding aarch64 to libbpf CI too, that would be great. > Thanks, great work and great addition to > libbpf! > Thanks again!
On Tue, Apr 5, 2022 at 3:28 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > On Mon, 4 Apr 2022, Andrii Nakryiko wrote: > > > On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > > > This patch series focuses on supporting name-based attach - similar > > > to that supported for kprobes - for uprobe BPF programs. > > > > > > Currently attach for such probes is done by determining the offset > > > manually, so the aim is to try and mimic the simplicity of kprobe > > > attach, making use of uprobe opts to specify a name string. > > > Patch 1 supports expansion of the binary_path argument used for > > > bpf_program__attach_uprobe_opts(), allowing it to determine paths > > > for programs and shared objects automatically, allowing for > > > specification of "libc.so.6" rather than the full path > > > "/usr/lib64/libc.so.6". > > > > > > Patch 2 adds the "func_name" option to allow uprobe attach by > > > name; the mechanics are described there. > > > > > > Having name-based support allows us to support auto-attach for > > > uprobes; patch 3 adds auto-attach support while attempting > > > to handle backwards-compatibility issues that arise. The format > > > supported is > > > > > > u[ret]probe/binary_path:[raw_offset|function[+offset]] > > > > > > For example, to attach to libc malloc: > > > > > > SEC("uprobe//usr/lib64/libc.so.6:malloc") > > > > > > ..or, making use of the path computation mechanisms introduced in patch 1 > > > > > > SEC("uprobe/libc.so.6:malloc") > > > > > > Finally patch 4 add tests to the attach_probe selftests covering > > > attach by name, with patch 5 covering skeleton auto-attach. > > > > > > Changes since v4 [1]: > > > - replaced strtok_r() usage with copying segments from static char *; avoids > > > unneeded string allocation (Andrii, patch 1) > > > - switched to using access() instead of stat() when checking path-resolved > > > binary (Andrii, patch 1) > > > - removed computation of .plt offset for instrumenting shared library calls > > > within binaries. Firstly it proved too brittle, and secondly it was somewhat > > > unintuitive in that this form of instrumentation did not support function+offset > > > as the "local function in binary" and "shared library function in shared library" > > > cases did. We can still instrument library calls, just need to do it in the > > > library .so (patch 2) > > > > ah, that's too bad, it seemed like a nice and clever idea. What was > > brittle? Curious to learn for the future. > > > > On Ubuntu, Daniel reported selftest failures which corresponded to the > cases where we attached to a library function in a non-library - i.e. used > the .plt computations. I reproduced this failure myself, and it seemed > that although we were correctly computing the size of the .plt initial > code - 16 bytes - and each .plt entry - again 16 bytes - finding the > .rela.plt entry and using its index as the basis for figuring out which > .plt entry to instrument wasn't working. > > Specifically, the .rela.plt entry for "free" was 146, but the actual .plt > location of free was the 372 entry in the .plt table. I'm not clear on > why, but the the correspondence betweeen .rela.plt and .plt order > isn't present on Ubuntu. Ok, curious. I'm not a big expert on PLT and stuff, but would be curious to look at such an ELF file and see if we are missing something that can be recovered from PLT relocations maybe? If you happen to have a small ELF with repro case, please share. > > > The fact that function+offset isn't supported int this "mode" > seems > > totally fine to me, we can provide a nice descriptive error in this > > case anyways. > > > > I'll try and figure out exactly what's going on above; would be nice if we > can still add this in the future. Yep, definitely, provided we are sure it will keep working reliably :) > > > Anyways, all the added functionality makes sense and we can further > > improve on it if necessary and possible. I've found a few small issues > > with your patches and fixed some of them while applying, please do a > > follow up for the rest. > > Yep, will do, thanks for the fix-ups! Ilya has fixed a few of the issues > too, so I'll have some follow-ups for the rest shortly. I'll take a look > at adding aarch64 to libbpf CI too, that would be great. Awesome, aarch64 seems like a logical addition, thanks! > > > Thanks, great work and great addition to > > libbpf! > > > > Thanks again!
This patch series focuses on supporting name-based attach - similar to that supported for kprobes - for uprobe BPF programs. Currently attach for such probes is done by determining the offset manually, so the aim is to try and mimic the simplicity of kprobe attach, making use of uprobe opts to specify a name string. Patch 1 supports expansion of the binary_path argument used for bpf_program__attach_uprobe_opts(), allowing it to determine paths for programs and shared objects automatically, allowing for specification of "libc.so.6" rather than the full path "/usr/lib64/libc.so.6". Patch 2 adds the "func_name" option to allow uprobe attach by name; the mechanics are described there. Having name-based support allows us to support auto-attach for uprobes; patch 3 adds auto-attach support while attempting to handle backwards-compatibility issues that arise. The format supported is u[ret]probe/binary_path:[raw_offset|function[+offset]] For example, to attach to libc malloc: SEC("uprobe//usr/lib64/libc.so.6:malloc") ..or, making use of the path computation mechanisms introduced in patch 1 SEC("uprobe/libc.so.6:malloc") Finally patch 4 add tests to the attach_probe selftests covering attach by name, with patch 5 covering skeleton auto-attach. Changes since v4 [1]: - replaced strtok_r() usage with copying segments from static char *; avoids unneeded string allocation (Andrii, patch 1) - switched to using access() instead of stat() when checking path-resolved binary (Andrii, patch 1) - removed computation of .plt offset for instrumenting shared library calls within binaries. Firstly it proved too brittle, and secondly it was somewhat unintuitive in that this form of instrumentation did not support function+offset as the "local function in binary" and "shared library function in shared library" cases did. We can still instrument library calls, just need to do it in the library .so (patch 2) - added binary path logging in cases where it was missing (Andrii, patch 2) - avoid strlen() calcuation in checking name match (Andrii, patch 2) - reword comments for func_name option (Andrii, patch 2) - tightened SEC() name validation to support "u[ret]probe" and fail on other permutations that do not support auto-attach (i.e. have u[ret]probe/binary_path:func format (Andrii, patch 3) - fixed selftests to fail independently rather than skip remainder on failure (Andrii, patches 4,5) Changes since v3 [2]: - reworked variable naming to fit better with libbpf conventions (Andrii, patch 2) - use quoted binary path in log messages (Andrii, patch 2) - added path determination mechanisms using LD_LIBRARY_PATH/PATH and standard locations (patch 1, Andrii) - changed section lookup to be type+name (if name is specified) to simplify use cases (patch 2, Andrii) - fixed .plt lookup scheme to match symbol table entries with .plt index via the .rela.plt table; also fix the incorrect assumption that the code in the .plt that does library linking is the same size as .plt entries (it just happens to be on x86_64) - aligned with pluggable section support such that uprobe SEC() names that do not conform to auto-attach format do not cause skeleton load failure (patch 3, Andrii) - no longer need to look up absolute path to libraries used by test_progs since we have mechanism to determine path automatically - replaced CHECK()s with ASSERT*()s for attach_probe test (Andrii, patch 4) - added auto-attach selftests also (Andrii, patch 5) Changes since RFC [3]: - used "long" for addresses instead of ssize_t (Andrii, patch 1). - used gelf_ interfaces to avoid assumptions about 64-bit binaries (Andrii, patch 1) - clarified string matching in symbol table lookups (Andrii, patch 1) - added support for specification of shared object functions in a non-shared object binary. This approach instruments the Procedure Linking Table (PLT) - malloc@PLT. - changed logic in symbol search to check dynamic symbol table first, then fall back to symbol table (Andrii, patch 1). - modified auto-attach string to require "/" separator prior to path prefix i.e. uprobe//path/to/binary (Andrii, patch 2) - modified auto-attach string to use ':' separator (Andrii, patch 2) - modified auto-attach to support raw offset (Andrii, patch 2) - modified skeleton attach to interpret -ESRCH errors as a non-fatal "unable to auto-attach" (Andrii suggested -EOPNOTSUPP but my concern was it might collide with other instances where that value is returned and reflects a failure to attach a to-be-expected attachment rather than skip a program that does not present an auto-attachable section name. Admittedly -EOPNOTSUPP seems a more natural value here). - moved library path retrieval code to trace_helpers (Andrii, patch 3) [1] https://lore.kernel.org/bpf/1647000658-16149-1-git-send-email-alan.maguire@oracle.com/ [2] https://lore.kernel.org/bpf/1643645554-28723-1-git-send-email-alan.maguire@oracle.com/ [3] https://lore.kernel.org/bpf/1642678950-19584-1-git-send-email-alan.maguire@oracle.com/ Alan Maguire (5): libbpf: bpf_program__attach_uprobe_opts() should determine paths for programs/libraries where possible libbpf: support function name-based attach uprobes libbpf: add auto-attach for uprobes based on section name selftests/bpf: add tests for u[ret]probe attach by name selftests/bpf: add tests for uprobe auto-attach via skeleton tools/lib/bpf/libbpf.c | 330 ++++++++++++++++++++- tools/lib/bpf/libbpf.h | 10 +- .../selftests/bpf/prog_tests/attach_probe.c | 85 +++++- .../selftests/bpf/prog_tests/uprobe_autoattach.c | 38 +++ .../selftests/bpf/progs/test_attach_probe.c | 41 ++- .../selftests/bpf/progs/test_uprobe_autoattach.c | 52 ++++ 6 files changed, 535 insertions(+), 21 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c create mode 100644 tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c