Skip to content

Commit c64da10

Browse files
committed
Daniel Borkmann says: ==================== pull-request: bpf 2024-06-14 We've added 8 non-merge commits during the last 2 day(s) which contain a total of 9 files changed, 92 insertions(+), 11 deletions(-). The main changes are: 1) Silence a syzkaller splat under CONFIG_DEBUG_NET=y in pskb_pull_reason() triggered via __bpf_try_make_writable(), from Florian Westphal. 2) Fix removal of kfuncs during linking phase which then throws a kernel build warning via resolve_btfids about unresolved symbols, from Tony Ambardar. 3) Fix a UML x86_64 compilation failure from BPF as pcpu_hot symbol is not available on User Mode Linux, from Maciej Żenczykowski. 4) Fix a register corruption in reg_set_min_max triggering an invariant violation in BPF verifier, from Daniel Borkmann. * tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: bpf: Harden __bpf_kfunc tag against linker kfunc removal compiler_types.h: Define __retain for __attribute__((__retain__)) bpf: Avoid splat in pskb_pull_reason bpf: fix UML x86_64 compile failure selftests/bpf: Add test coverage for reg_set_min_max handling bpf: Reduce stack consumption in check_stack_write_fixed_off bpf: Fix reg_set_min_max corruption of fake_reg MAINTAINERS: mailmap: Update Stanislav's email address ==================== Link: https://lore.kernel.org/r/20240614203223.26500-1-daniel@iogearbox.net Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 1afe4a6 + 7bdcedd commit c64da10

File tree

9 files changed

+92
-11
lines changed

9 files changed

+92
-11
lines changed

.mailmap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,7 @@ Simon Kelley <simon@thekelleys.org.uk>
608608
Sricharan Ramabadhran <quic_srichara@quicinc.com> <sricharan@codeaurora.org>
609609
Srinivas Ramana <quic_sramana@quicinc.com> <sramana@codeaurora.org>
610610
Sriram R <quic_srirrama@quicinc.com> <srirrama@codeaurora.org>
611+
Stanislav Fomichev <sdf@fomichev.me> <sdf@google.com>
611612
Stefan Wahren <wahrenst@gmx.net> <stefan.wahren@i2se.com>
612613
Stéphane Witzmann <stephane.witzmann@ubpmes.univ-bpclermont.fr>
613614
Stephen Hemminger <stephen@networkplumber.org> <shemminger@linux-foundation.org>

MAINTAINERS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3980,7 +3980,7 @@ R: Song Liu <song@kernel.org>
39803980
R: Yonghong Song <yonghong.song@linux.dev>
39813981
R: John Fastabend <john.fastabend@gmail.com>
39823982
R: KP Singh <kpsingh@kernel.org>
3983-
R: Stanislav Fomichev <sdf@google.com>
3983+
R: Stanislav Fomichev <sdf@fomichev.me>
39843984
R: Hao Luo <haoluo@google.com>
39853985
R: Jiri Olsa <jolsa@kernel.org>
39863986
L: bpf@vger.kernel.org

