From 8d44c3578b48d5f605eddcfd6a644e3944455a6b Mon Sep 17 00:00:00 2001 From: Alan Maguire Date: Fri, 16 Jul 2021 23:46:55 +0100 Subject: [PATCH 1/3] libbpf: Clarify/fix unaligned data issues for btf typed dump If data is packed, data structures can store it outside of usual boundaries. For example a 4-byte int can be stored on a unaligned boundary in a case like this: struct s { char f1; int f2; } __attribute((packed)); ...the int is stored at an offset of one byte. Some platforms have problems dereferencing data that is not aligned with its size, and code exists to handle most cases of this for BTF typed data display. However pointer display was missed, and a simple function to test if "ptr_is_aligned(data, data_sz)" would help clarify this code. Suggested-by: Andrii Nakryiko Signed-off-by: Alan Maguire Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/1626475617-25984-2-git-send-email-alan.maguire@oracle.com --- tools/lib/bpf/btf_dump.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index 929cf931ed66..bf5bb4b127ed 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -1654,6 +1654,11 @@ static int btf_dump_base_type_check_zero(struct btf_dump *d, return 0; } +static bool ptr_is_aligned(const void *data, int data_sz) +{ + return ((uintptr_t)data) % data_sz == 0; +} + static int btf_dump_int_data(struct btf_dump *d, const struct btf_type *t, __u32 type_id, @@ -1672,7 +1677,7 @@ static int btf_dump_int_data(struct btf_dump *d, /* handle packed int data - accesses of integers not aligned on * int boundaries can cause problems on some platforms. */ - if (((uintptr_t)data) % sz) + if (!ptr_is_aligned(data, sz)) return btf_dump_bitfield_data(d, t, data, 0, 0); switch (sz) { @@ -1739,7 +1744,7 @@ static int btf_dump_float_data(struct btf_dump *d, int sz = t->size; /* handle unaligned data; copy to local union */ - if (((uintptr_t)data) % sz) { + if (!ptr_is_aligned(data, sz)) { memcpy(&fl, data, sz); flp = &fl; } @@ -1892,12 +1897,27 @@ static int btf_dump_struct_data(struct btf_dump *d, return err; } +union ptr_data { + unsigned int p; + unsigned long long lp; +}; + static int btf_dump_ptr_data(struct btf_dump *d, const struct btf_type *t, __u32 id, const void *data) { - btf_dump_type_values(d, "%p", *(void **)data); + if (ptr_is_aligned(data, d->ptr_sz) && d->ptr_sz == sizeof(void *)) { + btf_dump_type_values(d, "%p", *(void **)data); + } else { + union ptr_data pt; + + memcpy(&pt, data, d->ptr_sz); + if (d->ptr_sz == 4) + btf_dump_type_values(d, "0x%x", pt.p); + else + btf_dump_type_values(d, "0x%llx", pt.lp); + } return 0; } @@ -1910,7 +1930,7 @@ static int btf_dump_get_enum_value(struct btf_dump *d, int sz = t->size; /* handle unaligned enum value */ - if (((uintptr_t)data) % sz) { + if (!ptr_is_aligned(data, sz)) { *value = (__s64)btf_dump_bitfield_get_data(d, t, data, 0, 0); return 0; } From 04eb4dff6a64d842f7f2c85c7cb1affc5ab3ebc9 Mon Sep 17 00:00:00 2001 From: Alan Maguire Date: Fri, 16 Jul 2021 23:46:56 +0100 Subject: [PATCH 2/3] libbpf: Fix compilation errors on ppc64le for btf dump typed data __s64 can be defined as either long or long long, depending on the architecture. On ppc64le it's defined as long, giving this error: In file included from btf_dump.c:22: btf_dump.c: In function 'btf_dump_type_data_check_overflow': libbpf_internal.h:111:22: error: format '%lld' expects argument of type 'long long int', but argument 3 has type '__s64' {aka 'long int'} [-Werror=format=] 111 | libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \ | ^~~~~~~~~~ libbpf_internal.h:114:27: note: in expansion of macro '__pr' 114 | #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__) | ^~~~ btf_dump.c:1992:3: note: in expansion of macro 'pr_warn' 1992 | pr_warn("unexpected size [%lld] for id [%u]\n", | ^~~~~~~ btf_dump.c:1992:32: note: format string is defined here 1992 | pr_warn("unexpected size [%lld] for id [%u]\n", | ~~~^ | | | long long int | %ld Cast to size_t and use %zu instead. Reported-by: Andrii Nakryiko Signed-off-by: Alan Maguire Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/1626475617-25984-3-git-send-email-alan.maguire@oracle.com --- tools/lib/bpf/btf_dump.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index bf5bb4b127ed..aa695ab9b826 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -2009,8 +2009,8 @@ static int btf_dump_type_data_check_overflow(struct btf_dump *d, __s64 size = btf__resolve_size(d->btf, id); if (size < 0 || size >= INT_MAX) { - pr_warn("unexpected size [%lld] for id [%u]\n", - size, id); + pr_warn("unexpected size [%zu] for id [%u]\n", + (size_t)size, id); return -EINVAL; } From add192f81ab21b58471577c75e7be9c9add98223 Mon Sep 17 00:00:00 2001 From: Alan Maguire Date: Fri, 16 Jul 2021 23:46:57 +0100 Subject: [PATCH 3/3] libbpf: Btf typed dump does not need to allocate dump data By using the stack for this small structure, we avoid the need for freeing memory in error paths. Suggested-by: Andrii Nakryiko Signed-off-by: Alan Maguire Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/1626475617-25984-4-git-send-email-alan.maguire@oracle.com --- tools/lib/bpf/btf_dump.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index aa695ab9b826..accf6fea57da 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -2238,6 +2238,7 @@ int btf_dump__dump_type_data(struct btf_dump *d, __u32 id, const void *data, size_t data_sz, const struct btf_dump_type_data_opts *opts) { + struct btf_dump_data typed_dump = {}; const struct btf_type *t; int ret; @@ -2248,12 +2249,10 @@ int btf_dump__dump_type_data(struct btf_dump *d, __u32 id, if (!t) return libbpf_err(-ENOENT); - d->typed_dump = calloc(1, sizeof(struct btf_dump_data)); - if (!d->typed_dump) - return libbpf_err(-ENOMEM); - + d->typed_dump = &typed_dump; d->typed_dump->data_end = data + data_sz; d->typed_dump->indent_lvl = OPTS_GET(opts, indent_level, 0); + /* default indent string is a tab */ if (!opts->indent_str) d->typed_dump->indent_str[0] = '\t'; @@ -2267,7 +2266,7 @@ int btf_dump__dump_type_data(struct btf_dump *d, __u32 id, ret = btf_dump_dump_type_data(d, NULL, t, id, data, 0, 0); - free(d->typed_dump); + d->typed_dump = NULL; return libbpf_err(ret); }