On Thu, Jun 30, 2022 at 6:53 AM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote:
>
> Currently, libbpf requires specifying binary path when attach USDT BPF program
> manually. This is not necessary because we can infer that from /proc/$PID/exe.
> This also avoids coredump when user do not provide binary path.
>
> Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>
> ---
Hm... just because I specify PID, doesn't mean I mean main binary of
that process, it could be some other shared library within that
process.
I don't really like this change because it doesn't feel 100% obvious
and natural. User can easily specify "/proc/self/exe" or
"/proc/<pid>/exe" if that's what they are targeting.
Documentation for bpf_program__attach_usdt() states
@param binary_path Path to binary that contains provided USDT probe
it doesn't say it is optional and can be NULL, so there is no valid
reason to pass NULL and expect things to work.
> tools/lib/bpf/libbpf.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8a45a84eb9b2..4ee9b6a0944e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10686,7 +10686,19 @@ struct bpf_link *bpf_program__attach_usdt(const struct bpf_program *prog,
> return libbpf_err_ptr(-EINVAL);
> }
>
> - if (!strchr(binary_path, '/')) {
> + if (!binary_path) {
> + if (pid < 0) {
> + pr_warn("prog '%s': missing attach target, pid or binary path required\n",
> + prog->name);
> + return libbpf_err_ptr(-EINVAL);
> + }
> + if (!pid)
> + binary_path = "/proc/self/exe";
> + else {
> + snprintf(resolved_path, sizeof(resolved_path), "/proc/%d/exe", pid);
> + binary_path = resolved_path;
> + }
> + } else if (!strchr(binary_path, '/')) {
> err = resolve_full_path(binary_path, resolved_path, sizeof(resolved_path));
> if (err) {
> pr_warn("prog '%s': failed to resolve full path for '%s': %d\n",
> --
> 2.30.2
>
