2022-04-22 20:50:52

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH v2 bpf 00/11] bpf: random unpopular userspace fixes (32 bit et al)

This mostly issues the cross build (1) errors for 32 bit (2)
MIPS (3) with minimal configuration (4) on Musl (5). The majority
of them aren't yesterday's, so it is a "who does need it outside
of x86_64 or ARM64?" moment again.
Trivial stuff in general, not counting the first three (they are
50/50).

From v1[0]:
- use *___local struct definitions for BPF programs instead of
BTF_TYPE_EMIT() and ifdef-play (Andrii);
- cast uin64_t to unsigned long long to *really* fix the format
literal warnings (Song, David, Andrii);
- collect Acked-bys for the rest (Maciej, Kumar, Song);
- adjust the subjects to match their usual look (Andrii);
- expand the commit messages a bit for 0008
(-Wshift-count-overflow) and 0010 (-Wsequence-point) a bit to
mention they actually mitigate the third-party issues (Andrii);
- rebase and send to bpf instead of bpf-next (hope the first three
are okay for it).

[0] https://lore.kernel.org/bpf/[email protected]

Alexander Lobakin (11):
bpftool: use a local copy of perf_event to fix accessing ::bpf_cookie
bpftool: define a local bpf_perf_link to fix accessing its fields
bpftool: use a local bpf_perf_event_value to fix accessing its fields
bpftool: fix fcntl.h include
samples/bpf: add 'asm/mach-generic' include path for every MIPS
samples/bpf: use host bpftool to generate vmlinux.h, not target
samples/bpf: fix uin64_t format literals
samples/bpf: fix false-positive right-shift underflow warnings
samples/bpf: fix include order for non-Glibc environments
samples/bpf: fix -Wsequence-point
samples/bpf: xdpsock: fix -Wmaybe-uninitialized

samples/bpf/Makefile | 7 +++---
samples/bpf/cookie_uid_helper_example.c | 12 +++++-----
samples/bpf/lathist_kern.c | 2 +-
samples/bpf/lwt_len_hist_kern.c | 2 +-
samples/bpf/lwt_len_hist_user.c | 7 +++---
samples/bpf/task_fd_query_user.c | 2 +-
samples/bpf/test_lru_dist.c | 3 ++-
samples/bpf/tracex2_kern.c | 2 +-
samples/bpf/xdpsock_user.c | 5 +++--
tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 15 ++++++++++---
tools/bpf/bpftool/skeleton/profiler.bpf.c | 27 ++++++++++++++---------
tools/bpf/bpftool/tracelog.c | 2 +-
12 files changed, 53 insertions(+), 33 deletions(-)

--
2.36.0



2022-04-22 21:43:54

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH v2 bpf 07/11] samples/bpf: fix uin64_t format literals

There's a couple places where uin64_t is being passed as an %lu
format argument. That type is defined as unsigned long on 64-bit
systems and as unsigned long long on 32-bit, so neither %lu nor
%llu are not universal.
One of the options is %PRIu64, but since it's always 8-byte long,
just cast it to the _proper_ __u64 and print as %llu.

Fixes: 51570a5ab2b7 ("A Sample of using socket cookie and uid for traffic monitoring")
Fixes: 00f660eaf378 ("Sample program using SO_COOKIE")
Signed-off-by: Alexander Lobakin <[email protected]>
---
samples/bpf/cookie_uid_helper_example.c | 12 ++++++------
samples/bpf/lwt_len_hist_user.c | 7 ++++---
2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/samples/bpf/cookie_uid_helper_example.c b/samples/bpf/cookie_uid_helper_example.c
index f0df3dda4b1f..269fac58fd5c 100644
--- a/samples/bpf/cookie_uid_helper_example.c
+++ b/samples/bpf/cookie_uid_helper_example.c
@@ -207,9 +207,9 @@ static void print_table(void)
error(1, errno, "fail to get entry value of Key: %u\n",
curN);
} else {
- printf("cookie: %u, uid: 0x%x, Packet Count: %lu,"
- " Bytes Count: %lu\n", curN, curEntry.uid,
- curEntry.packets, curEntry.bytes);
+ printf("cookie: %u, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n",
+ curN, curEntry.uid, (__u64)curEntry.packets,
+ (__u64)curEntry.bytes);
}
}
}
@@ -265,9 +265,9 @@ static void udp_client(void)
if (res < 0)
error(1, errno, "lookup sk stat failed, cookie: %lu\n",
cookie);
- printf("cookie: %lu, uid: 0x%x, Packet Count: %lu,"
- " Bytes Count: %lu\n\n", cookie, dataEntry.uid,
- dataEntry.packets, dataEntry.bytes);
+ printf("cookie: %llu, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n\n",
+ (__u64)cookie, dataEntry.uid, (__u64)dataEntry.packets,
+ (__u64)dataEntry.bytes);
}
close(s_send);
close(s_rcv);
diff --git a/samples/bpf/lwt_len_hist_user.c b/samples/bpf/lwt_len_hist_user.c
index 430a4b7e353e..c682faa75a2b 100644
--- a/samples/bpf/lwt_len_hist_user.c
+++ b/samples/bpf/lwt_len_hist_user.c
@@ -44,7 +44,8 @@ int main(int argc, char **argv)

