Eduard Zingerman [Thu, 22 Aug 2024 08:01:23 +0000 (01:01 -0700)]
bpf: correctly handle malformed BPF_CORE_TYPE_ID_LOCAL relos
In case of malformed relocation record of kind BPF_CORE_TYPE_ID_LOCAL
referencing a non-existing BTF type, function bpf_core_calc_relo_insn
would cause a null pointer deference.
Fix this by adding a proper check upper in call stack, as malformed
relocation records could be passed from user space.
Simplest reproducer is a program:
r0 = 0
exit
With a single relocation record:
.insn_off = 0, /* patch first instruction */
.type_id = 100500, /* this type id does not exist */
.access_str_off = 6, /* offset of string "0" */
.kind = BPF_CORE_TYPE_ID_LOCAL,
See the link for original reproducer or next commit for a test case.
Fixes:
74753e1462e7 ("libbpf: Replace btf__type_by_id() with btf_type_by_id().")
Reported-by: Liu RuiTong <cnitlrt@gmail.com>
Closes: https://lore.kernel.org/bpf/CAK55_s6do7C+DVwbwY_7nKfUz0YLDoiA1v6X3Y9+p0sWzipFSA@mail.gmail.com/
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240822080124.2995724-2-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Yu Jiaoliang [Wed, 21 Aug 2024 07:37:08 +0000 (15:37 +0800)]
bpf: Use kmemdup_array instead of kmemdup for multiple allocation
Let the kmemdup_array() take care about multiplication and possible
overflows.
Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20240821073709.4067177-1-yujiaoliang@vivo.com
Alexei Starovoitov [Wed, 21 Aug 2024 18:03:01 +0000 (11:03 -0700)]
Merge branch '__jited-test-tag-to-check-disassembly-after-jit'
Eduard Zingerman says:
====================
__jited test tag to check disassembly after jit
Some of the logic in the BPF jits might be non-trivial.
It might be useful to allow testing this logic by comparing
generated native code with expected code template.
This patch set adds a macro __jited() that could be used for
test_loader based tests in a following manner:
SEC("tp")
__arch_x86_64
__jited(" endbr64")
__jited(" nopl (%rax,%rax)")
__jited(" xorq %rax, %rax")
...
__naked void some_test(void) { ... }
Also add a test for jit code generated for tail calls handling to
demonstrate the feature.
The feature uses LLVM libraries to do the disassembly.
At selftests compilation time Makefile detects if these libraries are
available. When libraries are not available tests using __jit_x86()
are skipped.
Current CI environment does not include llvm development libraries,
but changes to add these are trivial.
This was previously discussed here:
https://lore.kernel.org/bpf/
20240718205158.
3651529-1-yonghong.song@linux.dev/
Patch-set includes a few auxiliary steps:
- patches #2 and #3 fix a few bugs in test_loader behaviour;
- patch #4 replaces __regex macro with ability to specify regular
expressions in __msg and __xlated using "{{" "}}" escapes;
- patch #8 updates __xlated to match disassembly lines consequently,
same way as __jited does.
Changes v2->v3:
- changed macro name from __jit_x86 to __jited with __arch_* to
specify disassembly arch (Yonghong);
- __jited matches disassembly lines consequently with "..."
allowing to skip some number of lines (Andrii);
- __xlated matches disassembly lines consequently, same as __jited;
- "{{...}}" regex brackets instead of __regex macro;
- bug fixes for old commits.
Changes v1->v2:
- stylistic changes suggested by Yonghong;
- fix for -Wformat-truncation related warning when compiled with
llvm15 (Yonghong).
v1: https://lore.kernel.org/bpf/
20240809010518.
1137758-1-eddyz87@gmail.com/
v2: https://lore.kernel.org/bpf/
20240815205449.242556-1-eddyz87@gmail.com/
====================
Link: https://lore.kernel.org/r/20240820102357.3372779-1-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Eduard Zingerman [Tue, 20 Aug 2024 10:23:57 +0000 (03:23 -0700)]
selftests/bpf: validate __xlated same way as __jited
Both __xlated and __jited work with disassembly.
It is logical to have both work in a similar manner.
This commit updates __xlated macro handling in test_loader.c by making
it expect matches on sequential lines, same way as __jited operates.
For example:
__xlated("1: *(u64 *)(r10 -16) = r1") ;; matched on line N
__xlated("3: r0 = &(void __percpu *)(r0)") ;; matched on line N+1
Also:
__xlated("1: *(u64 *)(r10 -16) = r1") ;; matched on line N
__xlated("...") ;; not matched
__xlated("3: r0 = &(void __percpu *)(r0)") ;; mantched on any
;; line >= N
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240820102357.3372779-10-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Eduard Zingerman [Tue, 20 Aug 2024 10:23:56 +0000 (03:23 -0700)]
selftests/bpf: validate jit behaviour for tail calls
A program calling sub-program which does a tail call.
The idea is to verify instructions generated by jit for tail calls:
- in program and sub-program prologues;
- for subprogram call instruction;
- for tail call itself.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240820102357.3372779-9-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Eduard Zingerman [Tue, 20 Aug 2024 10:23:54 +0000 (03:23 -0700)]
selftests/bpf: __jited test tag to check disassembly after jit
Allow to verify jit behaviour by writing tests as below:
SEC("tp")
__arch_x86_64
__jited(" endbr64")
__jited(" nopl (%rax,%rax)")
__jited(" xorq %rax, %rax")
...
__naked void some_test(void)
{
asm volatile (... ::: __clobber_all);
}
Allow regular expressions in patterns, same way as in __msg.
By default assume that each __jited pattern has to be matched on the
next consecutive line of the disassembly, e.g.:
__jited(" endbr64") # matched on line N
__jited(" nopl (%rax,%rax)") # matched on line N+1
If match occurs on a wrong line an error is reported.
To override this behaviour use __jited("..."), e.g.:
__jited(" endbr64") # matched on line N
__jited("...") # not matched
__jited(" nopl (%rax,%rax)") # matched on any line >= N
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240820102357.3372779-7-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Eduard Zingerman [Tue, 20 Aug 2024 10:23:53 +0000 (03:23 -0700)]
selftests/bpf: utility function to get program disassembly after jit
This commit adds a utility function to get disassembled text for jited
representation of a BPF program designated by file descriptor.
Function prototype looks as follows:
int get_jited_program_text(int fd, char *text, size_t text_sz)
Where 'fd' is a file descriptor for the program, 'text' and 'text_sz'
refer to a destination buffer for disassembled text.
Output format looks as follows:
18: 77 06 ja L0
1a: 50 pushq %rax
1b: 48 89 e0 movq %rsp, %rax
1e: eb 01 jmp L1
20: 50 L0: pushq %rax
21: 50 L1: pushq %rax
^ ^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^
| binary insn | textual insn
| representation | representation
| |
instruction offset inferred local label name
The code and makefile changes are inspired by jit_disasm.c from bpftool.
Use llvm libraries to disassemble BPF program instead of libbfd to avoid
issues with disassembly output stability pointed out in [1].
Selftests makefile uses Makefile.feature to detect if LLVM libraries
are available. If that is not the case selftests build proceeds but
the function returns -EOPNOTSUPP at runtime.
[1] commit
eb9d1acf634b ("bpftool: Add LLVM as default library for disassembling JIT-ed programs")
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240820102357.3372779-6-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Eduard Zingerman [Tue, 20 Aug 2024 10:23:52 +0000 (03:23 -0700)]
selftests/bpf: replace __regex macro with "{{...}}" patterns
Upcoming changes require a notation to specify regular expression
matches for regular verifier log messages, disassembly of BPF
instructions, disassembly of jited instructions.
Neither basic nor extended POSIX regular expressions w/o additional
escaping are good for this role because of wide use of special
characters in disassembly, for example:
movq -0x10(%rbp), %rax ;; () are special characters
cmpq $0x21, %rax ;; $ is a special character
*(u64 *)(r10 -16) = r1 ;; * and () are special characters
This commit borrows syntax from LLVM's FileCheck utility.
It replaces __regex macro with ability to embed regular expressions
in __msg patters using "{{" "}}" pairs for escaping.
Syntax for __msg patterns:
pattern := (<verbatim text> | regex)*
regex := "{{" <posix extended regular expression> "}}"
For example, pattern "foo{{[0-9]+}}" matches strings like
"foo0", "foo007", etc.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240820102357.3372779-5-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Eduard Zingerman [Tue, 20 Aug 2024 10:23:51 +0000 (03:23 -0700)]
selftests/bpf: fix to avoid __msg tag de-duplication by clang
__msg, __regex and __xlated tags are based on
__attribute__((btf_decl_tag("..."))) annotations.
Clang de-duplicates such annotations, e.g. the following
two sequences of tags are identical in final BTF:
/* seq A */ /* seq B */
__tag("foo") __tag("foo")
__tag("bar") __tag("bar")
__tag("foo")
Fix this by adding a unique suffix for each tag using __COUNTER__
pre-processor macro. E.g. here is a new definition for __msg:
#define __msg(msg) \
__attribute__((btf_decl_tag("comment:test_expect_msg=" XSTR(__COUNTER__) "=" msg)))
Using this definition the "seq A" from example above is translated to
BTF as follows:
[..] DECL_TAG 'comment:test_expect_msg=0=foo' type_id=X component_idx=-1
[..] DECL_TAG 'comment:test_expect_msg=1=bar' type_id=X component_idx=-1
[..] DECL_TAG 'comment:test_expect_msg=2=foo' type_id=X component_idx=-1
Surprisingly, this bug affects a single existing test:
verifier_spill_fill/old_stack_misc_vs_cur_ctx_ptr,
where sequence of identical messages was expected in the log.
Fixes:
537c3f66eac1 ("selftests/bpf: add generic BPF program tester-loader")
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240820102357.3372779-4-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Eduard Zingerman [Tue, 20 Aug 2024 10:23:50 +0000 (03:23 -0700)]
selftests/bpf: correctly move 'log' upon successful match
Suppose log="foo bar buz" and msg->substr="bar".
In such case current match processing logic would update 'log' as
follows: log += strlen(msg->substr); -> log += 3 -> log=" bar".
However, the intent behind the 'log' update is to make it point after
the successful match, e.g. to make log=" buz" in the example above.
Fixes:
4ef5d6af4935 ("selftests/bpf: no need to track next_match_pos in struct test_loader")
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240820102357.3372779-3-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Eduard Zingerman [Tue, 20 Aug 2024 10:23:49 +0000 (03:23 -0700)]
selftests/bpf: less spam in the log for message matching
When running test_loader based tests in the verbose mode each matched
message leaves a trace in the stderr, e.g.:
./test_progs -vvv -t ...
validate_msgs:PASS:expect_msg 0 nsec
validate_msgs:PASS:expect_msg 0 nsec
validate_msgs:PASS:expect_msg 0 nsec
validate_msgs:PASS:expect_msg 0 nsec
validate_msgs:PASS:expect_msg 0 nsec
This is not very helpful when debugging such tests and clobbers the
log a lot.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240820102357.3372779-2-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Alexei Starovoitov [Wed, 21 Aug 2024 17:37:52 +0000 (10:37 -0700)]
Merge branch 'support-passing-bpf-iterator-to-kfuncs'
Andrii Nakryiko says:
====================
Support passing BPF iterator to kfuncs
Add support for passing BPF iterator state to any kfunc. Such kfunc has to
declare such argument with valid `struct bpf_iter_<type> *` type and should
use "__iter" suffix in argument name, following the established suffix-based
convention. We add a simple test/demo iterator getter in bpf_testmod.
====================
Link: https://lore.kernel.org/r/20240808232230.2848712-1-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Andrii Nakryiko [Thu, 8 Aug 2024 23:22:30 +0000 (16:22 -0700)]
selftests/bpf: test passing iterator to a kfunc
Define BPF iterator "getter" kfunc, which accepts iterator pointer as
one of the arguments. Make sure that argument passed doesn't have to be
the very first argument (unlike new-next-destroy combo).
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20240808232230.2848712-4-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Andrii Nakryiko [Thu, 8 Aug 2024 23:22:29 +0000 (16:22 -0700)]
bpf: allow passing struct bpf_iter_<type> as kfunc arguments
There are potentially useful cases where a specific iterator type might
need to be passed into some kfunc. So, in addition to existing
bpf_iter_<type>_{new,next,destroy}() kfuncs, allow to pass iterator
pointer to any kfunc.
We employ "__iter" naming suffix for arguments that are meant to accept
iterators. We also enforce that they accept PTR -> STRUCT btf_iter_<type>
type chain and point to a valid initialized on-the-stack iterator state.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20240808232230.2848712-3-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Andrii Nakryiko [Thu, 8 Aug 2024 23:22:28 +0000 (16:22 -0700)]
bpf: extract iterator argument type and name validation logic
Verifier enforces that all iterator structs are named `bpf_iter_<name>`
and that whenever iterator is passed to a kfunc it's passed as a valid PTR ->
STRUCT chain (with potentially const modifiers in between).
We'll need this check for upcoming changes, so instead of duplicating
the logic, extract it into a helper function.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240808232230.2848712-2-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Andrii Nakryiko [Mon, 19 Aug 2024 22:30:06 +0000 (15:30 -0700)]
Merge branch 'correct-recent-gcc-incompatible-changes'
Cupertino Miranda says:
====================
Correct recent GCC incompatible changes.
Hi everyone,
Apologies for the previous patches which did not include a cover letter.
My wish was to send 3 indepepdent patches but after the initial mistake lets keep
this as a series although they are all independent from themselves.
The changes in this patch series is related to recovering GCC support to
build the selftests.
A few tests and a makefile change have broken the support for GCC in the
last few months.
Looking forward to your comments.
Best regards,
Cupertino
====================
Link: https://lore.kernel.org/r/20240819151129.1366484-1-cupertino.miranda@oracle.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Cupertino Miranda [Mon, 19 Aug 2024 15:11:29 +0000 (16:11 +0100)]
selftest/bpf: Adapt inline asm operand constraint for GCC support
GCC errors when compiling tailcall_bpf2bpf_hierarchy2.c and
tailcall_bpf2bpf_hierarchy3.c with the following error:
progs/tailcall_bpf2bpf_hierarchy2.c: In function 'tailcall_bpf2bpf_hierarchy_2':
progs/tailcall_bpf2bpf_hierarchy2.c:66:9: error: input operand constraint contains '+'
66 | asm volatile (""::"r+"(ret));
| ^~~
Changed implementation to make use of __sink macro that abstracts the
desired behaviour.
The proposed change seems valid for both GCC and CLANG.
Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240819151129.1366484-4-cupertino.miranda@oracle.com
Cupertino Miranda [Mon, 19 Aug 2024 15:11:27 +0000 (16:11 +0100)]
selftests/bpf: Disable strict aliasing for verifier_nocsr.c
verfifier_nocsr.c fails to compile in GCC. The reason behind it was
initially explained in commit
27a90b14b93d3b2e1efd10764e456af7e2a42991.
"A few BPF selftests perform type punning and they may break strict
aliasing rules, which are exploited by both GCC and clang by default
while optimizing. This can lead to broken compiled programs."
Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240819151129.1366484-2-cupertino.miranda@oracle.com
Andrii Nakryiko [Mon, 19 Aug 2024 22:25:31 +0000 (15:25 -0700)]
Merge branch 'bpf-enable-some-functions-in-cgroup-programs'
Matteo Croce says:
====================
bpf: enable some functions in cgroup programs
From: Matteo Croce <teknoraver@meta.com>
Enable some BPF kfuncs and the helper bpf_current_task_under_cgroup()
for program types BPF_CGROUP_*.
These will be used by systemd-networkd:
https://github.com/systemd/systemd/pull/32212
v5->v6:
Called register_btf_kfunc_id_set() only once
Fixed build error with !CONFIG_CGROUPS
v4->v5:
Same code, but v4 had an old cover letter
v3->v4:
Reset all the acked-by tags because the code changed a bit.
Signed-off-by: Matteo Croce <teknoraver@meta.com>
====================
Link: https://lore.kernel.org/r/20240819162805.78235-1-technoboy85@gmail.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Matteo Croce [Mon, 19 Aug 2024 16:28:05 +0000 (18:28 +0200)]
bpf: Allow bpf_current_task_under_cgroup() with BPF_CGROUP_*
The helper bpf_current_task_under_cgroup() currently is only allowed for
tracing programs, allow its usage also in the BPF_CGROUP_* program types.
Move the code from kernel/trace/bpf_trace.c to kernel/bpf/helpers.c,
so it compiles also without CONFIG_BPF_EVENTS.
This will be used in systemd-networkd to monitor the sysctl writes,
and filter it's own writes from others:
https://github.com/systemd/systemd/pull/32212
Signed-off-by: Matteo Croce <teknoraver@meta.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240819162805.78235-3-technoboy85@gmail.com
Matteo Croce [Mon, 19 Aug 2024 16:28:04 +0000 (18:28 +0200)]
bpf: Enable generic kfuncs for BPF_CGROUP_* programs
These kfuncs are enabled even in BPF_PROG_TYPE_TRACING, so they
should be safe also in BPF_CGROUP_* programs.
Since all BPF_CGROUP_* programs share the same hook,
call register_btf_kfunc_id_set() only once.
In enum btf_kfunc_hook, rename BTF_KFUNC_HOOK_CGROUP_SKB to a more
generic BTF_KFUNC_HOOK_CGROUP, since it's used for all the cgroup
related program types.
Signed-off-by: Matteo Croce <teknoraver@meta.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240819162805.78235-2-technoboy85@gmail.com
Jiangshan Yi [Thu, 15 Aug 2024 13:55:24 +0000 (21:55 +0800)]
samples/bpf: Fix compilation errors with cf-protection option
Currently, compiling the bpf programs will result the compilation errors
with the cf-protection option as follows in arm64 and loongarch64 machine
when using gcc 12.3.1 and clang 17.0.6. This commit fixes the compilation
errors by limited the cf-protection option only used in x86 platform.
[root@localhost linux]# make M=samples/bpf
......
CLANG-bpf samples/bpf/xdp2skb_meta_kern.o
error: option 'cf-protection=return' cannot be specified on this target
error: option 'cf-protection=branch' cannot be specified on this target
2 errors generated.
CLANG-bpf samples/bpf/syscall_tp_kern.o
error: option 'cf-protection=return' cannot be specified on this target
error: option 'cf-protection=branch' cannot be specified on this target
2 errors generated.
......
Fixes:
34f6e38f58db ("samples/bpf: fix warning with ignored-attributes")
Reported-by: Jiangshan Yi <yijiangshan@kylinos.cn>
Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Tested-by: Qiang Wang <wangqiang1@kylinos.cn>
Link: https://lore.kernel.org/bpf/20240815135524.140675-1-13667453960@163.com
Sam James [Tue, 13 Aug 2024 19:49:06 +0000 (20:49 +0100)]
libbpf: Workaround -Wmaybe-uninitialized false positive
In `elf_close`, we get this with GCC 15 -O3 (at least):
```
In function ‘elf_close’,
inlined from ‘elf_close’ at elf.c:53:6,
inlined from ‘elf_find_func_offset_from_file’ at elf.c:384:2:
elf.c:57:9: warning: ‘elf_fd.elf’ may be used uninitialized [-Wmaybe-uninitialized]
57 | elf_end(elf_fd->elf);
| ^~~~~~~~~~~~~~~~~~~~
elf.c: In function ‘elf_find_func_offset_from_file’:
elf.c:377:23: note: ‘elf_fd.elf’ was declared here
377 | struct elf_fd elf_fd;
| ^~~~~~
In function ‘elf_close’,
inlined from ‘elf_close’ at elf.c:53:6,
inlined from ‘elf_find_func_offset_from_file’ at elf.c:384:2:
elf.c:58:9: warning: ‘elf_fd.fd’ may be used uninitialized [-Wmaybe-uninitialized]
58 | close(elf_fd->fd);
| ^~~~~~~~~~~~~~~~~
elf.c: In function ‘elf_find_func_offset_from_file’:
elf.c:377:23: note: ‘elf_fd.fd’ was declared here
377 | struct elf_fd elf_fd;
| ^~~~~~
```
In reality, our use is fine, it's just that GCC doesn't model errno
here (see linked GCC bug). Suppress -Wmaybe-uninitialized accordingly
by initializing elf_fd.fd to -1 and elf_fd.elf to NULL.
I've done this in two other functions as well given it could easily
occur there too (same access/use pattern).
Signed-off-by: Sam James <sam@gentoo.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://gcc.gnu.org/PR114952
Link: https://lore.kernel.org/bpf/14ec488a1cac02794c2fa2b83ae0cef1bce2cb36.1723578546.git.sam@gentoo.org
Jeongjun Park [Wed, 7 Aug 2024 14:31:10 +0000 (23:31 +0900)]
bpf: Remove __btf_name_valid() and change to btf_name_valid_identifier()
__btf_name_valid() can be completely replaced with
btf_name_valid_identifier, and since most of the time you already call
btf_name_valid_identifier instead of __btf_name_valid , it would be
appropriate to rename the __btf_name_valid function to
btf_name_valid_identifier and remove __btf_name_valid.
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Link: https://lore.kernel.org/bpf/20240807143110.181497-1-aha310510@gmail.com
Martin KaFai Lau [Thu, 15 Aug 2024 17:26:38 +0000 (10:26 -0700)]
Merge branch 'monitor network traffic for flaky test cases'
Kui-Feng Lee says:
====================
Capture packets in the background for flaky test cases related to
network features.
We have some flaky test cases that are difficult to debug without
knowing what the traffic looks like. Capturing packets, the CI log and
packet files may help developers to fix these flaky test cases.
This patch set monitors a few test cases. Recently, they have been
showing flaky behavior.
lo In IPv4 127.0.0.1:40265 > 127.0.0.1:55907: TCP, length 68, SYN
lo In IPv4 127.0.0.1:55907 > 127.0.0.1:40265: TCP, length 60, SYN, ACK
lo In IPv4 127.0.0.1:40265 > 127.0.0.1:55907: TCP, length 60, ACK
lo In IPv4 127.0.0.1:55907 > 127.0.0.1:40265: TCP, length 52, ACK
lo In IPv4 127.0.0.1:40265 > 127.0.0.1:55907: TCP, length 52, FIN, ACK
lo In IPv4 127.0.0.1:55907 > 127.0.0.1:40265: TCP, length 52, RST, ACK
Packet file: packets-2173-86-select_reuseport:sockhash_IPv4_TCP_LOOPBACK_test_detach_bpf-test.log
#280/87 select_reuseport/sockhash IPv4/TCP LOOPBACK test_detach_bpf:OK
The above block is the log of a test case. It shows every packet of a
connection. The captured packets are stored in the file called
packets-2173-86-select_reuseport:sockhash_IPv4_TCP_LOOPBACK_test_detach_bpf-test.log.
We have a set of high-level helpers and a test_progs option to
simplify the process of enabling the traffic monitor. netns_new() and
netns_free() are helpers used to create and delete namespaces while
also enabling the traffic monitor for the namespace based on the
patterns provided by the "-m" option of test_progs. The value of the
"-m" option is a list of patterns used to enable the traffic monitor
for a group of tests or a file containing patterns. CI can utilize
this option to enable monitoring.
traffic_monitor_start() and traffic_monitor_stop() are low-level
functions to start monitoring explicitly. You can have more controls,
however high-level helpers are preferred.
The following block is an example that monitors the network traffic of
a test case in a network namespace.
struct netns_obj *netns;
...
netns = netns_new("test", true);
if (!ASSERT_TRUE(netns, "netns_new"))
goto err;
... test ...
netns_free(netns);
netns_new() will create a network namespace named "test" and bring up
"lo" in the namespace. By passing "true" as the 2nd argument, it will
set the network namespace of the current process to
"test".netns_free() will destroy the namespace, and the process will
leave the "test" namespace if the struct netns_obj returned by
netns_new() is created with "true" as the 2nd argument. If the name of
the test matches the patterns given by the "-m" option, the traffic
monitor will be enabled for the "test" namespace as well.
The packet files are located in the directory "/tmp/tmon_pcap/". The
directory is intended to be compressed as a file so that developers
can download it from the CI.
This feature is enabled only if libpcap is available when building
selftests.
---
Changes from v7:
- Remove ":" with "__" from the file names of traffic logs. ':' would
cause an error of the upload-artifact action of github.
- Move remove_netns() to avoid a forward declaration.
Changes from v6:
- Remove unnecessary memcpy for addresses.
- Make packet messages similar to what tcpdump prints.
- Check return value of inet_ntop().
- Remove duplicated errno in messages.
- Print arphdr_type for not handled packets.
- Set dev "lo" in make_netns().
- Avoid stacking netns by moving traffic_monitor_start() to earlier
position.
- Remove the word "packet" from packet messages.
- Replace pipe with eventfd (wake_fd) to synchronize background threads.
Changes from v5:
- Remove "-m" completely if traffic monitor is not enabled.
Changes from v4:
- Use pkg-config to detect libpcap, and enable traffic monitor if
there is libpcap.
- Move traffic monitor functions back to network_helper.c, and pass
extra parameters to traffic_monitor_start().
- Use flockfile() & funlockfile() to avoid log interleaving.
- Show "In", "Out", "M" ... for captured packets.
- Print a warning message if the user pass a "-m" when libpcap is not
available.
- Bring up dev lo in netns_new().
Changes from v3:
- Rebase to the latest tip of bpf-next/for-next
- Change verb back to C string.
Changes from v2:
- Include pcap header files conditionally.
- Move the implementation of traffic monitor to test_progs.c.
- Include test name and namespace as a part of names of packet files.
- Parse and print ICMP(v4|v6) packets.
- Add netns_new() and netns_free() to create and delete network
namespaces.
- Make tc_redirect, sockmap_listen and select_reuseport test in a
network namespace.
- Add the "-m" option to test_progs to enable traffic monitor for the
tests matching the pattern. CI may use this option to enable
monitoring for a given set of tests.
Changes from v1:
- Move to calling libpcap directly to capture packets in a background
thread.
- Print parsed packet information for TCP and UDP packets.
v1: https://lore.kernel.org/all/
20240713055552.
2482367-5-thinker.li@gmail.com/
v2: https://lore.kernel.org/all/
20240723182439.
1434795-1-thinker.li@gmail.com/
v3: https://lore.kernel.org/all/
20240730002745.
1484204-1-thinker.li@gmail.com/
v4: https://lore.kernel.org/all/
20240731193140.758210-1-thinker.li@gmail.com/
v5: https://lore.kernel.org/all/
20240806221243.
1806879-1-thinker.li@gmail.com/
v6: https://lore.kernel.org/all/
20240807183149.764711-1-thinker.li@gmail.com/
v7: https://lore.kernel.org/all/
20240810023534.
2458227-2-thinker.li@gmail.com/
====================
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Kui-Feng Lee [Thu, 15 Aug 2024 05:32:54 +0000 (22:32 -0700)]
selftests/bpf: Monitor traffic for select_reuseport.
Enable traffic monitoring for the subtests of select_reuseport.
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
Link: https://lore.kernel.org/r/20240815053254.470944-7-thinker.li@gmail.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Kui-Feng Lee [Thu, 15 Aug 2024 05:32:53 +0000 (22:32 -0700)]
selftests/bpf: Monitor traffic for sockmap_listen.
Enable traffic monitor for each subtest of sockmap_listen.
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
Link: https://lore.kernel.org/r/20240815053254.470944-6-thinker.li@gmail.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Kui-Feng Lee [Thu, 15 Aug 2024 05:32:52 +0000 (22:32 -0700)]
selftests/bpf: Monitor traffic for tc_redirect.
Enable traffic monitoring for the test case tc_redirect.
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
Link: https://lore.kernel.org/r/20240815053254.470944-5-thinker.li@gmail.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Kui-Feng Lee [Thu, 15 Aug 2024 05:32:51 +0000 (22:32 -0700)]
selftests/bpf: netns_new() and netns_free() helpers.
netns_new()/netns_free() create/delete network namespaces. They support the
option '-m' of test_progs to start/stop traffic monitor for the network
namespace being created for matched tests.
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
Link: https://lore.kernel.org/r/20240815053254.470944-4-thinker.li@gmail.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Kui-Feng Lee [Thu, 15 Aug 2024 05:32:50 +0000 (22:32 -0700)]
selftests/bpf: Add the traffic monitor option to test_progs.
Add option '-m' to test_progs to accept names and patterns of test cases.
This option will be used later to enable traffic monitor that capture
network packets generated by test cases.
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
Link: https://lore.kernel.org/r/20240815053254.470944-3-thinker.li@gmail.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Kui-Feng Lee [Thu, 15 Aug 2024 05:32:49 +0000 (22:32 -0700)]
selftests/bpf: Add traffic monitor functions.
Add functions that capture packets and print log in the background. They
are supposed to be used for debugging flaky network test cases. A monitored
test case should call traffic_monitor_start() to start a thread to capture
packets in the background for a given namespace and call
traffic_monitor_stop() to stop capturing. (Or, option '-m' implemented by
the later patches.)
lo In IPv4 127.0.0.1:40265 > 127.0.0.1:55907: TCP, length 68, SYN
lo In IPv4 127.0.0.1:55907 > 127.0.0.1:40265: TCP, length 60, SYN, ACK
lo In IPv4 127.0.0.1:40265 > 127.0.0.1:55907: TCP, length 60, ACK
lo In IPv4 127.0.0.1:55907 > 127.0.0.1:40265: TCP, length 52, ACK
lo In IPv4 127.0.0.1:40265 > 127.0.0.1:55907: TCP, length 52, FIN, ACK
lo In IPv4 127.0.0.1:55907 > 127.0.0.1:40265: TCP, length 52, RST, ACK
Packet file: packets-2173-86-select_reuseport:sockhash_IPv4_TCP_LOOPBACK_test_detach_bpf-test.log
#280/87 select_reuseport/sockhash IPv4/TCP LOOPBACK test_detach_bpf:OK
The above is the output of an example. It shows the packets of a connection
and the name of the file that contains captured packets in the directory
/tmp/tmon_pcap. The file can be loaded by tcpdump or wireshark.
This feature only works if libpcap is available. (Could be found by pkg-config)
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
Link: https://lore.kernel.org/r/20240815053254.470944-2-thinker.li@gmail.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Martin KaFai Lau [Thu, 15 Aug 2024 01:10:47 +0000 (18:10 -0700)]
Merge branch 'selftests/bpf: convert three other cgroup tests to test_progs'
Alexis Lothoré (eBPF Foundation) says:
====================
Hello,
this series brings a new set of test converted to the test_progs framework.
Since the tests are quite small, I chose to group three tests conversion in
the same series, but feel free to let me know if I should keep one series
per test. The series focuses on cgroup testing and converts the following
tests:
- get_cgroup_id_user
- cgroup_storage
- test_skb_cgroup_id_user
Changes in v4:
- Fix test after netns addition by making sure loopack interface is up
- Link to v3: https://lore.kernel.org/r/
20240812-convert_cgroup_tests-v3-0-
47ac6ce4e88b@bootlin.com
Changes in v3:
- Fixed multiple leaks on cgroup file descriptors and sockets
- Used dedicated network namespaces for tests involving network
- Link to v2: https://lore.kernel.org/r/
20240806-convert_cgroup_tests-v2-0-
180c57e5b710@bootlin.com
Changes in v2:
- Use global variables instead of maps when possible
- Collect review tags from Alan
- Link to v1: https://lore.kernel.org/r/
20240731-convert_cgroup_tests-v1-0-
14cbc51b6947@bootlin.com
====================
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Alexis Lothoré (eBPF Foundation) [Tue, 13 Aug 2024 12:45:08 +0000 (14:45 +0200)]
selftests/bpf: convert test_skb_cgroup_id_user to test_progs
test_skb_cgroup_id_user allows testing skb cgroup id retrieval at different
levels, but is not integrated in test_progs, so it is not run
automatically in CI. The test overlaps a bit with
cgroup_skb_sk_lookup_kern, which is integrated in test_progs and test
extensively skb cgroup helpers, but there is still one major difference
between the two tests which justifies the conversion:
cgroup_skb_sk_lookup_kern deals with a BPF_PROG_TYPE_CGROUP_SKB (attached
on a cgroup), while test_skb_cgroup_id_user deals with a
BPF_PROG_TYPE_SCHED_CLS (attached on a qdisc)
Convert test_skb_cgroup_id_user into test_progs framework in order to run
it automatically in CI. The main differences with the original test are the
following:
- rename the test to make it shorter and more straightforward regarding
tested feature
- the wrapping shell script has been dropped since every setup step is now
handled in the main C test file
- the test has been renamed for a shorter name and reflecting the tested
API
- add dedicated assert log per level to ease test failure debugging
- use global variables instead of maps to access bpf prog data
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Link: https://lore.kernel.org/r/20240813-convert_cgroup_tests-v4-4-a33c03458cf6@bootlin.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Alexis Lothoré (eBPF Foundation) [Tue, 13 Aug 2024 12:45:07 +0000 (14:45 +0200)]
selftests/bpf: add proper section name to bpf prog and rename it
test_skb_cgroup_id_kern.c is currently involved in a manual test. In its
current form, it can not be used with the auto-generated skeleton APIs,
because the section name is not valid to allow libbpf to deduce the program
type.
Update section name to allow skeleton APIs usage. Also rename the program
name to make it shorter and more straighforward regarding the API it is
testing. While doing so, make sure that test_skb_cgroup_id.sh passes to get
a working reference before converting it to test_progs
- update the obj name
- fix loading issue (verifier rejecting the program when loaded through tc,
because of map not found), by preloading the whole obj with bpftool
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Link: https://lore.kernel.org/r/20240813-convert_cgroup_tests-v4-3-a33c03458cf6@bootlin.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Alexis Lothoré (eBPF Foundation) [Tue, 13 Aug 2024 12:45:06 +0000 (14:45 +0200)]
selftests/bpf: convert test_cgroup_storage to test_progs
test_cgroup_storage is currently a standalone program which is not run
when executing test_progs.
Convert it to the test_progs framework so it can be automatically executed
in CI. The conversion led to the following changes:
- converted the raw bpf program in the userspace test file into a dedicated
test program in progs/ dir
- reduced the scope of cgroup_storage test: the content from this test
overlaps with some other tests already present in test_progs, most
notably netcnt and cgroup_storage_multi*. Those tests already check
extensively local storage, per-cpu local storage, cgroups interaction,
etc. So the new test only keep the part testing that the program return
code (based on map content) properly leads to packet being passed or
dropped.
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Link: https://lore.kernel.org/r/20240813-convert_cgroup_tests-v4-2-a33c03458cf6@bootlin.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Alexis Lothoré (eBPF Foundation) [Tue, 13 Aug 2024 12:45:05 +0000 (14:45 +0200)]
selftests/bpf: convert get_current_cgroup_id_user to test_progs
get_current_cgroup_id_user allows testing for bpf_get_current_cgroup_id()
bpf API but is not integrated into test_progs, and so is not tested
automatically in CI.
Convert it to the test_progs framework to allow running it automatically.
The most notable differences with the old test are the following:
- the new test relies on autoattach instead of manually hooking/enabling
the targeted tracepoint through perf_event, which reduces quite a lot the
test code size
- it also accesses bpf prog data through global variables instead of maps
- sleep duration passed to nanosleep syscall has been reduced to its
minimum to not impact overall CI duration (we only care about the syscall
being properly triggered, not about the passed duration)
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Link: https://lore.kernel.org/r/20240813-convert_cgroup_tests-v4-1-a33c03458cf6@bootlin.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Alan Maguire [Sat, 10 Aug 2024 09:35:04 +0000 (10:35 +0100)]
libbpf: Fix license for btf_relocate.c
License should be
// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
...as with other libbpf files.
Fixes:
19e00c897d50 ("libbpf: Split BTF relocation")
Reported-by: Neill Kapron <nkapron@google.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Link: https://lore.kernel.org/bpf/20240810093504.2111134-1-alan.maguire@oracle.com
Alexei Starovoitov [Wed, 7 Aug 2024 18:26:54 +0000 (11:26 -0700)]
Merge branch 'add-bpf_get_dentry_xattr'
Song Liu says:
====================
Add bpf_get_dentry_xattr
Add a kfunc to read xattr from dentry. Also add selftest for the new
kfunc.
Changes v3 => v4:
1. Fix selftest build.
V3: https://lore.kernel.org/bpf/
20240806203340.
3503805-1-song@kernel.org/T/#u
Changes v2 => v3:
1. Move the kfuncs to fs/bpf_fs_kfuncs.c.
2. Fix selftests build error on s390. (Alexei)
v2: https://lore.kernel.org/bpf/
20240730230805.42205-1-song@kernel.org/T/#u
Changes v1 => v2:
1. Remove 3 kfuncs that are ready yet.
v1: https://lore.kernel.org/linux-fsdevel/
20240725234706.655613-1-song@kernel.org/T/#u
====================
Link: https://lore.kernel.org/r/20240806230904.71194-1-song@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Song Liu [Tue, 6 Aug 2024 23:09:04 +0000 (16:09 -0700)]
selftests/bpf: Add tests for bpf_get_dentry_xattr
Add test for bpf_get_dentry_xattr on hook security_inode_getxattr.
Verify that the kfunc can read the xattr. Also test failing getxattr
from user space by returning non-zero from the LSM bpf program.
Acked-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/20240806230904.71194-4-song@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Song Liu [Tue, 6 Aug 2024 23:09:03 +0000 (16:09 -0700)]
bpf: Add kfunc bpf_get_dentry_xattr() to read xattr from dentry
This kfunc can be used in LSM hooks with dentry, such as:
security_inode_listxattr
security_inode_permission
and many more.
Acked-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/20240806230904.71194-3-song@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Song Liu [Tue, 6 Aug 2024 23:09:02 +0000 (16:09 -0700)]
bpf: Move bpf_get_file_xattr to fs/bpf_fs_kfuncs.c
We are putting all fs kfuncs in fs/bpf_fs_kfuncs.c. Move existing
bpf_get_file_xattr to it.
Signed-off-by: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/20240806230904.71194-2-song@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Alexei Starovoitov [Tue, 6 Aug 2024 15:38:17 +0000 (08:38 -0700)]
Merge branch 'bpf-introduce-new-vfs-based-bpf-kfuncs'
Matt Bobrowski says:
====================
bpf: introduce new VFS based BPF kfuncs
G'day!
A respin based off v3, which can be found here [0]. Original
motivations for introducing this suite of BPF kfuncs can be found here
[1].
The primary difference in this version of the patch series is that the
suite of VFS related BPF kfuncs added can be used from both sleepable
and non-sleepable BPF LSM program types. IOW, the KF_SLEEPABLE
annotation has been removed from all of them.
Changes sinve v3:
* KF_SLEEPABLE annotation has been dropped from all newly introduced
VFS related BPF kfuncs. This includes bpf_get_task_exe_file(),
bpf_put_file(), and bpf_path_d_path(). Both negative and positive
selftests backing these new BPF kfuncs have also been updated
accordingly.
* buf__sz conditional in bpf_path_d_path() has been updated from
buf__sz <= 0, to !buf__sz.
* Syntax issues as reported so here [2] have been corrected.
[0] https://lore.kernel.org/bpf/
20240726085604.
2369469-1-mattbobrowski@google.com/
[1] https://lore.kernel.org/bpf/cover.
1708377880.git.mattbobrowski@google.com/#t
[2] https://netdev.bots.linux.dev/static/nipa/874023/
13742510/checkpatch/stdout
====================
Link: https://lore.kernel.org/r/20240731110833.1834742-1-mattbobrowski@google.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Matt Bobrowski [Wed, 31 Jul 2024 11:08:33 +0000 (11:08 +0000)]
selftests/bpf: add positive tests for new VFS based BPF kfuncs
Add a bunch of positive selftests which extensively cover the various
contexts and parameters in which the new VFS based BPF kfuncs may be
used from.
Again, the following VFS based BPF kfuncs are thoroughly tested within
this new selftest:
* struct file *bpf_get_task_exe_file(struct task_struct *);
* void bpf_put_file(struct file *);
* int bpf_path_d_path(struct path *, char *, size_t);
Acked-by: Christian Brauner <brauner@kernel.org>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
Link: https://lore.kernel.org/r/20240731110833.1834742-4-mattbobrowski@google.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Matt Bobrowski [Wed, 31 Jul 2024 11:08:32 +0000 (11:08 +0000)]
selftests/bpf: add negative tests for new VFS based BPF kfuncs
Add a bunch of negative selftests responsible for asserting that the
BPF verifier successfully rejects a BPF program load when the
underlying BPF program misuses one of the newly introduced VFS based
BPF kfuncs.
The following VFS based BPF kfuncs are extensively tested within this
new selftest:
* struct file *bpf_get_task_exe_file(struct task_struct *);
* void bpf_put_file(struct file *);
* int bpf_path_d_path(struct path *, char *, size_t);
Acked-by: Christian Brauner <brauner@kernel.org>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
Link: https://lore.kernel.org/r/20240731110833.1834742-3-mattbobrowski@google.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Matt Bobrowski [Wed, 31 Jul 2024 11:08:31 +0000 (11:08 +0000)]
bpf: introduce new VFS based BPF kfuncs
Add a new variant of bpf_d_path() named bpf_path_d_path() which takes
the form of a BPF kfunc and enforces KF_TRUSTED_ARGS semantics onto
its arguments.
This new d_path() based BPF kfunc variant is intended to address the
legacy bpf_d_path() BPF helper's susceptability to memory corruption
issues [0, 1, 2] by ensuring to only operate on supplied arguments
which are deemed trusted by the BPF verifier. Typically, this means
that only pointers to a struct path which have been referenced counted
may be supplied.
In addition to the new bpf_path_d_path() BPF kfunc, we also add a
KF_ACQUIRE based BPF kfunc bpf_get_task_exe_file() and KF_RELEASE
counterpart BPF kfunc bpf_put_file(). This is so that the new
bpf_path_d_path() BPF kfunc can be used more flexibily from within the
context of a BPF LSM program. It's rather common to ascertain the
backing executable file for the calling process by performing the
following walk current->mm->exe_file while instrumenting a given
operation from the context of the BPF LSM program. However, walking
current->mm->exe_file directly is never deemed to be OK, and doing so
from both inside and outside of BPF LSM program context should be
considered as a bug. Using bpf_get_task_exe_file() and in turn
bpf_put_file() will allow BPF LSM programs to reliably get and put
references to current->mm->exe_file.
As of now, all the newly introduced BPF kfuncs within this patch are
limited to BPF LSM program types. These can be either sleepable or
non-sleepable variants of BPF LSM program types.
[0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=5FcwddF_VKfEBPs_Dg@mail.gmail.com/
[1] https://lore.kernel.org/bpf/
20230606181714.532998-1-jolsa@kernel.org/
[2] https://lore.kernel.org/bpf/
20220219113744.
1852259-1-memxor@gmail.com/
Acked-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
Acked-by: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/20240731110833.1834742-2-mattbobrowski@google.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Yonghong Song [Fri, 2 Aug 2024 18:54:34 +0000 (11:54 -0700)]
selftests/bpf: Fix a btf_dump selftest failure
Jakub reported bpf selftest "btf_dump" failure after forwarding to
v6.11-rc1 with netdev.
Error: #33 btf_dump
Error: #33/15 btf_dump/btf_dump: var_data
btf_dump_data:FAIL:find type id unexpected find type id: actual -2 < expected 0
The reason for the failure is due to
commit
94ede2a3e913 ("profiling: remove stale percpu flip buffer variables")
where percpu static variable "cpu_profile_flip" is removed.
Let us replace "cpu_profile_flip" with a variable in bpf subsystem
so whenever that variable gets deleted or renamed, we can detect the
failure immediately. In this case, I picked a static percpu variable
"bpf_cgrp_storage_busy" which is defined in kernel/bpf/bpf_cgrp_storage.c.
Reported-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240802185434.1749056-1-yonghong.song@linux.dev
Martin KaFai Lau [Wed, 31 Jul 2024 17:00:20 +0000 (10:00 -0700)]
Merge branch 'selftests/bpf: convert test_dev_cgroup to test_progs'
Alexis Lothoré (eBPF Foundation) says:
====================
Hello,
this small series aims to integrate test_dev_cgroup in test_progs so it
could be run automatically in CI. The new version brings a few differences
with the current one:
- test now uses directly syscalls instead of wrapping commandline tools
into system() calls
- test_progs manipulates /dev/null (eg: redirecting test logs into it), so
disabling access to it in the bpf program confuses the tests. To fix this,
the first commit modifies the bpf program to allow access to char devices
1:3 (/dev/null), and disable access to char devices 1:5 (/dev/zero)
- once test is converted, add a small subtest to also check for device type
interpretation (char or block)
- paths used in mknod tests are now in /dev instead of /tmp: due to the CI
runner organisation and mountpoints manipulations, trying to create nodes
in /tmp leads to errors unrelated to the test (ie, mknod calls refused by
kernel, not the bpf program). I don't understand exactly the root cause
at the deepest point (all I see in CI is an -ENXIO error on mknod when trying to
create the node in tmp, and I can not make sense out of it neither
replicate it locally), so I would gladly take inputs from anyone more
educated than me about this.
The new test_progs part has been tested in a local qemu environment as well
as in upstream CI:
./test_progs -a cgroup_dev
47/1 cgroup_dev/allow-mknod:OK
47/2 cgroup_dev/allow-read:OK
47/3 cgroup_dev/allow-write:OK
47/4 cgroup_dev/deny-mknod:OK
47/5 cgroup_dev/deny-read:OK
47/6 cgroup_dev/deny-write:OK
47/7 cgroup_dev/deny-mknod-wrong-type:OK
47 cgroup_dev:OK
Summary: 1/7 PASSED, 0 SKIPPED, 0 FAILED
---
Changes in v4:
- Fix mixup between ret and errno by testing both
- Properly apply ack tag from Stanislas
- Link to v3: https://lore.kernel.org/r/
20240730-convert_dev_cgroup-v3-0-
93e573b74357@bootlin.com
Changes in v3:
- delete mknod file only if it has been created
- use bpf_program__attach_cgroup() instead of bpf_prog_attach
- reorganize subtests order
- collect review/ack tags from Alan and Stanislas
- Link to v2: https://lore.kernel.org/r/
20240729-convert_dev_cgroup-v2-0-
4c1fc0520545@bootlin.com
Changes in v2:
- directly pass expected ret code to subtests instead of boolean pass/not
pass
- fix faulty fd check in subtest expected to fail on open
- fix wrong subtest name
- pass test buffer and corresponding size to read/write subtests
- use correct series prefix
- Link to v1: https://lore.kernel.org/r/
20240725-convert_dev_cgroup-v1-0-
2c8cbd487c44@bootlin.com
====================
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Alexis Lothoré (eBPF Foundation) [Wed, 31 Jul 2024 06:37:27 +0000 (08:37 +0200)]
selftests/bpf: add wrong type test to cgroup dev
Current cgroup_dev test mostly tests that device operation is accepted or
refused base on passed major/minor (and so, any operation performed during
test involves only char device)
Add a small subtest ensuring that the device type passed to bpf program
allows it to take decisions as well.
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Link: https://lore.kernel.org/r/20240731-convert_dev_cgroup-v4-3-849425d90de6@bootlin.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Alexis Lothoré (eBPF Foundation) [Wed, 31 Jul 2024 06:37:26 +0000 (08:37 +0200)]
selftests/bpf: convert test_dev_cgroup to test_progs
test_dev_cgroup is defined as a standalone test program, and so is not
executed in CI.
Convert it to test_progs framework so it is tested automatically in CI, and
remove the old test. In order to be able to run it in test_progs, /dev/null
must remain usable, so change the new test to test operations on devices
1:3 as valid, and operations on devices 1:5 (/dev/zero) as invalid.
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Link: https://lore.kernel.org/r/20240731-convert_dev_cgroup-v4-2-849425d90de6@bootlin.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Alexis Lothoré (eBPF Foundation) [Wed, 31 Jul 2024 06:37:25 +0000 (08:37 +0200)]
selftests/bpf: do not disable /dev/null device access in cgroup dev test
test_dev_cgroup currently loads a small bpf program allowing any access on
urandom and zero devices, disabling access to any other device. It makes
migrating this test to test_progs impossible, since this one manipulates
extensively /dev/null.
Allow /dev/null manipulation in dev_cgroup program to make its usage in
test_progs framework possible. Update test_dev_cgroup.c as well to match
this change while it has not been removed.
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Link: https://lore.kernel.org/r/20240731-convert_dev_cgroup-v4-1-849425d90de6@bootlin.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Andrii Nakryiko [Tue, 30 Jul 2024 23:18:05 +0000 (16:18 -0700)]
selftests/bpf: fix RELEASE=1 compilation for sock_addr.c
When building selftests with RELEASE=1 using GCC compiler, it complaints
about uninitialized err. Fix the problem.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20240730231805.1933923-1-andrii@kernel.org
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Stanislav Fomichev [Fri, 26 Jul 2024 22:20:48 +0000 (15:20 -0700)]
xsk: Try to make xdp_umem_reg extension a bit more future-proof
We recently found out that extending xsk_umem_reg might be a bit
complicated due to not enforcing padding to be zero [0]. Add
a couple of things to make it less error-prone:
1. Remove xdp_umem_reg_v2 since its sizeof is the same as xdp_umem_reg
2. Add BUILD_BUG_ON that checks that the size of xdp_umem_reg_v1 is less
than xdp_umem_reg; presumably, when we get to v2, there is gonna
be a similar line to enforce that sizeof(v2) > sizeof(v1)
3. Add BUILD_BUG_ON to make sure the last field plus its size matches
the overall struct size. The intent is to demonstrate that we don't
have any lingering padding.
0: https://lore.kernel.org/bpf/ZqI29QE+5JnkdPmE@boxer/T/#me03113f7c2458fd08f3c4114a7a9472ac3646c98
Reported-by: Julian Schindel <mail@arctic-alpaca.de>
Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://lore.kernel.org/r/20240726222048.1397869-1-sdf@fomichev.me
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Menglong Dong [Tue, 30 Jul 2024 05:37:33 +0000 (13:37 +0800)]
bpf: kprobe: Remove unused declaring of bpf_kprobe_override
After the commit
66665ad2f102 ("tracing/kprobe: bpf: Compare instruction
pointer with original one"), "bpf_kprobe_override" is not used anywhere
anymore, and we can remove it now.
Fixes:
66665ad2f102 ("tracing/kprobe: bpf: Compare instruction pointer with original one")
Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240730053733.885785-1-dongml2@chinatelecom.cn
Tony Ambardar [Mon, 29 Jul 2024 09:24:24 +0000 (02:24 -0700)]
selftests/bpf: Fix error compiling tc_redirect.c with musl libc
Linux 5.1 implemented 64-bit time types and related syscalls to address the
Y2038 problem generally across archs. Userspace handling of Y2038 varies
with the libc however. While musl libc uses 64-bit time across all 32-bit
and 64-bit platforms, GNU glibc uses 64-bit time on 64-bit platforms but
defaults to 32-bit time on 32-bit platforms unless they "opt-in" to 64-bit
time or explicitly use 64-bit syscalls and time structures.
One specific area is the standard setsockopt() call, SO_TIMESTAMPNS option
used for timestamping, and the related output 'struct timespec'. GNU glibc
defaults as above, also exposing the SO_TIMESTAMPNS_NEW flag to explicitly
use a 64-bit call and 'struct __kernel_timespec'. Since these are not
exposed or needed with musl libc, their use in tc_redirect.c leads to
compile errors building for mips64el/musl:
tc_redirect.c: In function 'rcv_tstamp':
tc_redirect.c:425:32: error: 'SO_TIMESTAMPNS_NEW' undeclared (first use in this function); did you mean 'SO_TIMESTAMPNS'?
425 | cmsg->cmsg_type == SO_TIMESTAMPNS_NEW)
| ^~~~~~~~~~~~~~~~~~
| SO_TIMESTAMPNS
tc_redirect.c:425:32: note: each undeclared identifier is reported only once for each function it appears in
tc_redirect.c: In function 'test_inet_dtime':
tc_redirect.c:491:49: error: 'SO_TIMESTAMPNS_NEW' undeclared (first use in this function); did you mean 'SO_TIMESTAMPNS'?
491 | err = setsockopt(listen_fd, SOL_SOCKET, SO_TIMESTAMPNS_NEW,
| ^~~~~~~~~~~~~~~~~~
| SO_TIMESTAMPNS
However, using SO_TIMESTAMPNS_NEW isn't strictly needed, nor is Y2038 being
explicitly tested. The timestamp checks in tc_redirect.c are simple: the
packet receive timestamp is non-zero and processed/handled in less than 5
seconds.
Switch to using the standard setsockopt() call and SO_TIMESTAMPNS option to
ensure compatibility across glibc and musl libc. In the worst-case, there
is a 5-second window 14 years from now where tc_redirect tests may fail on
32-bit systems. However, we should reasonably expect glibc to adopt a
64-bit mandate rather than the current "opt-in" policy before the Y2038
roll-over.
Fixes:
ce6f6cffaeaa ("selftests/bpf: Wait for the netstamp_needed_key static key to be turned on")
Fixes:
c803475fd8dd ("bpf: selftests: test skb->tstamp in redirect_neigh")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/031d656c058b4e55ceae56ef49c4e1729b5090f3.1722244708.git.tony.ambardar@gmail.com
Tony Ambardar [Mon, 29 Jul 2024 09:24:23 +0000 (02:24 -0700)]
selftests/bpf: Fix using stdout, stderr as struct field names
Typically stdin, stdout, stderr are treated as reserved identifiers under
ISO/ANSI C and libc implementations further define these as macros, both in
glibc and musl <stdio.h>.
However, while glibc defines:
...
/* Standard streams. */
extern FILE *stdin; /* Standard input stream. */
extern FILE *stdout; /* Standard output stream. */
extern FILE *stderr; /* Standard error output stream. */
/* C89/C99 say they're macros. Make them happy. */
#define stdin stdin
#define stdout stdout
#define stderr stderr
...
musl instead uses (legally):
...
extern FILE *const stdin;
extern FILE *const stdout;
extern FILE *const stderr;
#define stdin (stdin)
#define stdout (stdout)
#define stderr (stderr)
...
The latter results in compile errors when the names are reused as fields of
'struct test_env' and elsewhere in test_progs.[ch] and reg_bounds.c.
Rename the fields to stdout_saved and stderr_saved to avoid many errors
seen building against musl, e.g.:
In file included from test_progs.h:6,
from test_progs.c:5:
test_progs.c: In function 'print_test_result':
test_progs.c:237:21: error: expected identifier before '(' token
237 | fprintf(env.stdout, "#%-*d %s:", TEST_NUM_WIDTH, test->test_num, test->test_name);
| ^~~~~~
test_progs.c:237:9: error: too few arguments to function 'fprintf'
237 | fprintf(env.stdout, "#%-*d %s:", TEST_NUM_WIDTH, test->test_num, test->test_name);
| ^~~~~~~
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/ZqR2DuHdBXPX%2Fyx8@kodidev-ubuntu/
Link: https://lore.kernel.org/bpf/684ea17548e237f39dfb3f7a3d33450069015b21.1722244708.git.tony.ambardar@gmail.com
Tony Ambardar [Mon, 29 Jul 2024 09:24:22 +0000 (02:24 -0700)]
selftests/bpf: Fix compile if backtrace support missing in libc
Include GNU <execinfo.h> header only with glibc and provide weak, stubbed
backtrace functions as a fallback in test_progs.c. This allows for non-GNU
replacements while avoiding compile errors (e.g. with musl libc) like:
test_progs.c:13:10: fatal error: execinfo.h: No such file or directory
13 | #include <execinfo.h> /* backtrace */
| ^~~~~~~~~~~~
test_progs.c: In function 'crash_handler':
test_progs.c:1034:14: error: implicit declaration of function 'backtrace' [-Werror=implicit-function-declaration]
1034 | sz = backtrace(bt, ARRAY_SIZE(bt));
| ^~~~~~~~~
test_progs.c:1045:9: error: implicit declaration of function 'backtrace_symbols_fd' [-Werror=implicit-function-declaration]
1045 | backtrace_symbols_fd(bt, sz, STDERR_FILENO);
| ^~~~~~~~~~~~~~~~~~~~
Fixes:
9fb156bb82a3 ("selftests/bpf: Print backtrace on SIGSEGV in test_progs")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/aa6dc8e23710cb457b278039d0081de7e7b4847d.1722244708.git.tony.ambardar@gmail.com
Tony Ambardar [Mon, 29 Jul 2024 09:24:21 +0000 (02:24 -0700)]
selftests/bpf: Fix redefinition errors compiling lwt_reroute.c
Compiling lwt_reroute.c with GCC 12.3 for mips64el/musl-libc yields errors:
In file included from .../include/arpa/inet.h:9,
from ./test_progs.h:18,
from tools/testing/selftests/bpf/prog_tests/lwt_helpers.h:11,
from tools/testing/selftests/bpf/prog_tests/lwt_reroute.c:52:
.../include/netinet/in.h:23:8: error: redefinition of 'struct in6_addr'
23 | struct in6_addr {
| ^~~~~~~~
In file included from .../include/linux/icmp.h:24,
from tools/testing/selftests/bpf/prog_tests/lwt_helpers.h:9:
.../include/linux/in6.h:33:8: note: originally defined here
33 | struct in6_addr {
| ^~~~~~~~
.../include/netinet/in.h:34:8: error: redefinition of 'struct sockaddr_in6'
34 | struct sockaddr_in6 {
| ^~~~~~~~~~~~
.../include/linux/in6.h:50:8: note: originally defined here
50 | struct sockaddr_in6 {
| ^~~~~~~~~~~~
.../include/netinet/in.h:42:8: error: redefinition of 'struct ipv6_mreq'
42 | struct ipv6_mreq {
| ^~~~~~~~~
.../include/linux/in6.h:60:8: note: originally defined here
60 | struct ipv6_mreq {
| ^~~~~~~~~
These errors occur because <linux/in6.h> is included before <netinet/in.h>,
bypassing the Linux uapi/libc compat mechanism's partial musl support. As
described in [1] and [2], fix these errors by including <netinet/in.h> in
lwt_reroute.c before any uapi headers.
[1]: commit
c0bace798436 ("uapi libc compat: add fallback for unsupported libcs")
[2]: https://git.musl-libc.org/cgit/musl/commit/?id=
04983f227238
Fixes:
6c77997bc639 ("selftests/bpf: Add lwt_xmit tests for BPF_REROUTE")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/bd2908aec0755ba8b75f5dc41848b00585f5c73e.1722244708.git.tony.ambardar@gmail.com
Tony Ambardar [Mon, 29 Jul 2024 09:24:20 +0000 (02:24 -0700)]
selftests/bpf: Fix C++ compile error from missing _Bool type
While building, bpftool makes a skeleton from test_core_extern.c, which
itself includes <stdbool.h> and uses the 'bool' type. However, the skeleton
test_core_extern.skel.h generated *does not* include <stdbool.h> or use the
'bool' type, instead using the C-only '_Bool' type. Compiling test_cpp.cpp
with g++ 12.3 for mips64el/musl-libc then fails with error:
In file included from test_cpp.cpp:9:
test_core_extern.skel.h:45:17: error: '_Bool' does not name a type
45 | _Bool CONFIG_BOOL;
| ^~~~~
This was likely missed previously because glibc uses a GNU extension for
<stdbool.h> with C++ (#define _Bool bool), not supported by musl libc.
Normally, a C fragment would include <stdbool.h> and use the 'bool' type,
and thus cleanly work after import by C++. The ideal fix would be for
'bpftool gen skeleton' to output the correct type/include supporting C++,
but in the meantime add a conditional define as above.
Fixes:
7c8dce4b1661 ("bpftool: Make skeleton C code compilable with C++ compiler")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/6fc1dd28b8bda49e51e4f610bdc9d22f4455632d.1722244708.git.tony.ambardar@gmail.com
Tony Ambardar [Mon, 29 Jul 2024 09:24:19 +0000 (02:24 -0700)]
selftests/bpf: Fix error compiling test_lru_map.c
Although the post-increment in macro 'CPU_SET(next++, &cpuset)' seems safe,
the sequencing can raise compile errors, so move the increment outside the
macro. This avoids an error seen using gcc 12.3.0 for mips64el/musl-libc:
In file included from test_lru_map.c:11:
test_lru_map.c: In function 'sched_next_online':
test_lru_map.c:129:29: error: operation on 'next' may be undefined [-Werror=sequence-point]
129 | CPU_SET(next++, &cpuset);
| ^
cc1: all warnings being treated as errors
Fixes:
3fbfadce6012 ("bpf: Fix test_lru_sanity5() in test_lru_map.c")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/22993dfb11ccf27925a626b32672fd3324cb76c4.1722244708.git.tony.ambardar@gmail.com
Tony Ambardar [Mon, 29 Jul 2024 09:24:18 +0000 (02:24 -0700)]
selftests/bpf: Fix arg parsing in veristat, test_progs
Current code parses arguments with strtok_r() using a construct like
char *state = NULL;
while ((next = strtok_r(state ? NULL : input, ",", &state))) {
...
}
where logic assumes the 'state' var can distinguish between first and
subsequent strtok_r() calls, and adjusts parameters accordingly. However,
'state' is strictly internal context for strtok_r() and no such assumptions
are supported in the man page. Moreover, the exact behaviour of 'state'
depends on the libc implementation, making the above code fragile.
Indeed, invoking "./test_progs -t <test_name>" on mips64el/musl will hang,
with the above code in an infinite loop.
Similarly, we see strange behaviour running 'veristat' on mips64el/musl:
$ ./veristat -e file,prog,verdict,insns -C two-ok add-failure
Can't specify more than 9 stats
Rewrite code using a counter to distinguish between strtok_r() calls.
Fixes:
61ddff373ffa ("selftests/bpf: Improve by-name subtest selection logic in prog_tests")
Fixes:
394169b079b5 ("selftests/bpf: add comparison mode to veristat")
Fixes:
c8bc5e050976 ("selftests/bpf: Add veristat tool for mass-verifying BPF object files")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/392d8bf5559f85fa37926c1494e62312ef252c3d.1722244708.git.tony.ambardar@gmail.com
Tony Ambardar [Mon, 29 Jul 2024 09:24:17 +0000 (02:24 -0700)]
selftests/bpf: Use portable POSIX basename()
Use the POSIX version of basename() to allow compilation against non-gnu
libc (e.g. musl). Include <libgen.h> ahead of <string.h> to enable using
functions from the latter while preferring POSIX over GNU basename().
In veristat.c, rely on strdupa() to avoid basename() altering the passed
"const char" argument. This is not needed in xskxceiver.c since the arg
is mutable and the program exits immediately after usage.
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/0fd3c9f3c605e6cba33504213c9df287817ade04.1722244708.git.tony.ambardar@gmail.com
Zhu Jun [Wed, 24 Jul 2024 11:11:20 +0000 (04:11 -0700)]
tools/bpf: Fix the wrong format specifier
The format specifier of "unsigned int" in printf() should be "%u", not
"%d".
Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Quentin Monnet <qmo@kernel.org>
Link: https://lore.kernel.org/bpf/20240724111120.11625-1-zhujun2@cmss.chinamobile.com
David Vernet [Thu, 25 Jul 2024 03:22:14 +0000 (22:22 -0500)]
selftests/bpf: Load struct_ops map in global_maps_resize test
In prog_tests/test_global_maps_resize.c, we test various use cases for
resizing global maps. Commit
7244100e0389 ("libbpf: Don't take direct
pointers into BTF data from st_ops") updated libbpf to not store pointers
to volatile BTF data, which for some users, was causing a UAF when resizing
a datasec array.
Let's ensure we have coverage for resizing datasec arrays with struct_ops
progs by also including a struct_ops map and struct_ops prog in the
test_global_map_resize skeleton. The map is automatically loaded, so we
don't need to do anything other than add it to the BPF prog being tested
to get the coverage.
Signed-off-by: David Vernet <void@manifault.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240725032214.50676-1-void@manifault.com
Alexis Lothoré (eBPF Foundation) [Sun, 21 Jul 2024 19:33:04 +0000 (21:33 +0200)]
selftests/bpf: Integrate test_xdp_veth into test_progs
test_xdp_veth.sh tests that XDP return codes work as expected, by bringing
up multiple veth pairs isolated in different namespaces, attaching specific
xdp programs to each interface, and ensuring that the whole chain allows to
ping one end interface from the first one. The test runs well but is
currently not integrated in test_progs, which prevents it from being run
automatically in the CI infrastructure.
Rewrite it as a C test relying on libbpf to allow running it in the CI
infrastructure. The new code brings up the same network infrastructure and
reuses the same eBPF programs as test_xdp_veth.sh, for which skeletons are
already generated by the bpf tests makefile.
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://lore.kernel.org/bpf/20240721-convert_test_xdp_veth-v4-2-23bdba21b2f9@bootlin.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Alexis Lothoré (eBPF Foundation) [Sun, 21 Jul 2024 19:33:03 +0000 (21:33 +0200)]
selftests/bpf: Update xdp_redirect_map prog sections for libbpf
xdp_redirect_map.c is a bpf program used by test_xdp_veth.sh, which is not
handled by the generic test runner (test_progs). To allow converting this
test to test_progs, the corresponding program must be updated to allow
handling it through skeletons generated by bpftool and libbpf.
Update programs section names to allow to manipulate those with libbpf.
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://lore.kernel.org/bpf/20240721-convert_test_xdp_veth-v4-1-23bdba21b2f9@bootlin.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
David Vernet [Wed, 24 Jul 2024 17:14:58 +0000 (12:14 -0500)]
libbpf: Don't take direct pointers into BTF data from st_ops
In struct bpf_struct_ops, we have take a pointer to a BTF type name, and
a struct btf_type. This was presumably done for convenience, but can
actually result in subtle and confusing bugs given that BTF data can be
invalidated before a program is loaded. For example, in sched_ext, we
may sometimes resize a data section after a skeleton has been opened,
but before the struct_ops scheduler map has been loaded. This may cause
the BTF data to be realloc'd, which can then cause a UAF when loading
the program because the struct_ops map has pointers directly into the
BTF data.
We're already storing the BTF type_id in struct bpf_struct_ops. Because
type_id is stable, we can therefore just update the places where we were
looking at those pointers to instead do the lookups we need from the
type_id.
Fixes:
590a00888250 ("bpf: libbpf: Add STRUCT_OPS support")
Signed-off-by: David Vernet <void@manifault.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240724171459.281234-1-void@manifault.com
Andrii Nakryiko [Wed, 24 Jul 2024 23:46:05 +0000 (16:46 -0700)]
Merge branch 'selftests-bpf-improve-libc-portability-musl-support-part-1'
Tony Ambardar says:
====================
selftests/bpf: Improve libc portability / musl support (part 1)
Hello all,
This series includes the bulk of libc-related compile fixes accumulated to
support systems using musl, with smaller numbers to follow. These patches
are simple and straightforward, and the series has been tested with the
kernel-patches/bpf CI and locally using mips64el-gcc/musl-libc and QEMU
with an OpenWrt rootfs.
The patches address a few general categories of libc portability issues:
- missing, redundant or incorrect include headers
- disabled GNU header extensions (i.e. missing #define _GNU_SOURCE)
- issues with types and casting
Feedback and suggestions for improvement are welcome!
Thanks,
Tony
====================
Link: https://lore.kernel.org/r/cover.1721713597.git.tony.ambardar@gmail.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Tony Ambardar [Tue, 23 Jul 2024 05:54:46 +0000 (22:54 -0700)]
selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc
Remove a redundant include of '<asm/types.h>', whose needed definitions are
already included (via '<linux/types.h>') in cg_storage_multi_egress_only.c,
cg_storage_multi_isolated.c, and cg_storage_multi_shared.c. This avoids
redefinition errors seen compiling for mips64el/musl-libc like:
In file included from progs/cg_storage_multi_egress_only.c:13:
In file included from progs/cg_storage_multi.h:6:
In file included from /usr/mips64el-linux-gnuabi64/include/asm/types.h:23:
/usr/include/asm-generic/int-l64.h:29:25: error: typedef redefinition with different types ('long' vs 'long long')
29 | typedef __signed__ long __s64;
| ^
/usr/include/asm-generic/int-ll64.h:30:44: note: previous definition is here
30 | __extension__ typedef __signed__ long long __s64;
| ^
Fixes:
9e5bd1f7633b ("selftests/bpf: Test CGROUP_STORAGE map can't be used by multiple progs")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/4f4702e9f6115b7f84fea01b2326ca24c6df7ba8.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:45 +0000 (22:54 -0700)]
selftests/bpf: Fix errors compiling crypto_sanity.c with musl libc
Remove a redundant include of '<linux/in6.h>', whose needed definitions are
already provided by 'test_progs.h'. This avoids errors seen compiling for
mips64el/musl-libc:
In file included from .../arpa/inet.h:9,
from ./test_progs.h:17,
from prog_tests/crypto_sanity.c:10:
.../netinet/in.h:23:8: error: redefinition of 'struct in6_addr'
23 | struct in6_addr {
| ^~~~~~~~
In file included from crypto_sanity.c:7:
.../linux/in6.h:33:8: note: originally defined here
33 | struct in6_addr {
| ^~~~~~~~
.../netinet/in.h:34:8: error: redefinition of 'struct sockaddr_in6'
34 | struct sockaddr_in6 {
| ^~~~~~~~~~~~
.../linux/in6.h:50:8: note: originally defined here
50 | struct sockaddr_in6 {
| ^~~~~~~~~~~~
.../netinet/in.h:42:8: error: redefinition of 'struct ipv6_mreq'
42 | struct ipv6_mreq {
| ^~~~~~~~~
.../linux/in6.h:60:8: note: originally defined here
60 | struct ipv6_mreq {
| ^~~~~~~~~
Fixes:
91541ab192fc ("selftests: bpf: crypto skcipher algo selftests")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Link: https://lore.kernel.org/bpf/911293968f424ad7b462d8805aeb3baee8f4985b.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:44 +0000 (22:54 -0700)]
selftests/bpf: Fix errors compiling decap_sanity.c with musl libc
Remove a redundant include of '<linux/in6.h>', whose needed definitions are
already provided by 'test_progs.h'. This avoids errors seen compiling for
mips64el/musl-libc:
In file included from .../arpa/inet.h:9,
from ./test_progs.h:17,
from prog_tests/decap_sanity.c:9:
.../netinet/in.h:23:8: error: redefinition of 'struct in6_addr'
23 | struct in6_addr {
| ^~~~~~~~
In file included from decap_sanity.c:7:
.../linux/in6.h:33:8: note: originally defined here
33 | struct in6_addr {
| ^~~~~~~~
.../netinet/in.h:34:8: error: redefinition of 'struct sockaddr_in6'
34 | struct sockaddr_in6 {
| ^~~~~~~~~~~~
.../linux/in6.h:50:8: note: originally defined here
50 | struct sockaddr_in6 {
| ^~~~~~~~~~~~
.../netinet/in.h:42:8: error: redefinition of 'struct ipv6_mreq'
42 | struct ipv6_mreq {
| ^~~~~~~~~
.../linux/in6.h:60:8: note: originally defined here
60 | struct ipv6_mreq {
| ^~~~~~~~~
Fixes:
70a00e2f1dba ("selftests/bpf: Test bpf_skb_adjust_room on CHECKSUM_PARTIAL")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/e986ba2d7edccd254b54f7cd049b98f10bafa8c3.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:43 +0000 (22:54 -0700)]
selftests/bpf: Fix errors compiling lwt_redirect.c with musl libc
Remove a redundant include of '<linux/icmp.h>' which is already provided in
'lwt_helpers.h'. This avoids errors seen compiling for mips64el/musl-libc:
In file included from .../arpa/inet.h:9,
from lwt_redirect.c:51:
.../netinet/in.h:23:8: error: redefinition of 'struct in6_addr'
23 | struct in6_addr {
| ^~~~~~~~
In file included from .../linux/icmp.h:24,
from lwt_redirect.c:50:
.../linux/in6.h:33:8: note: originally defined here
33 | struct in6_addr {
| ^~~~~~~~
.../netinet/in.h:34:8: error: redefinition of 'struct sockaddr_in6'
34 | struct sockaddr_in6 {
| ^~~~~~~~~~~~
.../linux/in6.h:50:8: note: originally defined here
50 | struct sockaddr_in6 {
| ^~~~~~~~~~~~
.../netinet/in.h:42:8: error: redefinition of 'struct ipv6_mreq'
42 | struct ipv6_mreq {
| ^~~~~~~~~
.../linux/in6.h:60:8: note: originally defined here
60 | struct ipv6_mreq {
| ^~~~~~~~~
Fixes:
43a7c3ef8a15 ("selftests/bpf: Add lwt_xmit tests for BPF_REDIRECT")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/3869dda876d5206d2f8d4dd67331c739ceb0c7f8.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:42 +0000 (22:54 -0700)]
selftests/bpf: Fix compiling core_reloc.c with musl-libc
The type 'loff_t' is a GNU extension and not exposed by the musl 'fcntl.h'
header unless _GNU_SOURCE is defined. Add this definition to fix errors
seen compiling for mips64el/musl-libc:
In file included from tools/testing/selftests/bpf/prog_tests/core_reloc.c:4:
./bpf_testmod/bpf_testmod.h:10:9: error: unknown type name 'loff_t'
10 | loff_t off;
| ^~~~~~
./bpf_testmod/bpf_testmod.h:16:9: error: unknown type name 'loff_t'
16 | loff_t off;
| ^~~~~~
Fixes:
6bcd39d366b6 ("selftests/bpf: Add CO-RE relocs selftest relying on kernel module BTF")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/11c3af75a7eb6bcb7ad9acfae6a6f470c572eb82.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:41 +0000 (22:54 -0700)]
selftests/bpf: Fix compiling tcp_rtt.c with musl-libc
The GNU version of 'struct tcp_info' in 'netinet/tcp.h' is not exposed by
musl headers unless _GNU_SOURCE is defined.
Add this definition to fix errors seen compiling for mips64el/musl-libc:
tcp_rtt.c: In function 'wait_for_ack':
tcp_rtt.c:24:25: error: storage size of 'info' isn't known
24 | struct tcp_info info;
| ^~~~
tcp_rtt.c:24:25: error: unused variable 'info' [-Werror=unused-variable]
cc1: all warnings being treated as errors
Fixes:
1f4f80fed217 ("selftests/bpf: test_progs: convert test_tcp_rtt")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/f2329767b15df206f08a5776d35a47c37da855ae.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:40 +0000 (22:54 -0700)]
selftests/bpf: Fix compiling flow_dissector.c with musl-libc
The GNU version of 'struct tcphdr' has members 'doff', 'source' and 'dest',
which are not exposed by musl libc headers unless _GNU_SOURCE is defined.
Add this definition to fix errors seen compiling for mips64el/musl-libc:
flow_dissector.c:118:30: error: 'struct tcphdr' has no member named 'doff'
118 | .tcp.doff = 5,
| ^~~~
flow_dissector.c:119:30: error: 'struct tcphdr' has no member named 'source'
119 | .tcp.source = 80,
| ^~~~~~
flow_dissector.c:120:30: error: 'struct tcphdr' has no member named 'dest'
120 | .tcp.dest = 8080,
| ^~~~
Fixes:
ae173a915785 ("selftests/bpf: support BPF_FLOW_DISSECTOR_F_PARSE_1ST_FRAG")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/8f7ab21a73f678f9cebd32b26c444a686e57414d.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:39 +0000 (22:54 -0700)]
selftests/bpf: Fix compiling kfree_skb.c with musl-libc
The GNU version of 'struct tcphdr' with member 'doff' is not exposed by
musl headers unless _GNU_SOURCE is defined. Add this definition to fix
errors seen compiling for mips64el/musl-libc:
In file included from kfree_skb.c:2:
kfree_skb.c: In function 'on_sample':
kfree_skb.c:45:30: error: 'struct tcphdr' has no member named 'doff'
45 | if (CHECK(pkt_v6->tcp.doff != 5, "check_tcp",
| ^
Fixes:
580d656d80cf ("selftests/bpf: Add kfree_skb raw_tp test")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/e2d8cedc790959c10d6822a51f01a7a3616bea1b.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:38 +0000 (22:54 -0700)]
selftests/bpf: Fix compiling parse_tcp_hdr_opt.c with musl-libc
The GNU version of 'struct tcphdr', with members 'doff' and 'urg_ptr', is
not exposed by musl headers unless _GNU_SOURCE is defined.
Add this definition to fix errors seen compiling for mips64el/musl-libc:
parse_tcp_hdr_opt.c:18:21: error: 'struct tcphdr' has no member named 'urg_ptr'
18 | .pk6_v6.tcp.urg_ptr = 123,
| ^~~~~~~
parse_tcp_hdr_opt.c:19:21: error: 'struct tcphdr' has no member named 'doff'
19 | .pk6_v6.tcp.doff = 9, /* 16 bytes of options */
| ^~~~
Fixes:
cfa7b011894d ("selftests/bpf: tests for using dynptrs to parse skb and xdp buffers")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/ac5440213c242c62cb4e0d9e0a9cd5058b6a31f6.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:37 +0000 (22:54 -0700)]
selftests/bpf: Fix include of <sys/fcntl.h>
Update ns_current_pid_tgid.c to use '#include <fcntl.h>' and avoid compile
error against mips64el/musl libc:
In file included from .../prog_tests/ns_current_pid_tgid.c:14:
.../include/sys/fcntl.h:1:2: error: #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> [-Werror=cpp]
1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>
| ^~~~~~~
cc1: all warnings being treated as errors
Fixes:
09c02d553c49 ("bpf, selftests: Fold test_current_pid_tgid_new_ns into test_progs.")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/8bdc869749177b575025bf69600a4ce591822609.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:36 +0000 (22:54 -0700)]
selftests/bpf: Fix missing BUILD_BUG_ON() declaration
Explicitly include '<linux/build_bug.h>' to fix errors seen compiling with
gcc targeting mips64el/musl-libc:
user_ringbuf.c: In function 'test_user_ringbuf_loop':
user_ringbuf.c:426:9: error: implicit declaration of function 'BUILD_BUG_ON' [-Werror=implicit-function-declaration]
426 | BUILD_BUG_ON(total_samples <= c_max_entries);
| ^~~~~~~~~~~~
cc1: all warnings being treated as errors
Fixes:
e5a9df51c746 ("selftests/bpf: Add selftests validating the user ringbuf")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/b28575f9221ec54871c46a2e87612bb4bbf46ccd.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:35 +0000 (22:54 -0700)]
selftests/bpf: Fix missing UINT_MAX definitions in benchmarks
Include <limits.h> in 'bench.h' to provide a UINT_MAX definition and avoid
multiple compile errors against mips64el/musl-libc like:
benchs/bench_local_storage.c: In function 'parse_arg':
benchs/bench_local_storage.c:40:38: error: 'UINT_MAX' undeclared (first use in this function)
40 | if (ret < 1 || ret > UINT_MAX) {
| ^~~~~~~~
benchs/bench_local_storage.c:11:1: note: 'UINT_MAX' is defined in header '<limits.h>'; did you forget to '#include <limits.h>'?
10 | #include <test_btf.h>
+++ |+#include <limits.h>
11 |
seen with bench_local_storage.c, bench_local_storage_rcu_tasks_trace.c, and
bench_bpf_hashmap_lookup.c.
Fixes:
73087489250d ("selftests/bpf: Add benchmark for local_storage get")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/8f64a9d9fcff40a7fca090a65a68a9b62a468e16.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:34 +0000 (22:54 -0700)]
selftests/bpf: Fix missing ARRAY_SIZE() definition in bench.c
Add a "bpf_util.h" include to avoid the following error seen compiling for
mips64el with musl libc:
bench.c: In function 'find_benchmark':
bench.c:590:25: error: implicit declaration of function 'ARRAY_SIZE' [-Werror=implicit-function-declaration]
590 | for (i = 0; i < ARRAY_SIZE(benchs); i++) {
| ^~~~~~~~~~
cc1: all warnings being treated as errors
Fixes:
8e7c2a023ac0 ("selftests/bpf: Add benchmark runner infrastructure")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/bc4dde77dfcd17a825d8f28f72f3292341966810.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:31 +0000 (22:54 -0700)]
selftests/bpf: Drop unneeded error.h includes
The addition of general support for unprivileged tests in test_loader.c
breaks building test_verifier on non-glibc (e.g. musl) systems, due to the
inclusion of glibc extension '<error.h>' in 'unpriv_helpers.c'. However,
the header is actually not needed, so remove it to restore building.
Similarly for sk_lookup.c and flow_dissector.c, error.h is not necessary
and causes problems, so drop them.
Fixes:
1d56ade032a4 ("selftests/bpf: Unprivileged tests for test_loader.c")
Fixes:
0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach point")
Fixes:
0905beec9f52 ("selftests/bpf: run flow dissector tests in skb-less mode")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/5664367edf5fea4f3f4b4aec3b182bcfc6edff9c.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:30 +0000 (22:54 -0700)]
selftests/bpf: Fix error compiling bpf_iter_setsockopt.c with musl libc
Existing code calls getsockname() with a 'struct sockaddr_in6 *' argument
where a 'struct sockaddr *' argument is declared, yielding compile errors
when building for mips64el/musl-libc:
bpf_iter_setsockopt.c: In function 'get_local_port':
bpf_iter_setsockopt.c:98:30: error: passing argument 2 of 'getsockname' from incompatible pointer type [-Werror=incompatible-pointer-types]
98 | if (!getsockname(fd, &addr, &addrlen))
| ^~~~~
| |
| struct sockaddr_in6 *
In file included from .../netinet/in.h:10,
from .../arpa/inet.h:9,
from ./test_progs.h:17,
from bpf_iter_setsockopt.c:5:
.../sys/socket.h:391:23: note: expected 'struct sockaddr * restrict' but argument is of type 'struct sockaddr_in6 *'
391 | int getsockname (int, struct sockaddr *__restrict, socklen_t *__restrict);
| ^
cc1: all warnings being treated as errors
This compiled under glibc only because the argument is declared to be a
"funky" transparent union which includes both types above. Explicitly cast
the argument to allow compiling for both musl and glibc.
Fixes:
eed92afdd14c ("bpf: selftest: Test batching and bpf_(get|set)sockopt in bpf tcp iter")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Geliang Tang <geliang@kernel.org>
Link: https://lore.kernel.org/bpf/f41def0f17b27a23b1709080e4e3f37f4cc11ca9.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:29 +0000 (22:54 -0700)]
selftests/bpf: Fix compile error from rlim_t in sk_storage_map.c
Cast 'rlim_t' argument to match expected type of printf() format and avoid
compile errors seen building for mips64el/musl-libc:
In file included from map_tests/sk_storage_map.c:20:
map_tests/sk_storage_map.c: In function 'test_sk_storage_map_stress_free':
map_tests/sk_storage_map.c:414:56: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'rlim_t' {aka 'long long unsigned int'} [-Werror=format=]
414 | CHECK(err, "setrlimit(RLIMIT_NOFILE)", "rlim_new:%lu errno:%d",
| ^~~~~~~~~~~~~~~~~~~~~~~
415 | rlim_new.rlim_cur, errno);
| ~~~~~~~~~~~~~~~~~
| |
| rlim_t {aka long long unsigned int}
./test_maps.h:12:24: note: in definition of macro 'CHECK'
12 | printf(format); \
| ^~~~~~
map_tests/sk_storage_map.c:414:68: note: format string is defined here
414 | CHECK(err, "setrlimit(RLIMIT_NOFILE)", "rlim_new:%lu errno:%d",
| ~~^
| |
| long unsigned int
| %llu
cc1: all warnings being treated as errors
Fixes:
51a0e301a563 ("bpf: Add BPF_MAP_TYPE_SK_STORAGE test to test_maps")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/1e00a1fa7acf91b4ca135c4102dc796d518bad86.1721713597.git.tony.ambardar@gmail.com
Tony Ambardar [Tue, 23 Jul 2024 05:54:28 +0000 (22:54 -0700)]
selftests/bpf: Use pid_t consistently in test_progs.c
Use pid_t rather than __pid_t when allocating memory for 'worker_pids' in
'struct test_env', as this is its declared type and also avoids compile
errors seen building against musl libc on mipsel64:
test_progs.c:1738:49: error: '__pid_t' undeclared (first use in this function); did you mean 'pid_t'?
1738 | env.worker_pids = calloc(sizeof(__pid_t), env.workers);
| ^~~~~~~
| pid_t
test_progs.c:1738:49: note: each undeclared identifier is reported only once for each function it appears in
Fixes:
91b2c0afd00c ("selftests/bpf: Add parallelism to test_progs")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Geliang Tang <geliang@kernel.org>
Link: https://lore.kernel.org/bpf/c6447da51a94babc1931711a43e2ceecb135c93d.1721713597.git.tony.ambardar@gmail.com
Ihor Solodrai [Tue, 23 Jul 2024 20:57:43 +0000 (20:57 +0000)]
Alexei Starovoitov [Wed, 24 Jul 2024 03:47:07 +0000 (20:47 -0700)]
Merge branch 'no_caller_saved_registers-attribute-for-helper-calls'
Eduard Zingerman says:
====================
no_caller_saved_registers attribute for helper calls
This patch-set seeks to allow using no_caller_saved_registers gcc/clang
attribute with some BPF helper functions (and kfuncs in the future).
As documented in [1], this attribute means that function scratches
only some of the caller saved registers defined by ABI.
For BPF the set of such registers could be defined as follows:
- R0 is scratched only if function is non-void;
- R1-R5 are scratched only if corresponding parameter type is defined
in the function prototype.
The goal of the patch-set is to implement no_caller_saved_registers
(nocsr for short) in a backwards compatible manner:
- for kernels that support the feature, gain some performance boost
from better register allocation;
- for kernels that don't support the feature, allow programs execution
with minor performance losses.
To achieve this, use a scheme suggested by Alexei Starovoitov:
- for nocsr calls clang allocates registers as-if relevant r0-r5
registers are not scratched by the call;
- as a post-processing step, clang visits each nocsr call and adds
spill/fill for every live r0-r5;
- stack offsets used for spills/fills are allocated as lowest
stack offsets in whole function and are not used for any other
purpose;
- when kernel loads a program, it looks for such patterns
(nocsr function surrounded by spills/fills) and checks if
spill/fill stack offsets are used exclusively in nocsr patterns;
- if so, and if current JIT inlines the call to the nocsr function
(e.g. a helper call), kernel removes unnecessary spill/fill pairs;
- when old kernel loads a program, presence of spill/fill pairs
keeps BPF program valid, albeit slightly less efficient.
Corresponding clang/llvm changes are available in [2].
The patch-set uses bpf_get_smp_processor_id() function as a canary,
making it the first helper with nocsr attribute.
For example, consider the following program:
#define __no_csr __attribute__((no_caller_saved_registers))
#define SEC(name) __attribute__((section(name), used))
#define bpf_printk(fmt, ...) bpf_trace_printk((fmt), sizeof(fmt), __VA_ARGS__)
typedef unsigned int __u32;
static long (* const bpf_trace_printk)(const char *fmt, __u32 fmt_size, ...) = (void *) 6;
static __u32 (*const bpf_get_smp_processor_id)(void) __no_csr = (void *)8;
SEC("raw_tp")
int test(void *ctx)
{
__u32 task = bpf_get_smp_processor_id();
bpf_printk("ctx=%p, smp=%d", ctx, task);
return 0;
}
char _license[] SEC("license") = "GPL";
Compiled (using [2]) as follows:
$ clang --target=bpf -O2 -g -c -o nocsr.bpf.o nocsr.bpf.c
$ llvm-objdump --no-show-raw-insn -Sd nocsr.bpf.o
...
3rd parameter for printk call removable spill/fill pair
.--- 0: r3 = r1 |
; | __u32 task = bpf_get_smp_processor_id(); |
| 1: *(u64 *)(r10 - 0x8) = r3 <----------|
| 2: call 0x8 |
| 3: r3 = *(u64 *)(r10 - 0x8) <----------'
; | bpf_printk("ctx=%p, smp=%d", ctx, task);
| 4: r1 = 0x0 ll
| 6: r2 = 0xf
| 7: r4 = r0
'--> 8: call 0x6
; return 0;
9: r0 = 0x0
10: exit
Here is how the program looks after verifier processing:
# bpftool prog load ./nocsr.bpf.o /sys/fs/bpf/nocsr-test
# bpftool prog dump xlated pinned /sys/fs/bpf/nocsr-test
int test(void * ctx):
0: (bf) r3 = r1 <--- 3rd printk parameter
; __u32 task = bpf_get_smp_processor_id();
1: (b4) w0 = 197324 <--. inlined helper call,
2: (bf) r0 = &(void __percpu *)(r0) <--- spill/fill
3: (61) r0 = *(u32 *)(r0 +0) <--' pair removed
; bpf_printk("ctx=%p, smp=%d", ctx, task);
4: (18) r1 = map[id:5][0]+0
6: (b7) r2 = 15
7: (bf) r4 = r0
8: (85) call bpf_trace_printk#-125920
; return 0;
9: (b7) r0 = 0
10: (95) exit
[1] https://clang.llvm.org/docs/AttributeReference.html#no-caller-saved-registers
[2] https://github.com/eddyz87/llvm-project/tree/bpf-no-caller-saved-registers
Change list:
- v3 -> v4:
- When nocsr spills/fills are removed in the subprogram, allow these
spills/fills to reside in [-MAX_BPF_STACK-48..MAX_BPF_STACK) range
(suggested by Alexei);
- Dropped patches with special handling for bpf_probe_read_kernel()
(requested by Alexei);
- Reset aux .nocsr_pattern and .nocsr_spills_num fields in
check_nocsr_stack_contract() (requested by Andrii).
Andrii, I have not added an additional flag to
struct bpf_subprog_info, it currently does not have holes
and I really don't like adding a bool field there just as an
alternative indicator that nocsr is disabled.
Indicator at the moment:
- nocsr_stack_off >= S16_MIN means that nocsr rewrite is enabled;
- nocsr_stack_off == S16_MIN means that nocsr rewrite is disabled.
- v2 -> v3:
- As suggested by Andrii, 'nocsr_stack_off' is no longer checked at
rewrite time, instead mark_nocsr_patterns() now does two passes
over BPF program:
- on a first pass it computes the lowest stack spill offset for
the subprogram;
- on a second pass this offset is used to recognize nocsr pattern.
- As suggested by Alexei, a new mechanic is added to work around a
situation mentioned by Andrii, when more helper functions are
marked as nocsr at compile time than current kernel supports:
- all {spill*,helper call,fill*} patterns are now marked as
insn_aux_data[*].nocsr_pattern, thus relaxing failure condition
for check_nocsr_stack_contract();
- spill/fill pairs are not removed for patterns where helper can't
be inlined;
- see mark_nocsr_pattern_for_call() for details an example.
- As suggested by Alexei, subprogram stack depth is now adjusted
if all spill/fill pairs could be removed. This adjustment has
to take place before optimize_bpf_loop(), hence the rewrite
is moved from do_misc_fixups() to remove_nocsr_spills_fills()
(again).
- As suggested by Andrii, special measures are taken to work around
bpf_probe_read_kernel() access to BPF stack, see patches 11, 12.
Patch #11 is very simplistic, a more comprehensive solution would
be to change the type of the third parameter of the
bpf_probe_read_kernel() from ARG_ANYTHING to something else and
not only check nocsr contract, but also propagate stack slot
liveness information. However, such change would require update in
struct bpf_call_arg_meta processing, which currently implies that
every memory parameter is followed by a size parameter.
I can work on these changes, please comment.
- Stylistic changes suggested by Andrii.
- Added acks from Andrii.
- Dropped RFC tag.
- v1 -> v2:
- assume that functions inlined by either jit or verifier
conform to no_caller_saved_registers contract (Andrii, Puranjay);
- allow nocsr rewrite for bpf_get_smp_processor_id()
on arm64 and riscv64 architectures (Puranjay);
- __arch_{x86_64,arm64,riscv64} macro for test_loader;
- moved remove_nocsr_spills_fills() inside do_misc_fixups() (Andrii);
- moved nocsr pattern detection from check_cfg() to a separate pass
(Andrii);
- various stylistic/correctness changes according to Andrii's
comments.
Revisions:
- v1 https://lore.kernel.org/bpf/
20240629094733.
3863850-1-eddyz87@gmail.com/
- v2 https://lore.kernel.org/bpf/
20240704102402.
1644916-1-eddyz87@gmail.com/
- v3 https://lore.kernel.org/bpf/
20240715230201.
3901423-1-eddyz87@gmail.com/
====================
Link: https://lore.kernel.org/r/20240722233844.1406874-1-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:44 +0000 (16:38 -0700)]
selftests/bpf: test no_caller_saved_registers spill/fill removal
Tests for no_caller_saved_registers processing logic
(see verifier.c:match_and_mark_nocsr_pattern()):
- a canary positive test case;
- a canary test case for arm64 and riscv64;
- various tests with broken patterns;
- tests with read/write fixed/varying stack access that violate nocsr
stack access contract;
- tests with multiple subprograms;
- tests using nocsr in combination with may_goto/bpf_loop,
as all of these features affect stack depth;
- tests for nocsr stack spills below max stack depth.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-11-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:43 +0000 (16:38 -0700)]
selftests/bpf: __arch_* macro to limit test cases to specific archs
Add annotations __arch_x86_64, __arch_arm64, __arch_riscv64
to specify on which architecture the test case should be tested.
Several __arch_* annotations could be specified at once.
When test case is not run on current arch it is marked as skipped.
For example, the following would be tested only on arm64 and riscv64:
SEC("raw_tp")
__arch_arm64
__arch_riscv64
__xlated("1: *(u64 *)(r10 - 16) = r1")
__xlated("2: call")
__xlated("3: r1 = *(u64 *)(r10 - 16);")
__success
__naked void canary_arm64_riscv64(void)
{
asm volatile (
"r1 = 1;"
"*(u64 *)(r10 - 16) = r1;"
"call %[bpf_get_smp_processor_id];"
"r1 = *(u64 *)(r10 - 16);"
"exit;"
:
: __imm(bpf_get_smp_processor_id)
: __clobber_all);
}
On x86 it would be skipped:
#467/2 verifier_nocsr/canary_arm64_riscv64:SKIP
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-10-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:42 +0000 (16:38 -0700)]
selftests/bpf: allow checking xlated programs in verifier_* tests
Add a macro __xlated("...") for use with test_loader tests.
When such annotations are present for the test case:
- bpf_prog_get_info_by_fd() is used to get BPF program after all
rewrites are applied by verifier.
- the program is disassembled and patterns specified in __xlated are
searched for in the disassembly text.
__xlated matching follows the same mechanics as __msg:
each subsequent pattern is matched from the point where
previous pattern ended.
This allows to write tests like below, where the goal is to verify the
behavior of one of the of the transformations applied by verifier:
SEC("raw_tp")
__xlated("1: w0 = ")
__xlated("2: r0 = &(void __percpu *)(r0)")
__xlated("3: r0 = *(u32 *)(r0 +0)")
__xlated("4: exit")
__success __naked void simple(void)
{
asm volatile (
"call %[bpf_get_smp_processor_id];"
"exit;"
:
: __imm(bpf_get_smp_processor_id)
: __clobber_all);
}
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-9-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:41 +0000 (16:38 -0700)]
selftests/bpf: extract test_loader->expect_msgs as a data structure
Non-functional change: use a separate data structure to represented
expected messages in test_loader.
This would allow to use the same functionality for expected set of
disassembled instructions in the follow-up commit.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-8-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:40 +0000 (16:38 -0700)]
selftests/bpf: no need to track next_match_pos in struct test_loader
The call stack for validate_case() function looks as follows:
- test_loader__run_subtests()
- process_subtest()
- run_subtest()
- prepare_case(), which does 'tester->next_match_pos = 0';
- validate_case(), which increments tester->next_match_pos.
Hence, each subtest is run with next_match_pos freshly set to zero.
Meaning that there is no need to persist this variable in the
struct test_loader, use local variable instead.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-7-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:39 +0000 (16:38 -0700)]
selftests/bpf: print correct offset for pseudo calls in disasm_insn()
Adjust disasm_helpers.c:disasm_insn() to account for the following
part of the verifier.c:jit_subprogs:
for (i = 0, insn = prog->insnsi; i < prog->len; i++, insn++) {
/* ... */
if (!bpf_pseudo_call(insn))
continue;
insn->off = env->insn_aux_data[i].call_imm;
subprog = find_subprog(env, i + insn->off + 1);
insn->imm = subprog;
}
Where verifier moves offset of the subprogram to the insn->off field.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-6-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Yonghong Song [Tue, 23 Jul 2024 16:29:40 +0000 (09:29 -0700)]
selftests/bpf: Add reg_bounds tests for ldsx and subreg compare
Add a few reg_bounds selftests to test 32/16/8-bit ldsx and subreg comparison.
Without the previous patch, all added tests will fail.
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
Link: https://lore.kernel.org/r/20240723162940.2732171-1-yonghong.song@linux.dev
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:38 +0000 (16:38 -0700)]
selftests/bpf: extract utility function for BPF disassembly
struct bpf_insn *disasm_insn(struct bpf_insn *insn, char *buf, size_t buf_sz);
Disassembles instruction 'insn' to a text buffer 'buf'.
Removes insn->code hex prefix added by kernel disassembly routine.
Returns a pointer to the next instruction
(increments insn by either 1 or 2).
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-5-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Yonghong Song [Tue, 23 Jul 2024 16:29:33 +0000 (09:29 -0700)]
bpf: Get better reg range with ldsx and 32bit compare
With latest llvm19, the selftest iters/iter_arr_with_actual_elem_count
failed with -mcpu=v4.
The following are the details:
0: R1=ctx() R10=fp0
; int iter_arr_with_actual_elem_count(const void *ctx) @ iters.c:1420
0: (b4) w7 = 0 ; R7_w=0
; int i, n = loop_data.n, sum = 0; @ iters.c:1422
1: (18) r1 = 0xffffc90000191478 ; R1_w=map_value(map=iters.bss,ks=4,vs=1280,off=1144)
3: (61) r6 = *(u32 *)(r1 +128) ; R1_w=map_value(map=iters.bss,ks=4,vs=1280,off=1144) R6_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
; if (n > ARRAY_SIZE(loop_data.data)) @ iters.c:1424
4: (26) if w6 > 0x20 goto pc+27 ; R6_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=32,var_off=(0x0; 0x3f))
5: (bf) r8 = r10 ; R8_w=fp0 R10=fp0
6: (07) r8 += -8 ; R8_w=fp-8
; bpf_for(i, 0, n) { @ iters.c:1427
7: (bf) r1 = r8 ; R1_w=fp-8 R8_w=fp-8
8: (b4) w2 = 0 ; R2_w=0
9: (bc) w3 = w6 ; R3_w=scalar(id=1,smin=smin32=0,smax=umax=smax32=umax32=32,var_off=(0x0; 0x3f)) R6_w=scalar(id=1,smin=smin32=0,smax=umax=smax32=umax32=32,var_off=(0x0; 0x3f))
10: (85) call bpf_iter_num_new#45179 ; R0=scalar() fp-8=iter_num(ref_id=2,state=active,depth=0) refs=2
11: (bf) r1 = r8 ; R1=fp-8 R8=fp-8 refs=2
12: (85) call bpf_iter_num_next#45181 13: R0=rdonly_mem(id=3,ref_obj_id=2,sz=4) R6=scalar(id=1,smin=smin32=0,smax=umax=smax32=umax32=32,var_off=(0x0; 0x3f)) R7=0 R8=fp-8 R10=fp0 fp-8=iter_num(ref_id=2,state=active,depth=1) refs=2
; bpf_for(i, 0, n) { @ iters.c:1427
13: (15) if r0 == 0x0 goto pc+2 ; R0=rdonly_mem(id=3,ref_obj_id=2,sz=4) refs=2
14: (81) r1 = *(s32 *)(r0 +0) ; R0=rdonly_mem(id=3,ref_obj_id=2,sz=4) R1_w=scalar(smin=0xffffffff80000000,smax=0x7fffffff) refs=2
15: (ae) if w1 < w6 goto pc+4 20: R0=rdonly_mem(id=3,ref_obj_id=2,sz=4) R1=scalar(smin=0xffffffff80000000,smax=smax32=umax32=31,umax=0xffffffff0000001f,smin32=0,var_off=(0x0; 0xffffffff0000001f)) R6=scalar(id=1,smin=umin=smin32=umin32=1,smax=umax=smax32=umax32=32,var_off=(0x0; 0x3f)) R7=0 R8=fp-8 R10=fp0 fp-8=iter_num(ref_id=2,state=active,depth=1) refs=2
; sum += loop_data.data[i]; @ iters.c:1429
20: (67) r1 <<= 2 ; R1_w=scalar(smax=0x7ffffffc0000007c,umax=0xfffffffc0000007c,smin32=0,smax32=umax32=124,var_off=(0x0; 0xfffffffc0000007c)) refs=2
21: (18) r2 = 0xffffc90000191478 ; R2_w=map_value(map=iters.bss,ks=4,vs=1280,off=1144) refs=2
23: (0f) r2 += r1
math between map_value pointer and register with unbounded min value is not allowed
The source code:
int iter_arr_with_actual_elem_count(const void *ctx)
{
int i, n = loop_data.n, sum = 0;
if (n > ARRAY_SIZE(loop_data.data))
return 0;
bpf_for(i, 0, n) {
/* no rechecking of i against ARRAY_SIZE(loop_data.n) */
sum += loop_data.data[i];
}
return sum;
}
The insn #14 is a sign-extenstion load which is related to 'int i'.
The insn #15 did a subreg comparision. Note that smin=0xffffffff80000000 and this caused later
insn #23 failed verification due to unbounded min value.
Actually insn #15 R1 smin range can be better. Before insn #15, we have
R1_w=scalar(smin=0xffffffff80000000,smax=0x7fffffff)
With the above range, we know for R1, upper 32bit can only be 0xffffffff or 0.
Otherwise, the value range for R1 could be beyond [smin=0xffffffff80000000,smax=0x7fffffff].
After insn #15, for the true patch, we know smin32=0 and smax32=32. With the upper 32bit 0xffffffff,
then the corresponding value is [0xffffffff00000000, 0xffffffff00000020]. The range is
obviously beyond the original range [smin=0xffffffff80000000,smax=0x7fffffff] and the
range is not possible. So the upper 32bit must be 0, which implies smin = smin32 and
smax = smax32.
This patch fixed the issue by adding additional register deduction after 32-bit compare
insn. If the signed 32-bit register range is non-negative then 64-bit smin is
in range of [S32_MIN, S32_MAX], then the actual 64-bit smin/smax should be the same
as 32-bit smin32/smax32.
With this patch, iters/iter_arr_with_actual_elem_count succeeded with better register range:
from 15 to 20: R0=rdonly_mem(id=7,ref_obj_id=2,sz=4) R1_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=31,var_off=(0x0; 0x1f)) R6=scalar(id=1,smin=umin=smin32=umin32=1,smax=umax=smax32=umax32=32,var_off=(0x0; 0x3f)) R7=scalar(id=9,smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff)) R8=scalar(id=9,smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff)) R10=fp0 fp-8=iter_num(ref_id=2,state=active,depth=3) refs=2
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
Link: https://lore.kernel.org/r/20240723162933.2731620-1-yonghong.song@linux.dev
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:37 +0000 (16:38 -0700)]
bpf, x86, riscv, arm: no_caller_saved_registers for bpf_get_smp_processor_id()
The function bpf_get_smp_processor_id() is processed in a different
way, depending on the arch:
- on x86 verifier replaces call to bpf_get_smp_processor_id() with a
sequence of instructions that modify only r0;
- on riscv64 jit replaces call to bpf_get_smp_processor_id() with a
sequence of instructions that modify only r0;
- on arm64 jit replaces call to bpf_get_smp_processor_id() with a
sequence of instructions that modify only r0 and tmp registers.
These rewrites satisfy attribute no_caller_saved_registers contract.
Allow rewrite of no_caller_saved_registers patterns for
bpf_get_smp_processor_id() in order to use this function as a canary
for no_caller_saved_registers tests.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-4-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Yonghong Song [Tue, 23 Jul 2024 15:34:44 +0000 (08:34 -0700)]
selftests/bpf: Add tests for ldsx of pkt data/data_end/data_meta accesses
The following tests are added to verifier_ldsx.c:
- sign extension of data/data_end/data_meta for tcx programs.
The actual checking is in bpf_skb_is_valid_access() which
is called by sk_filter, cg_skb, lwt, tc(tcx) and sk_skb.
- sign extension of data/data_end/data_meta for xdp programs.
- sign extension of data/data_end for flow_dissector programs.
All newly-added tests have verification failure with message
"invalid bpf_context access". Without previous patch, all these
tests succeeded verification.
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
Link: https://lore.kernel.org/r/20240723153444.2430365-1-yonghong.song@linux.dev
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:36 +0000 (16:38 -0700)]
bpf: no_caller_saved_registers attribute for helper calls
GCC and LLVM define a no_caller_saved_registers function attribute.
This attribute means that function scratches only some of
the caller saved registers defined by ABI.
For BPF the set of such registers could be defined as follows:
- R0 is scratched only if function is non-void;
- R1-R5 are scratched only if corresponding parameter type is defined
in the function prototype.
This commit introduces flag bpf_func_prot->allow_nocsr.
If this flag is set for some helper function, verifier assumes that
it follows no_caller_saved_registers calling convention.
The contract between kernel and clang allows to simultaneously use
such functions and maintain backwards compatibility with old
kernels that don't understand no_caller_saved_registers calls
(nocsr for short):
- clang generates a simple pattern for nocsr calls, e.g.:
r1 = 1;
r2 = 2;
*(u64 *)(r10 - 8) = r1;
*(u64 *)(r10 - 16) = r2;
call %[to_be_inlined]
r2 = *(u64 *)(r10 - 16);
r1 = *(u64 *)(r10 - 8);
r0 = r1;
r0 += r2;
exit;
- kernel removes unnecessary spills and fills, if called function is
inlined by verifier or current JIT (with assumption that patch
inserted by verifier or JIT honors nocsr contract, e.g. does not
scratch r3-r5 for the example above), e.g. the code above would be
transformed to:
r1 = 1;
r2 = 2;
call %[to_be_inlined]
r0 = r1;
r0 += r2;
exit;
Technically, the transformation is split into the following phases:
- function mark_nocsr_patterns(), called from bpf_check()
searches and marks potential patterns in instruction auxiliary data;
- upon stack read or write access,
function check_nocsr_stack_contract() is used to verify if
stack offsets, presumably reserved for nocsr patterns, are used
only from those patterns;
- function remove_nocsr_spills_fills(), called from bpf_check(),
applies the rewrite for valid patterns.
See comment in mark_nocsr_pattern_for_call() for more details.
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-3-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Yonghong Song [Tue, 23 Jul 2024 15:34:39 +0000 (08:34 -0700)]
bpf: Fail verification for sign-extension of packet data/data_end/data_meta
syzbot reported a kernel crash due to
commit
1f1e864b6555 ("bpf: Handle sign-extenstin ctx member accesses").
The reason is due to sign-extension of 32-bit load for
packet data/data_end/data_meta uapi field.
The original code looks like:
r2 = *(s32 *)(r1 + 76) /* load __sk_buff->data */
r3 = *(u32 *)(r1 + 80) /* load __sk_buff->data_end */
r0 = r2
r0 += 8
if r3 > r0 goto +1
...
Note that __sk_buff->data load has 32-bit sign extension.
After verification and convert_ctx_accesses(), the final asm code looks like:
r2 = *(u64 *)(r1 +208)
r2 = (s32)r2
r3 = *(u64 *)(r1 +80)
r0 = r2
r0 += 8
if r3 > r0 goto pc+1
...
Note that 'r2 = (s32)r2' may make the kernel __sk_buff->data address invalid
which may cause runtime failure.
Currently, in C code, typically we have
void *data = (void *)(long)skb->data;
void *data_end = (void *)(long)skb->data_end;
...
and it will generate
r2 = *(u64 *)(r1 +208)
r3 = *(u64 *)(r1 +80)
r0 = r2
r0 += 8
if r3 > r0 goto pc+1
If we allow sign-extension,
void *data = (void *)(long)(int)skb->data;
void *data_end = (void *)(long)skb->data_end;
...
the generated code looks like
r2 = *(u64 *)(r1 +208)
r2 <<= 32
r2 s>>= 32
r3 = *(u64 *)(r1 +80)
r0 = r2
r0 += 8
if r3 > r0 goto pc+1
and this will cause verification failure since "r2 <<= 32" is not allowed
as "r2" is a packet pointer.
To fix this issue for case
r2 = *(s32 *)(r1 + 76) /* load __sk_buff->data */
this patch added additional checking in is_valid_access() callback
function for packet data/data_end/data_meta access. If those accesses
are with sign-extenstion, the verification will fail.
[1] https://lore.kernel.org/bpf/
000000000000c90eee061d236d37@google.com/
Reported-by: syzbot+ad9ec60c8eaf69e6f99c@syzkaller.appspotmail.com
Fixes:
1f1e864b6555 ("bpf: Handle sign-extenstin ctx member accesses")
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
Link: https://lore.kernel.org/r/20240723153439.2429035-1-yonghong.song@linux.dev
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Eduard Zingerman [Mon, 22 Jul 2024 23:38:35 +0000 (16:38 -0700)]
bpf: add a get_helper_proto() utility function
Extract the part of check_helper_call() as a utility function allowing
to query 'struct bpf_func_proto' for a specific helper function id.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20240722233844.1406874-2-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>