Hi Hengqi,
On Thu, Jun 30, 2022 at 7:07 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>
> ---
> 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;
> + }
Please add matching brackets for the 'then' clause.
Besides, do you need to call readlink() to extract the real path of
the binary? Reading /proc/$pid/exe may fail due to not being root.
Detecting such failures early and giving a warning would be great.
> + } 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
>