include/linux/bpf_verifier.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,8 @@ struct bpf_verifier_env {
746746
/* Same as scratched_regs but for stack slots */
747747
u64 scratched_stack_slots;
748748
u64 prev_log_pos, prev_insn_print_pos;
749+
/* buffer used to temporary hold constants as scalar registers */
750+
struct bpf_reg_state fake_reg[2];
749751
/* buffer used to generate temporary string representations,
750752
* e.g., in reg_type_str() to generate reg_type string
751753
*/

include/linux/btf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
* as to avoid issues such as the compiler inlining or eliding either a static
8383
* kfunc, or a global kfunc in an LTO build.
8484
*/
85-
#define __bpf_kfunc __used noinline
85+
#define __bpf_kfunc __used __retain noinline
8686

8787
#define __bpf_kfunc_start_defs() \
8888
__diag_push(); \

include/linux/compiler_types.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,29 @@ static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
143143
# define __preserve_most
144144
#endif
145145

146+
/*
147+
* Annotating a function/variable with __retain tells the compiler to place
148+
* the object in its own section and set the flag SHF_GNU_RETAIN. This flag
149+
* instructs the linker to retain the object during garbage-cleanup or LTO
150+
* phases.
151+
*
152+
* Note that the __used macro is also used to prevent functions or data
153+
* being optimized out, but operates at the compiler/IR-level and may still
154+
* allow unintended removal of objects during linking.
155+
*
156+
* Optional: only supported since gcc >= 11, clang >= 13
157+
*
158+
* gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-retain-function-attribute
159+
* clang: https://clang.llvm.org/docs/AttributeReference.html#retain
160+
*/
161+
#if __has_attribute(__retain__) && \
162+
(defined(CONFIG_LD_DEAD_CODE_DATA_ELIMINATION) || \
163+
defined(CONFIG_LTO_CLANG))
164+
# define __retain __attribute__((__retain__))
165+
#else
166+
# define __retain
167+
#endif
168+
146169
/* Compiler specific macros. */
147170
#ifdef __clang__
148171
#include <linux/compiler-clang.h>

kernel/bpf/verifier.c

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4549,11 +4549,12 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env,
45494549
state->stack[spi].spilled_ptr.id = 0;
45504550
} else if (!reg && !(off % BPF_REG_SIZE) && is_bpf_st_mem(insn) &&
45514551
env->bpf_capable) {
4552-
struct bpf_reg_state fake_reg = {};
4552+
struct bpf_reg_state *tmp_reg = &env->fake_reg[0];
45534553

4554-
__mark_reg_known(&fake_reg, insn->imm);
4555-
fake_reg.type = SCALAR_VALUE;
4556-
save_register_state(env, state, spi, &fake_reg, size);
4554+
memset(tmp_reg, 0, sizeof(*tmp_reg));
4555+
__mark_reg_known(tmp_reg, insn->imm);
4556+
tmp_reg->type = SCALAR_VALUE;
4557+
save_register_state(env, state, spi, tmp_reg, size);
45574558
} else if (reg && is_spillable_regtype(reg->type)) {
45584559
/* register containing pointer is being spilled into stack */
45594560
if (size != BPF_REG_SIZE) {
@@ -15113,7 +15114,6 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
1511315114
struct bpf_reg_state *regs = this_branch->frame[this_branch->curframe]->regs;
1511415115
struct bpf_reg_state *dst_reg, *other_branch_regs, *src_reg = NULL;
1511515116
struct bpf_reg_state *eq_branch_regs;
15116-
struct bpf_reg_state fake_reg = {};
1511715117
u8 opcode = BPF_OP(insn->code);
1511815118
bool is_jmp32;
1511915119
int pred = -1;
@@ -15179,7 +15179,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
1517915179
verbose(env, "BPF_JMP/JMP32 uses reserved fields\n");
1518015180
return -EINVAL;
1518115181
}
15182-
src_reg = &fake_reg;
15182+
src_reg = &env->fake_reg[0];
15183+
memset(src_reg, 0, sizeof(*src_reg));
1518315184
src_reg->type = SCALAR_VALUE;
1518415185
__mark_reg_known(src_reg, insn->imm);
1518515186
}
@@ -15239,10 +15240,16 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
1523915240
&other_branch_regs[insn->src_reg],
1524015241
dst_reg, src_reg, opcode, is_jmp32);
1524115242
} else /* BPF_SRC(insn->code) == BPF_K */ {
15243+
/* reg_set_min_max() can mangle the fake_reg. Make a copy
15244+
* so that these are two different memory locations. The
15245+
* src_reg is not used beyond here in context of K.
15246+
*/
15247+
memcpy(&env->fake_reg[1], &env->fake_reg[0],
15248+
sizeof(env->fake_reg[0]));
1524215249
err = reg_set_min_max(env,
1524315250
&other_branch_regs[insn->dst_reg],
15244-
src_reg /* fake one */,
15245-
dst_reg, src_reg /* same fake one */,
15251+
&env->fake_reg[0],
15252+
dst_reg, &env->fake_reg[1],
1524615253
opcode, is_jmp32);
1524715254
}
1524815255
if (err)
@@ -20313,7 +20320,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
2031320320
goto next_insn;
2031420321
}
2031520322

