Skip to content

Commit 9984522

Browse files
committed
Daniel Borkmann says: ==================== pull-request: bpf 2022-01-19 We've added 12 non-merge commits during the last 8 day(s) which contain a total of 12 files changed, 262 insertions(+), 64 deletions(-). The main changes are: 1) Various verifier fixes mainly around register offset handling when passed to helper functions, from Daniel Borkmann. 2) Fix XDP BPF link handling to assert program type, from Toke Høiland-Jørgensen. 3) Fix regression in mount parameter handling for BPF fs, from Yafang Shao. 4) Fix incorrect integer literal when marking scratched stack slots in verifier, from Christy Lee. * https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: bpf, selftests: Add ringbuf memory type confusion test bpf, selftests: Add various ringbuf tests with invalid offset bpf: Fix ringbuf memory type confusion when passing to helpers bpf: Fix out of bounds access for ringbuf helpers bpf: Generally fix helper register offset check bpf: Mark PTR_TO_FUNC register initially with zero offset bpf: Generalize check_ctx_reg for reuse with other types bpf: Fix incorrect integer literal used for marking scratched stack. bpf/selftests: Add check for updating XDP bpf_link with wrong program type bpf/selftests: convert xdp_link test to ASSERT_* macros xdp: check prog type before updating BPF link bpf: Fix mount source show for bpffs ==================== Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents 2836615 + 37c8d48 commit 9984522

File tree

12 files changed

+262
-64
lines changed

12 files changed

+262
-64
lines changed

include/linux/bpf.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,12 @@ enum bpf_type_flag {
316316
*/
317317
MEM_RDONLY = BIT(1 + BPF_BASE_TYPE_BITS),
318318

319-
__BPF_TYPE_LAST_FLAG = MEM_RDONLY,
319+
/* MEM was "allocated" from a different helper, and cannot be mixed
320+
* with regular non-MEM_ALLOC'ed MEM types.
321+
*/
322+
MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS),
323+
324+
__BPF_TYPE_LAST_FLAG = MEM_ALLOC,
320325
};
321326

322327
/* Max number of base types. */
@@ -400,7 +405,7 @@ enum bpf_return_type {
400405
RET_PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCKET,
401406
RET_PTR_TO_TCP_SOCK_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_TCP_SOCK,
402407
RET_PTR_TO_SOCK_COMMON_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCK_COMMON,
403-
RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_ALLOC_MEM,
408+
RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_ALLOC | RET_PTR_TO_ALLOC_MEM,
404409
RET_PTR_TO_BTF_ID_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_BTF_ID,
405410

406411
/* This must be the last entry. Its purpose is to ensure the enum is

include/linux/bpf_verifier.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,8 +519,8 @@ bpf_prog_offload_replace_insn(struct bpf_verifier_env *env, u32 off,
519519
void
520520
bpf_prog_offload_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt);
521521

522-
int check_ctx_reg(struct bpf_verifier_env *env,
523-
const struct bpf_reg_state *reg, int regno);
522+
int check_ptr_off_reg(struct bpf_verifier_env *env,
523+
const struct bpf_reg_state *reg, int regno);
524524
int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
525525
u32 regno, u32 mem_size);
526526

kernel/bpf/btf.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5686,7 +5686,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
56865686
i, btf_type_str(t));
56875687
return -EINVAL;
56885688
}
5689-
if (check_ctx_reg(env, reg, regno))
5689+
if (check_ptr_off_reg(env, reg, regno))
56905690
return -EINVAL;
56915691
} else if (is_kfunc && (reg->type == PTR_TO_BTF_ID || reg2btf_ids[reg->type])) {
56925692
const struct btf_type *reg_ref_t;

kernel/bpf/inode.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -648,12 +648,22 @@ static int bpf_parse_param(struct fs_context *fc, struct fs_parameter *param)
648648
int opt;
649649

650650
opt = fs_parse(fc, bpf_fs_parameters, param, &result);
651-
if (opt < 0)
651+
if (opt < 0) {
652652
/* We might like to report bad mount options here, but
653653
* traditionally we've ignored all mount options, so we'd
654654
* better continue to ignore non-existing options for bpf.
655655
*/
656-
return opt == -ENOPARAM ? 0 : opt;
656+
if (opt == -ENOPARAM) {
657+
opt = vfs_parse_fs_param_source(fc, param);
658+
if (opt != -ENOPARAM)
659+
return opt;
660+
661+
return 0;
662+
}
663+
664+
if (opt < 0)
665+
return opt;
666+
}
657667

