From: Yonghong Song <yhs(a)fb.com>
[ Upstream commit 7fb5eefd76394cfefb380724a87ca40b47d44405 ]
Andrii reported that with latest clang, when building selftests, we have
error likes:
error: progs/test_sysctl_loop1.c:23:16: in function sysctl_tcp_mem i32 (%struct.bpf_sysctl*):
Looks like the BPF stack limit of 512 bytes is exceeded.
Please move large on stack variables into BPF per-cpu array map.
The error is triggered by the following LLVM patch:
https://reviews.llvm.org/D87134
For example, the following code is from test_sysctl_loop1.c:
static __always_inline int is_tcp_mem(struct bpf_sysctl *ctx)
{
volatile char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string";
...
}
Without the above LLVM patch, the compiler did optimization to load the string
(59 bytes long) with 7 64bit loads, 1 8bit load and 1 16bit load,
occupying 64 byte stack size.
With the above LLVM patch, the compiler only uses 8bit loads, but subregister is 32bit.
So stack requirements become 4 * 59 = 236 bytes. Together with other stuff on
the stack, total stack size exceeds 512 bytes, hence compiler complains and quits.
To fix the issue, removing "volatile" key word or changing "volatile" to
"const"/"static const" does not work, the string is put in .rodata.str1.1 section,
which libbpf did not process it and errors out with
libbpf: elf: skipping unrecognized data section(6) .rodata.str1.1
libbpf: prog 'sysctl_tcp_mem': bad map relo against '.L__const.is_tcp_mem.tcp_mem_name'
in section '.rodata.str1.1'
Defining the string const as global variable can fix the issue as it puts the string constant
in '.rodata' section which is recognized by libbpf. In the future, when libbpf can process
'.rodata.str*.*' properly, the global definition can be changed back to local definition.
Defining tcp_mem_name as a global, however, triggered a verifier failure.
./test_progs -n 7/21
libbpf: load bpf program failed: Permission denied
libbpf: -- BEGIN DUMP LOG ---
libbpf:
invalid stack off=0 size=1
verification time 6975 usec
stack depth 160+64
processed 889 insns (limit 1000000) max_states_per_insn 4 total_states
14 peak_states 14 mark_read 10
libbpf: -- END LOG --
libbpf: failed to load program 'sysctl_tcp_mem'
libbpf: failed to load object 'test_sysctl_loop2.o'
test_bpf_verif_scale:FAIL:114
#7/21 test_sysctl_loop2.o:FAIL
This actually exposed a bpf program bug. In test_sysctl_loop{1,2}, we have code
like
const char tcp_mem_name[] = "<...long string...>";
...
char name[64];
...
for (i = 0; i < sizeof(tcp_mem_name); ++i)
if (name[i] != tcp_mem_name[i])
return 0;
In the above code, if sizeof(tcp_mem_name) > 64, name[i] access may be
out of bound. The sizeof(tcp_mem_name) is 59 for test_sysctl_loop1.c and
79 for test_sysctl_loop2.c.
Without promotion-to-global change, old compiler generates code where
the overflowed stack access is actually filled with valid value, so hiding
the bpf program bug. With promotion-to-global change, the code is different,
more specifically, the previous loading constants to stack is gone, and
"name" occupies stack[-64:0] and overflow access triggers a verifier error.
To fix the issue, adjust "name" buffer size properly.
Reported-by: Andrii Nakryiko <andriin(a)fb.com>
Signed-off-by: Yonghong Song <yhs(a)fb.com>
Signed-off-by: Alexei Starovoitov <ast(a)kernel.org>
Acked-by: Andrii Nakryiko <andriin(a)fb.com>
Link: https://lore.kernel.org/bpf/20200909171542.3673449-1-yhs@fb.com
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
tools/testing/selftests/bpf/progs/test_sysctl_loop1.c | 4 ++--
tools/testing/selftests/bpf/progs/test_sysctl_loop2.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c b/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
index 458b0d69133e4..553a282d816ab 100644
--- a/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
+++ b/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
@@ -18,11 +18,11 @@
#define MAX_ULONG_STR_LEN 7
#define MAX_VALUE_STR_LEN (TCP_MEM_LOOPS * MAX_ULONG_STR_LEN)
+const char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string";
static __always_inline int is_tcp_mem(struct bpf_sysctl *ctx)
{
- volatile char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string";
unsigned char i;
- char name[64];
+ char name[sizeof(tcp_mem_name)];
int ret;
memset(name, 0, sizeof(name));
diff --git a/tools/testing/selftests/bpf/progs/test_sysctl_loop2.c b/tools/testing/selftests/bpf/progs/test_sysctl_loop2.c
index b2e6f9b0894d8..2b64bc563a12e 100644
--- a/tools/testing/selftests/bpf/progs/test_sysctl_loop2.c
+++ b/tools/testing/selftests/bpf/progs/test_sysctl_loop2.c
@@ -18,11 +18,11 @@
#define MAX_ULONG_STR_LEN 7
#define MAX_VALUE_STR_LEN (TCP_MEM_LOOPS * MAX_ULONG_STR_LEN)
+const char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string_to_stress_byte_loop";
static __attribute__((noinline)) int is_tcp_mem(struct bpf_sysctl *ctx)
{
- volatile char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string_to_stress_byte_loop";
unsigned char i;
- char name[64];
+ char name[sizeof(tcp_mem_name)];
int ret;
memset(name, 0, sizeof(name));
--
2.25.1
Hi Linus,
Please pull the following Kunit fixes update for Linux 5.10-rc1
This Kunit fixes update consists of several kunit tool bug fixes in
flag handling, run outside kernel tree, make errors, and generating
results.
diff is attached.
thanks,
-- Shuah
----------------------------------------------------------------
The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5:
Linux 5.9-rc1 (2020-08-16 13:04:57 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest
tags/linux-kselftest-kunit-fixes-5.10-rc1
for you to fetch changes up to 1abdd39f14b25dd2d69096b624a4f86f158a9feb:
kunit: tool: fix display of make errors (2020-10-09 14:04:09 -0600)
----------------------------------------------------------------
linux-kselftest-kunit-fixes-5.10-rc1
This Kunit fixes update consists of several kunit tool bug fixes in
flag handling, run outside kernel tree, make errors, and generating
results.
----------------------------------------------------------------
Brendan Higgins (3):
kunit: tool: fix running kunit_tool from outside kernel tree
kunit: tool: fix --alltests flag
kunit: tool: handle when .kunit exists but .kunitconfig does not
Daniel Latypov (1):
kunit: tool: fix display of make errors
Heidi Fahim (1):
kunit: tool: allow generating test results in JSON
tools/testing/kunit/configs/broken_on_uml.config | 1 +
tools/testing/kunit/kunit.py | 58
+++++++++++++++-------
tools/testing/kunit/kunit_json.py | 63
++++++++++++++++++++++++
tools/testing/kunit/kunit_kernel.py | 27 +++++-----
tools/testing/kunit/kunit_tool_test.py | 33 +++++++++++++
5 files changed, 154 insertions(+), 28 deletions(-)
create mode 100644 tools/testing/kunit/kunit_json.py
----------------------------------------------------------------
Hi Linus,
Please pull the following Kselftest next update for Linux 5.10-rc1
This kselftest update for Linux 5.10-rc1 consists of enhancements to
-- speed up headers_install done during selftest build
-- add generic make nesting support
-- add support to select individual tests:
- Selftests build/install generates run_kselftest.sh script to run
selftests on a target system. Currently the script doesn't have
support for selecting individual tests. Add support for it.
With this enhancement, user can select test collections (or tests)
individually. e.g:
run_kselftest.sh -c seccomp -t timers:posix_timers -t timers:nanosleep
Additionally adds a way to list all known tests with "-l", usage
with "-h", and perform a dry run without running tests with "-n".
diff is attached.
thanks,
-- Shuah
----------------------------------------------------------------
The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5:
Linux 5.9-rc1 (2020-08-16 13:04:57 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest
tags/linux-kselftest-next-5.10-rc1
for you to fetch changes up to 4c8511317b4ec75cc3868f80a7b9fddb8322e512:
doc: dev-tools: kselftest.rst: Update examples and paths (2020-10-07
07:59:24 -0600)
----------------------------------------------------------------
linux-kselftest-next-5.10-rc1
This kselftest update for Linux 5.10-rc1 consists of enhancements to
-- speed up headers_install done during selftest build
-- add generic make nesting support
-- add support to select individual tests:
- Selftests build/install generates run_kselftest.sh script to run
selftests on a target system. Currently the script doesn't have
support for selecting individual tests. Add support for it.
With this enhancement, user can select test collections (or tests)
individually. e.g:
run_kselftest.sh -c seccomp -t timers:posix_timers -t timers:nanosleep
Additionally adds a way to list all known tests with "-l", usage
with "-h", and perform a dry run without running tests with "-n".
----------------------------------------------------------------
Denys Vlasenko (1):
selftests: use "$(MAKE)" instead of "make" for headers_install
Gabriel Krisman Bertazi (1):
selftests: Add missing gitignore entries
Greg Thelen (1):
selftests: more general make nesting support
Kees Cook (3):
selftests: Extract run_kselftest.sh and generate stand-alone test
list
selftests/run_kselftest.sh: Make each test individually selectable
doc: dev-tools: kselftest.rst: Update examples and paths
Documentation/dev-tools/kselftest.rst | 35 +++++++----
tools/testing/selftests/Makefile | 34 ++++------
tools/testing/selftests/firmware/.gitignore | 2 +
tools/testing/selftests/lib.mk | 9 ++-
tools/testing/selftests/netfilter/.gitignore | 2 +
tools/testing/selftests/ptrace/.gitignore | 1 +
tools/testing/selftests/run_kselftest.sh | 93
++++++++++++++++++++++++++++
7 files changed, 135 insertions(+), 41 deletions(-)
create mode 100644 tools/testing/selftests/firmware/.gitignore
create mode 100644 tools/testing/selftests/netfilter/.gitignore
create mode 100755 tools/testing/selftests/run_kselftest.sh
----------------------------------------------------------------
v2:
- update documentation
- include SPDX line in extracted script
v1: https://lore.kernel.org/linux-kselftest/20200925234527.1885234-1-keescook@c…
Hi!
I really like Hangbin Liu's intent[1] but I think we need to be a little
more clean about the implementation. This extracts run_kselftest.sh from
the Makefile so it can actually be changed without embeds, etc. Instead,
generate the test list into a text file. Everything gets much simpler.
:)
And in patch 2, I add back Hangbin Liu's new options (with some extra
added) with knowledge of "collections" (i.e. Makefile TARGETS) and
subtests. This should work really well with LAVA too, which needs to
manipulate the lists of tests being run.
Thoughts?
-Kees
[1] https://lore.kernel.org/lkml/20200914022227.437143-1-liuhangbin@gmail.com/
Kees Cook (3):
selftests: Extract run_kselftest.sh and generate stand-alone test list
selftests/run_kselftest.sh: Make each test individually selectable
doc: dev-tools: kselftest.rst: Update examples and paths
Documentation/dev-tools/kselftest.rst | 35 +++++----
tools/testing/selftests/Makefile | 26 ++-----
tools/testing/selftests/lib.mk | 5 +-
tools/testing/selftests/run_kselftest.sh | 93 ++++++++++++++++++++++++
4 files changed, 124 insertions(+), 35 deletions(-)
create mode 100755 tools/testing/selftests/run_kselftest.sh
--
2.25.1