mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-11-01 00:48:50 +00:00
7fb5eefd76
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@fb.com> Signed-off-by: Yonghong Song <yhs@fb.com> Signed-off-by: Alexei Starovoitov <ast@kernel.org> Acked-by: Andrii Nakryiko <andriin@fb.com> Link: https://lore.kernel.org/bpf/20200909171542.3673449-1-yhs@fb.com
72 lines
1.7 KiB
C
72 lines
1.7 KiB
C
// SPDX-License-Identifier: GPL-2.0
|
|
// Copyright (c) 2019 Facebook
|
|
|
|
#include <stdint.h>
|
|
#include <string.h>
|
|
|
|
#include <linux/stddef.h>
|
|
#include <linux/bpf.h>
|
|
|
|
#include <bpf/bpf_helpers.h>
|
|
|
|
#ifndef ARRAY_SIZE
|
|
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
|
|
#endif
|
|
|
|
/* tcp_mem sysctl has only 3 ints, but this test is doing TCP_MEM_LOOPS */
|
|
#define TCP_MEM_LOOPS 20 /* because 30 doesn't fit into 512 bytes of stack */
|
|
#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)
|
|
{
|
|
unsigned char i;
|
|
char name[sizeof(tcp_mem_name)];
|
|
int ret;
|
|
|
|
memset(name, 0, sizeof(name));
|
|
ret = bpf_sysctl_get_name(ctx, name, sizeof(name), 0);
|
|
if (ret < 0 || ret != sizeof(tcp_mem_name) - 1)
|
|
return 0;
|
|
|
|
#pragma clang loop unroll(disable)
|
|
for (i = 0; i < sizeof(tcp_mem_name); ++i)
|
|
if (name[i] != tcp_mem_name[i])
|
|
return 0;
|
|
|
|
return 1;
|
|
}
|
|
|
|
|
|
SEC("cgroup/sysctl")
|
|
int sysctl_tcp_mem(struct bpf_sysctl *ctx)
|
|
{
|
|
unsigned long tcp_mem[TCP_MEM_LOOPS] = {};
|
|
char value[MAX_VALUE_STR_LEN];
|
|
unsigned char i, off = 0;
|
|
int ret;
|
|
|
|
if (ctx->write)
|
|
return 0;
|
|
|
|
if (!is_tcp_mem(ctx))
|
|
return 0;
|
|
|
|
ret = bpf_sysctl_get_current_value(ctx, value, MAX_VALUE_STR_LEN);
|
|
if (ret < 0 || ret >= MAX_VALUE_STR_LEN)
|
|
return 0;
|
|
|
|
#pragma clang loop unroll(disable)
|
|
for (i = 0; i < ARRAY_SIZE(tcp_mem); ++i) {
|
|
ret = bpf_strtoul(value + off, MAX_ULONG_STR_LEN, 0,
|
|
tcp_mem + i);
|
|
if (ret <= 0 || ret > MAX_ULONG_STR_LEN)
|
|
return 0;
|
|
off += ret & MAX_ULONG_STR_LEN;
|
|
}
|
|
|
|
return tcp_mem[0] < tcp_mem[1] && tcp_mem[1] < tcp_mem[2];
|
|
}
|
|
|
|
char _license[] SEC("license") = "GPL";
|