658668
switch (opt) {
659669
case OPT_MODE:

kernel/bpf/verifier.c

Lines changed: 56 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,8 @@ static const char *reg_type_str(struct bpf_verifier_env *env,
570570

571571
if (type & MEM_RDONLY)
572572
strncpy(prefix, "rdonly_", 16);
573+
if (type & MEM_ALLOC)
574+
strncpy(prefix, "alloc_", 16);
573575

574576
snprintf(env->type_str_buf, TYPE_STR_BUF_LEN, "%s%s%s",
575577
prefix, str[base_type(type)], postfix);
@@ -616,7 +618,7 @@ static void mark_reg_scratched(struct bpf_verifier_env *env, u32 regno)
616618

617619
static void mark_stack_slot_scratched(struct bpf_verifier_env *env, u32 spi)
618620
{
619-
env->scratched_stack_slots |= 1UL << spi;
621+
env->scratched_stack_slots |= 1ULL << spi;
620622
}
621623

622624
static bool reg_scratched(const struct bpf_verifier_env *env, u32 regno)
@@ -637,14 +639,14 @@ static bool verifier_state_scratched(const struct bpf_verifier_env *env)
637639
static void mark_verifier_state_clean(struct bpf_verifier_env *env)
638640
{
639641
env->scratched_regs = 0U;
640-
env->scratched_stack_slots = 0UL;
642+
env->scratched_stack_slots = 0ULL;
641643
}
642644

643645
/* Used for printing the entire verifier state. */
644646
static void mark_verifier_state_scratched(struct bpf_verifier_env *env)
645647
{
646648
env->scratched_regs = ~0U;
647-
env->scratched_stack_slots = ~0UL;
649+
env->scratched_stack_slots = ~0ULL;
648650
}
649651

650652
/* The reg state of a pointer or a bounded scalar was saved when
@@ -3969,30 +3971,38 @@ static int get_callee_stack_depth(struct bpf_verifier_env *env,
39693971
}
39703972
#endif
39713973

3972-
int check_ctx_reg(struct bpf_verifier_env *env,
3973-
const struct bpf_reg_state *reg, int regno)
3974+
static int __check_ptr_off_reg(struct bpf_verifier_env *env,
3975+
const struct bpf_reg_state *reg, int regno,
3976+
bool fixed_off_ok)
39743977
{
3975-
/* Access to ctx or passing it to a helper is only allowed in
3976-
* its original, unmodified form.
3978+
/* Access to this pointer-typed register or passing it to a helper
3979+
* is only allowed in its original, unmodified form.
39773980
*/
39783981

3979-
if (reg->off) {
3980-
verbose(env, "dereference of modified ctx ptr R%d off=%d disallowed\n",
3981-
regno, reg->off);
3982+
if (!fixed_off_ok && reg->off) {
3983+
verbose(env, "dereference of modified %s ptr R%d off=%d disallowed\n",
3984+
reg_type_str(env, reg->type), regno, reg->off);
39823985
return -EACCES;
39833986
}
39843987

39853988
if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
39863989
char tn_buf[48];
39873990

39883991
tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
3989-
verbose(env, "variable ctx access var_off=%s disallowed\n", tn_buf);
3992+
verbose(env, "variable %s access var_off=%s disallowed\n",
3993+
reg_type_str(env, reg->type), tn_buf);
39903994
return -EACCES;
39913995
}
39923996

39933997
return 0;
39943998
}
39953999

4000+
int check_ptr_off_reg(struct bpf_verifier_env *env,
4001+
const struct bpf_reg_state *reg, int regno)
4002+
{
4003+
return __check_ptr_off_reg(env, reg, regno, false);
4004+
}
4005+
39964006
static int __check_buffer_access(struct bpf_verifier_env *env,
39974007
const char *buf_info,
39984008
const struct bpf_reg_state *reg,
@@ -4437,7 +4447,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
44374447
return -EACCES;
44384448
}
44394449

4440-
err = check_ctx_reg(env, reg, regno);
4450+
err = check_ptr_off_reg(env, reg, regno);
44414451
if (err < 0)
44424452
return err;
44434453

@@ -5127,6 +5137,7 @@ static const struct bpf_reg_types mem_types = {
51275137
PTR_TO_MAP_KEY,
51285138
PTR_TO_MAP_VALUE,
51295139
PTR_TO_MEM,
5140+
PTR_TO_MEM | MEM_ALLOC,
51305141
PTR_TO_BUF,
51315142
},
51325143
};
@@ -5144,7 +5155,7 @@ static const struct bpf_reg_types int_ptr_types = {
51445155
static const struct bpf_reg_types fullsock_types = { .types = { PTR_TO_SOCKET } };
51455156
static const struct bpf_reg_types scalar_types = { .types = { SCALAR_VALUE } };
51465157
static const struct bpf_reg_types context_types = { .types = { PTR_TO_CTX } };
5147-
static const struct bpf_reg_types alloc_mem_types = { .types = { PTR_TO_MEM } };
5158+
static const struct bpf_reg_types alloc_mem_types = { .types = { PTR_TO_MEM | MEM_ALLOC } };
51485159
static const struct bpf_reg_types const_map_ptr_types = { .types = { CONST_PTR_TO_MAP } };
51495160
static const struct bpf_reg_types btf_ptr_types = { .types = { PTR_TO_BTF_ID } };
51505161
static const struct bpf_reg_types spin_lock_types = { .types = { PTR_TO_MAP_VALUE } };
@@ -5244,12 +5255,6 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
52445255
kernel_type_name(btf_vmlinux, *arg_btf_id));
52455256
return -EACCES;
52465257
}
5247-
5248-
if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
5249-
verbose(env, "R%d is a pointer to in-kernel struct with non-zero offset\n",
5250-
regno);
5251-
return -EACCES;
5252-
}
52535258
}
52545259

