On Wed, May 29, 2019 at 11:30 AM Michal Rostecki <mrostecki@xxxxxxxxxxxx> wrote:
>
> Before this change, function load_sk_storage_btf expected that
> libbpf__probe_raw_btf was returning a BTF descriptor, but in fact it was
> returning an information about whether the probe was successful (0 or
> 1). load_sk_storage_btf was using that value as an argument of the close
> function, which was resulting in closing stdout and thus terminating the
> process which called that function.
>
> That bug was visible in bpftool. `bpftool feature` subcommand was always
> exiting too early (because of closed stdout) and it didn't display all
> requested probes. `bpftool -j feature` or `bpftool -p feature` were not
> returning a valid json object.
>
> This change renames the libbpf__probe_raw_btf function to
> libbpf__load_raw_btf, which now returns a BTF descriptor, as expected in
> load_sk_storage_btf.
>
> v2:
> - Fix typo in the commit message.
>
> v3:
> - Simplify BTF descriptor handling in bpf_object__probe_btf_* functions.
> - Rename libbpf__probe_raw_btf function to libbpf__load_raw_btf and
> return a BTF descriptor.
>
> v4:
> - Fix typo in the commit message.
>
> Fixes: d7c4b3980c18 ("libbpf: detect supported kernel BTF features and sanitize BTF")
> Signed-off-by: Michal Rostecki <mrostecki@xxxxxxxxxxxx>
> Acked-by: Andrii Nakryiko <andriin@xxxxxx>
Acked-by: Song Liu <songliubraving@xxxxxx>
Thanks for the fix!
> ---
> tools/lib/bpf/libbpf.c | 28 ++++++++++++++++------------
> tools/lib/bpf/libbpf_internal.h | 4 ++--
> tools/lib/bpf/libbpf_probes.c | 13 ++++---------
> 3 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 197b574406b3..5d046cc7b207 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1645,14 +1645,16 @@ static int bpf_object__probe_btf_func(struct bpf_object *obj)
> /* FUNC x */ /* [3] */
> BTF_TYPE_ENC(5, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 0), 2),
> };
> - int res;
> + int btf_fd;
>
> - res = libbpf__probe_raw_btf((char *)types, sizeof(types),
> - strs, sizeof(strs));
> - if (res < 0)
> - return res;
> - if (res > 0)
> + btf_fd = libbpf__load_raw_btf((char *)types, sizeof(types),
> + strs, sizeof(strs));
> + if (btf_fd >= 0) {
> obj->caps.btf_func = 1;
> + close(btf_fd);
> + return 1;
> + }
> +
> return 0;
> }
>
> @@ -1670,14 +1672,16 @@ static int bpf_object__probe_btf_datasec(struct bpf_object *obj)
> BTF_TYPE_ENC(3, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 1), 4),
> BTF_VAR_SECINFO_ENC(2, 0, 4),
> };
> - int res;
> + int btf_fd;
>
> - res = libbpf__probe_raw_btf((char *)types, sizeof(types),
> - strs, sizeof(strs));
> - if (res < 0)
> - return res;
> - if (res > 0)
> + btf_fd = libbpf__load_raw_btf((char *)types, sizeof(types),
> + strs, sizeof(strs));
> + if (btf_fd >= 0) {
> obj->caps.btf_datasec = 1;
> + close(btf_fd);
> + return 1;
> + }
> +
> return 0;
> }
>
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index f3025b4d90e1..dfab8012185c 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -34,7 +34,7 @@ do { \
> #define pr_info(fmt, ...) __pr(LIBBPF_INFO, fmt, ##__VA_ARGS__)
> #define pr_debug(fmt, ...) __pr(LIBBPF_DEBUG, fmt, ##__VA_ARGS__)
>
> -int libbpf__probe_raw_btf(const char *raw_types, size_t types_len,
> - const char *str_sec, size_t str_len);
> +int libbpf__load_raw_btf(const char *raw_types, size_t types_len,
> + const char *str_sec, size_t str_len);
>
> #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index 5e2aa83f637a..6635a31a7a16 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -133,8 +133,8 @@ bool bpf_probe_prog_type(enum bpf_prog_type prog_type, __u32 ifindex)
> return errno != EINVAL && errno != EOPNOTSUPP;
> }
>
> -int libbpf__probe_raw_btf(const char *raw_types, size_t types_len,
> - const char *str_sec, size_t str_len)
> +int libbpf__load_raw_btf(const char *raw_types, size_t types_len,
> + const char *str_sec, size_t str_len)
> {
> struct btf_header hdr = {
> .magic = BTF_MAGIC,
> @@ -157,14 +157,9 @@ int libbpf__probe_raw_btf(const char *raw_types, size_t types_len,
> memcpy(raw_btf + hdr.hdr_len + hdr.type_len, str_sec, hdr.str_len);
>
> btf_fd = bpf_load_btf(raw_btf, btf_len, NULL, 0, false);
> - if (btf_fd < 0) {
> - free(raw_btf);
> - return 0;
> - }
>
> - close(btf_fd);
> free(raw_btf);
> - return 1;
> + return btf_fd;
> }
>
> static int load_sk_storage_btf(void)
> @@ -190,7 +185,7 @@ static int load_sk_storage_btf(void)
> BTF_MEMBER_ENC(23, 2, 32),/* struct bpf_spin_lock l; */
> };
>
> - return libbpf__probe_raw_btf((char *)types, sizeof(types),
> + return libbpf__load_raw_btf((char *)types, sizeof(types),
> strs, sizeof(strs));
> }
>
> --
> 2.21.0
>