20316-
#ifdef CONFIG_X86_64
20323+
#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
2031720324
/* Implement bpf_get_smp_processor_id() inline. */
2031820325
if (insn->imm == BPF_FUNC_get_smp_processor_id &&
2031920326
prog->jit_requested && bpf_jit_supports_percpu_insn()) {

net/core/filter.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,6 +1665,11 @@ static DEFINE_PER_CPU(struct bpf_scratchpad, bpf_sp);
16651665
static inline int __bpf_try_make_writable(struct sk_buff *skb,
16661666
unsigned int write_len)
16671667
{
1668+
#ifdef CONFIG_DEBUG_NET
1669+
/* Avoid a splat in pskb_may_pull_reason() */
1670+
if (write_len > INT_MAX)
1671+
return -EINVAL;
1672+
#endif
16681673
return skb_ensure_writable(skb, write_len);
16691674
}
16701675

tools/testing/selftests/bpf/prog_tests/verifier.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
#include "verifier_movsx.skel.h"
5454
#include "verifier_netfilter_ctx.skel.h"
5555
#include "verifier_netfilter_retcode.skel.h"
56+
#include "verifier_or_jmp32_k.skel.h"
5657
#include "verifier_precision.skel.h"
5758
#include "verifier_prevent_map_lookup.skel.h"
5859
#include "verifier_raw_stack.skel.h"
@@ -170,6 +171,7 @@ void test_verifier_meta_access(void) { RUN(verifier_meta_access); }
170171
void test_verifier_movsx(void) { RUN(verifier_movsx); }
171172
void test_verifier_netfilter_ctx(void) { RUN(verifier_netfilter_ctx); }
172173
void test_verifier_netfilter_retcode(void) { RUN(verifier_netfilter_retcode); }
174+
void test_verifier_or_jmp32_k(void) { RUN(verifier_or_jmp32_k); }
173175
void test_verifier_precision(void) { RUN(verifier_precision); }
174176
void test_verifier_prevent_map_lookup(void) { RUN(verifier_prevent_map_lookup); }
175177
void test_verifier_raw_stack(void) { RUN(verifier_raw_stack); }
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
3+
#include <linux/bpf.h>
4+
#include <bpf/bpf_helpers.h>
5+
#include "bpf_misc.h"
6+
7+
SEC("socket")
8+
__description("or_jmp32_k: bit ops + branch on unknown value")
9+
__failure
10+
__msg("R0 invalid mem access 'scalar'")
11+
__naked void or_jmp32_k(void)
12+
{
13+
asm volatile (" \
14+
r0 = 0xffffffff; \
15+
r0 /= 1; \
16+
r1 = 0; \
17+
w1 = -1; \
18+
w1 >>= 1; \
19+
w0 &= w1; \
20+
w0 |= 2; \
21+
if w0 != 0x7ffffffd goto l1; \
22+
r0 = 1; \
23+
exit; \
24+
l3: \
25+
r0 = 5; \
26+
*(u64*)(r0 - 8) = r0; \
27+
exit; \
28+
l2: \
29+
w0 -= 0xe; \
30+
if w0 == 1 goto l3; \
31+
r0 = 4; \
32+
exit; \
33+
l1: \
34+
w0 -= 0x7ffffff0; \
35+
if w0 s>= 0xe goto l2; \
36+
r0 = 3; \
37+
exit; \
38+
" ::: __clobber_all);
39+
}
40+
41+
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)