52555260
return 0;
@@ -5304,10 +5309,33 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
53045309
if (err)
53055310
return err;
53065311

5307-
if (type == PTR_TO_CTX) {
5308-
err = check_ctx_reg(env, reg, regno);
5312+
switch ((u32)type) {
5313+
case SCALAR_VALUE:
5314+
/* Pointer types where reg offset is explicitly allowed: */
5315+
case PTR_TO_PACKET:
5316+
case PTR_TO_PACKET_META:
5317+
case PTR_TO_MAP_KEY:
5318+
case PTR_TO_MAP_VALUE:
5319+
case PTR_TO_MEM:
5320+
case PTR_TO_MEM | MEM_RDONLY:
5321+
case PTR_TO_MEM | MEM_ALLOC:
5322+
case PTR_TO_BUF:
5323+
case PTR_TO_BUF | MEM_RDONLY:
5324+
case PTR_TO_STACK:
5325+
/* Some of the argument types nevertheless require a
5326+
* zero register offset.
5327+
*/
5328+
if (arg_type == ARG_PTR_TO_ALLOC_MEM)
5329+
goto force_off_check;
5330+
break;
5331+
/* All the rest must be rejected: */
5332+
default:
5333+
force_off_check:
5334+
err = __check_ptr_off_reg(env, reg, regno,
5335+
type == PTR_TO_BTF_ID);
53095336
if (err < 0)
53105337
return err;
5338+
break;
53115339
}
53125340

53135341
skip_type_check:
@@ -9507,9 +9535,13 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
95079535
return 0;
95089536
}
95099537

9510-
if (insn->src_reg == BPF_PSEUDO_BTF_ID) {
9511-
mark_reg_known_zero(env, regs, insn->dst_reg);
9538+
/* All special src_reg cases are listed below. From this point onwards
9539+
* we either succeed and assign a corresponding dst_reg->type after
9540+
* zeroing the offset, or fail and reject the program.
9541+
*/
9542+
mark_reg_known_zero(env, regs, insn->dst_reg);
95129543

9544+
if (insn->src_reg == BPF_PSEUDO_BTF_ID) {
95139545
dst_reg->type = aux->btf_var.reg_type;
95149546
switch (base_type(dst_reg->type)) {
95159547
case PTR_TO_MEM:
@@ -9547,7 +9579,6 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
95479579
}
95489580

95499581
map = env->used_maps[aux->map_index];
9550-
mark_reg_known_zero(env, regs, insn->dst_reg);
95519582
dst_reg->map_ptr = map;
95529583

95539584
if (insn->src_reg == BPF_PSEUDO_MAP_VALUE ||
@@ -9651,7 +9682,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
96519682
return err;
96529683
}
96539684

9654-
err = check_ctx_reg(env, &regs[ctx_reg], ctx_reg);
9685+
err = check_ptr_off_reg(env, &regs[ctx_reg], ctx_reg);
96559686
if (err < 0)
96569687
return err;
96579688

net/core/dev.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8981,6 +8981,12 @@ static int bpf_xdp_link_update(struct bpf_link *link, struct bpf_prog *new_prog,
89818981
goto out_unlock;
89828982
}
89838983
old_prog = link->prog;
8984+
if (old_prog->type != new_prog->type ||
8985+
old_prog->expected_attach_type != new_prog->expected_attach_type) {
8986+
err = -EINVAL;
8987+
goto out_unlock;
8988+
}
8989+
89848990
if (old_prog == new_prog) {
89858991
/* no-op, don't disturb drivers */
89868992
bpf_prog_put(new_prog);

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include "test_d_path.skel.h"
1212
#include "test_d_path_check_rdonly_mem.skel.h"
13+
#include "test_d_path_check_types.skel.h"
1314

1415
static int duration;
1516

@@ -167,11 +168,24 @@ static void test_d_path_check_rdonly_mem(void)
167168
test_d_path_check_rdonly_mem__destroy(skel);
168169
}
169170

171+
static void test_d_path_check_types(void)
172+
{
173+
struct test_d_path_check_types *skel;
174+
175+
skel = test_d_path_check_types__open_and_load();
176+
ASSERT_ERR_PTR(skel, "unexpected_load_passing_wrong_type");
177+
178+
test_d_path_check_types__destroy(skel);
179+
}
180+
170181
void test_d_path(void)
171182
{
172183
if (test__start_subtest("basic"))
173184
test_d_path_basic();
174185

175186
if (test__start_subtest("check_rdonly_mem"))
176187
test_d_path_check_rdonly_mem();
188+
189+
if (test__start_subtest("check_alloc_mem"))
190+
test_d_path_check_types();
177191
}

0 commit comments

Comments
 (0)