while (bpf_map_get_next_key(map_fd, &key, &next_key) == 0) {
if (next_key >= MAX_INDEX) {
- fprintf(stderr, "Key %lu out of bounds\n", next_key);
+ fprintf(stderr, "Key %llu out of bounds\n",
+ (__u64)next_key);
continue;
}

@@ -66,8 +67,8 @@ int main(int argc, char **argv)

for (i = 1; i <= max_key + 1; i++) {
stars(starstr, data[i - 1], max_value, MAX_STARS);
- printf("%8ld -> %-8ld : %-8ld |%-*s|\n",
- (1l << i) >> 1, (1l << i) - 1, data[i - 1],
+ printf("%8ld -> %-8ld : %-8lld |%-*s|\n",
+ (1l << i) >> 1, (1l << i) - 1, (__u64)data[i - 1],
MAX_STARS, starstr);
}

--
2.36.0


2022-04-22 22:03:24

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH v2 bpf 08/11] samples/bpf: fix false-positive right-shift underflow warnings

On 32 bit systems, shifting an unsigned long by 32 positions
yields the following warning:

samples/bpf/tracex2_kern.c:60:23: warning: shift count >= width of type [-Wshift-count-overflow]
unsigned int hi = v >> 32;
^ ~~

sizeof(long) is always 8 for the BPF architecture, so this is not
correct, but the BPF samples Makefile still uses the Clang native +
LLC combo which enforces that.
Until the samples are switched to `-target bpf`, do it the usual
way: shift by 16 two times (see upper_32_bits() macro in the
kernel).

Fixes: d822a1926849 ("samples/bpf: Add counting example for kfree_skb() function calls and the write() syscall")
Fixes: 0fb1170ee68a ("bpf: BPF based latency tracing")
Fixes: f74599f7c530 ("bpf: Add tests and samples for LWT-BPF")
Acked-by: Song Liu <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
samples/bpf/lathist_kern.c | 2 +-
samples/bpf/lwt_len_hist_kern.c | 2 +-
samples/bpf/tracex2_kern.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/samples/bpf/lathist_kern.c b/samples/bpf/lathist_kern.c
index 4adfcbbe6ef4..9744ed547abe 100644
--- a/samples/bpf/lathist_kern.c
+++ b/samples/bpf/lathist_kern.c
@@ -53,7 +53,7 @@ static unsigned int log2(unsigned int v)

static unsigned int log2l(unsigned long v)
{
- unsigned int hi = v >> 32;
+ unsigned int hi = (v >> 16) >> 16;

if (hi)
return log2(hi) + 32;
diff --git a/samples/bpf/lwt_len_hist_kern.c b/samples/bpf/lwt_len_hist_kern.c
index 1fa14c54963a..bf32fa04c91f 100644
--- a/samples/bpf/lwt_len_hist_kern.c
+++ b/samples/bpf/lwt_len_hist_kern.c
@@ -49,7 +49,7 @@ static unsigned int log2(unsigned int v)

static unsigned int log2l(unsigned long v)
{
- unsigned int hi = v >> 32;
+ unsigned int hi = (v >> 16) >> 16;
if (hi)
return log2(hi) + 32;
else
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index 5bc696bac27d..6bf22056ff95 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -57,7 +57,7 @@ static unsigned int log2(unsigned int v)

static unsigned int log2l(unsigned long v)
{
- unsigned int hi = v >> 32;
+ unsigned int hi = (v >> 16) >> 16;
if (hi)
return log2(hi) + 32;
else
--
2.36.0


2022-04-22 22:41:49

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH v2 bpf 05/11] samples/bpf: add 'asm/mach-generic' include path for every MIPS

Fix the following:

In file included from samples/bpf/tracex2_kern.c:7:
In file included from ./include/linux/skbuff.h:13:
In file included from ./include/linux/kernel.h:22:
In file included from ./include/linux/bitops.h:33:
In file included from ./arch/mips/include/asm/bitops.h:20:
In file included from ./arch/mips/include/asm/barrier.h:11:
./arch/mips/include/asm/addrspace.h:13:10: fatal error: 'spaces.h' file not found
#include <spaces.h>
^~~~~~~~~~

'arch/mips/include/asm/mach-generic' should always be included as
many other MIPS include files rely on this.
Move it from under CONFIG_MACH_LOONGSON64 to let it be included
for every MIPS.

Fixes: 058107abafc7 ("samples/bpf: Add include dir for MIPS Loongson64 to fix build errors")
Acked-by: Song Liu <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
samples/bpf/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 38638845db9d..70323ac1114f 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -194,8 +194,8 @@ ifeq ($(ARCH), mips)
TPROGS_CFLAGS += -D__SANE_USERSPACE_TYPES__
ifdef CONFIG_MACH_LOONGSON64
BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-loongson64
-BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-generic
endif
+BPF_EXTRA_CFLAGS += -I$(srctree)/arch/mips/include/asm/mach-generic
endif

TPROGS_CFLAGS += -Wall -O2
--
2.36.0