mirror of
https://github.com/torvalds/linux.git
synced 2026-09-11 02:48:03 +02:00
Merge branch 'bpf-add-missing-precision-propagation-after-bpf_register_is_null-calls'
Eduard Zingerman says: ==================== bpf: add missing precision propagation after bpf_register_is_null calls Fix [1] uncovered a host of locations where the call to bpf_register_is_null() is not followed by a call to bpf_mark_chain_precision(). check_map_kptr_access() is omitted as it is handled [2] by another series. [1] https://lore.kernel.org/bpf/20260904083325.2083493-7-eddyz87@gmail.com/ [2] https://lore.kernel.org/bpf/20260904104203.345917-6-memxor@gmail.com/ --- ==================== Link: https://patch.msgid.link/20260904-register-is-null-precise-fixes-v1-0-0f5a360ff15d@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
commit
b75a000f2a
|
|
@ -4246,6 +4246,15 @@ static int mark_stack_arg_precision(struct bpf_verifier_env *env, int arg_idx)
|
|||
return mark_chain_precision_batch(env, env->cur_state);
|
||||
}
|
||||
|
||||
static int mark_arg_precision(struct bpf_verifier_env *env, argno_t argno)
|
||||
{
|
||||
int regno = reg_from_argno(argno);
|
||||
|
||||
if (regno >= 0)
|
||||
return mark_chain_precision(env, regno);
|
||||
return mark_stack_arg_precision(env, arg_idx_from_argno(argno));
|
||||
}
|
||||
|
||||
static int check_outgoing_stack_args(struct bpf_verifier_env *env, struct bpf_func_state *caller,
|
||||
int nargs, const char *callee_name, const struct btf *btf,
|
||||
const struct btf_param *args)
|
||||
|
|
@ -7151,14 +7160,8 @@ static int check_mem_size_reg(struct bpf_verifier_env *env,
|
|||
if (err && failure)
|
||||
*failure = BPF_MEM_SIZE_FAIL_MEMORY;
|
||||
|
||||
if (!err) {
|
||||
int regno = reg_from_argno(size_argno);
|
||||
|
||||
if (regno >= 0)
|
||||
err = mark_chain_precision(env, regno);
|
||||
else
|
||||
err = mark_stack_arg_precision(env, arg_idx_from_argno(size_argno));
|
||||
}
|
||||
if (!err)
|
||||
err = mark_arg_precision(env, size_argno);
|
||||
|
||||
return err;
|
||||
|
||||
|
|
@ -7175,7 +7178,7 @@ static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg
|
|||
int size, err = 0;
|
||||
|
||||
if (bpf_register_is_null(reg))
|
||||
return 0;
|
||||
return mark_arg_precision(env, argno);
|
||||
if (known_memory)
|
||||
*known_memory = true;
|
||||
|
||||
|
|
@ -8759,11 +8762,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
|
|||
return err;
|
||||
}
|
||||
|
||||
if (bpf_register_is_null(reg) && type_may_be_null(arg_type))
|
||||
if (bpf_register_is_null(reg) && type_may_be_null(arg_type)) {
|
||||
/* A NULL register has a SCALAR_VALUE type, so skip
|
||||
* type checking.
|
||||
*/
|
||||
err = mark_chain_precision(env, regno);
|
||||
if (err)
|
||||
return err;
|
||||
goto skip_type_check;
|
||||
}
|
||||
|
||||
/* arg_btf_id and arg_size are in a union. */
|
||||
if (base_type(arg_type) == ARG_PTR_TO_BTF_ID ||
|
||||
|
|
@ -9761,8 +9768,12 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
|
|||
struct bpf_call_arg_meta meta;
|
||||
int err;
|
||||
|
||||
if (bpf_register_is_null(reg) && type_may_be_null(arg->arg_type))
|
||||
if (bpf_register_is_null(reg) && type_may_be_null(arg->arg_type)) {
|
||||
err = mark_arg_precision(env, argno);
|
||||
if (err)
|
||||
return err;
|
||||
continue;
|
||||
}
|
||||
|
||||
memset(&meta, 0, sizeof(meta)); /* leave func_id as zero */
|
||||
err = check_reg_type(env, reg, argno, arg->arg_type, &arg->btf_id, &meta,
|
||||
|
|
@ -10685,33 +10696,45 @@ static struct bpf_insn_aux_data *cur_aux(const struct bpf_verifier_env *env)
|
|||
return &env->insn_aux_data[env->insn_idx];
|
||||
}
|
||||
|
||||
static bool loop_flag_is_zero(struct bpf_verifier_env *env)
|
||||
/* Returns 1 if R4 is a known zero, 0 if it is not, a negative errno on error. */
|
||||
static int loop_flag_is_zero(struct bpf_verifier_env *env)
|
||||
{
|
||||
struct bpf_reg_state *reg = reg_state(env, BPF_REG_4);
|
||||
bool reg_is_null = bpf_register_is_null(reg);
|
||||
int err;
|
||||
|
||||
if (reg_is_null)
|
||||
mark_chain_precision(env, BPF_REG_4);
|
||||
if (!bpf_register_is_null(reg))
|
||||
return 0;
|
||||
|
||||
return reg_is_null;
|
||||
err = mark_chain_precision(env, BPF_REG_4);
|
||||
if (err)
|
||||
return err;
|
||||
return 1;
|
||||
}
|
||||
|
||||
static void update_loop_inline_state(struct bpf_verifier_env *env, u32 subprogno)
|
||||
static int update_loop_inline_state(struct bpf_verifier_env *env, u32 subprogno)
|
||||
{
|
||||
struct bpf_loop_inline_state *state = &cur_aux(env)->loop_inline_state;
|
||||
int flag_is_zero;
|
||||
|
||||
if (!state->initialized) {
|
||||
flag_is_zero = loop_flag_is_zero(env);
|
||||
if (flag_is_zero < 0)
|
||||
return flag_is_zero;
|
||||
state->initialized = 1;
|
||||
state->fit_for_inline = loop_flag_is_zero(env);
|
||||
state->fit_for_inline = flag_is_zero;
|
||||
state->callback_subprogno = subprogno;
|
||||
return;
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (!state->fit_for_inline)
|
||||
return;
|
||||
return 0;
|
||||
|
||||
state->fit_for_inline = (loop_flag_is_zero(env) &&
|
||||
flag_is_zero = loop_flag_is_zero(env);
|
||||
if (flag_is_zero < 0)
|
||||
return flag_is_zero;
|
||||
state->fit_for_inline = (flag_is_zero &&
|
||||
state->callback_subprogno == subprogno);
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Returns whether or not the given map can potentially elide
|
||||
|
|
@ -10923,6 +10946,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
|
|||
verbose(env, "get_local_storage() doesn't support non-zero flags\n");
|
||||
return -EINVAL;
|
||||
}
|
||||
err = mark_chain_precision(env, BPF_REG_2);
|
||||
if (err)
|
||||
return err;
|
||||
break;
|
||||
case BPF_FUNC_for_each_map_elem:
|
||||
err = push_callback_call(env, insn, insn_idx, meta.subprogno,
|
||||
|
|
@ -10940,7 +10966,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
|
|||
err = check_bpf_snprintf_call(env, regs);
|
||||
break;
|
||||
case BPF_FUNC_loop:
|
||||
update_loop_inline_state(env, meta.subprogno);
|
||||
err = update_loop_inline_state(env, meta.subprogno);
|
||||
if (err)
|
||||
return err;
|
||||
/* Verifier relies on R1 value to determine if bpf_loop() iteration
|
||||
* is finished, thus mark it precise.
|
||||
*/
|
||||
|
|
@ -12717,8 +12745,12 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
|
|||
if (reg_is_referenced(env, reg))
|
||||
update_ref_obj(&meta->ref_obj, reg);
|
||||
|
||||
if (bpf_register_is_null(reg) && type_may_be_null(kf_arg_type))
|
||||
if (bpf_register_is_null(reg) && type_may_be_null(kf_arg_type)) {
|
||||
ret = mark_arg_precision(env, argno);
|
||||
if (ret)
|
||||
return ret;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (is_kfunc_arg_map(btf, &args[i])) {
|
||||
ref_id = *reg2btf_ids[CONST_PTR_TO_MAP];
|
||||
|
|
|
|||
|
|
@ -305,4 +305,33 @@ __naked void cpu_cgroup_storage_access_6(void)
|
|||
: __clobber_all);
|
||||
}
|
||||
|
||||
/*
|
||||
* Verification takes two paths: with r2 being scalar zero on path (1)
|
||||
* and with r2 being some other scalar on path (2).
|
||||
* Check that the verifier does not use checkpoints created
|
||||
* on path (1) to prune path (2).
|
||||
*/
|
||||
SEC("cgroup/skb")
|
||||
__failure
|
||||
__flag(BPF_F_TEST_STATE_FREQ)
|
||||
__msg("get_local_storage() doesn't support non-zero flags")
|
||||
__naked void non_zero_flags_on_a_pruned_path(void)
|
||||
{
|
||||
asm volatile (" \
|
||||
call %[bpf_get_prandom_u32]; \
|
||||
/* r2 is 0 on the path explored first, 1 on the other */\
|
||||
r2 = 1; \
|
||||
if r0 == 0 goto 1f; \
|
||||
r2 = 0; \
|
||||
1: r1 = %[cgroup_storage] ll; \
|
||||
call %[bpf_get_local_storage]; \
|
||||
r0 = 0; \
|
||||
exit; \
|
||||
" :
|
||||
: __imm(bpf_get_prandom_u32),
|
||||
__imm(bpf_get_local_storage),
|
||||
__imm_addr(cgroup_storage)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
char _license[] SEC("license") = "GPL";
|
||||
|
|
|
|||
|
|
@ -56,6 +56,30 @@ int trusted_task_arg_nullable(void *ctx)
|
|||
return res;
|
||||
}
|
||||
|
||||
/*
|
||||
* Check that the verifier does not use checkpoints created
|
||||
* on path with r1 == 0 to prune path with r1 != 0.
|
||||
*/
|
||||
SEC("?tp_btf/task_newtask")
|
||||
__failure
|
||||
__flag(BPF_F_TEST_STATE_FREQ)
|
||||
__msg("R1 type=scalar expected=ptr_, trusted_ptr_, rcu_ptr_")
|
||||
__naked int null_btf_id_arg_global_subprog(void)
|
||||
{
|
||||
asm volatile (
|
||||
"call %[bpf_get_prandom_u32];"
|
||||
"r1 = 42;"
|
||||
"if r0 > 42 goto 1f;"
|
||||
"r1 = 0;"
|
||||
"1:"
|
||||
"call subprog_trusted_task_nullable;"
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_get_prandom_u32)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
__weak int subprog_trusted_task_nonnull(struct task_struct *task __arg_trusted)
|
||||
{
|
||||
return task->pid + task->tgid;
|
||||
|
|
|
|||
|
|
@ -2,8 +2,10 @@
|
|||
/* Copyright (C) 2023 SUSE LLC */
|
||||
#include <linux/bpf.h>
|
||||
#include <bpf/bpf_helpers.h>
|
||||
#include <stdbool.h>
|
||||
#include "../../../include/linux/filter.h"
|
||||
#include "bpf_misc.h"
|
||||
#include "bpf_kfuncs.h"
|
||||
|
||||
struct {
|
||||
__uint(type, BPF_MAP_TYPE_ARRAY);
|
||||
|
|
@ -642,4 +644,102 @@ __naked int bpf_atomic_cmpxchg_32bit_precision(void)
|
|||
: __clobber_all);
|
||||
}
|
||||
|
||||
/*
|
||||
* Verification takes two paths: with r1 being scalar zero on path (1)
|
||||
* and with r1 being some other scalar on path (2).
|
||||
* Check that the verifier does not use checkpoints created
|
||||
* on path (1) to prune path (2).
|
||||
*/
|
||||
SEC("?tc")
|
||||
__flag(BPF_F_TEST_STATE_FREQ)
|
||||
__failure __msg("R1 type=scalar expected=fp")
|
||||
__naked int null_mem_arg_zero_size(void)
|
||||
{
|
||||
asm volatile (
|
||||
"call %[bpf_get_prandom_u32];"
|
||||
"r1 = 42;"
|
||||
"if r0 > 42 goto 1f;"
|
||||
"r1 = 0;"
|
||||
"1:"
|
||||
"r2 = 0;"
|
||||
"r3 = 0;"
|
||||
"r4 = 0;"
|
||||
"r5 = 0;"
|
||||
/*
|
||||
* ARG_PTR_TO_MEM | PTR_MAYBE_NULL parameter can be NULL,
|
||||
* but can't be some other scalar value.
|
||||
*/
|
||||
"call %[bpf_csum_diff];"
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_get_prandom_u32),
|
||||
__imm(bpf_csum_diff)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
__weak int subprog_mem_arg(int *p)
|
||||
{
|
||||
if (p)
|
||||
return *p;
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* Verification takes two paths: with r1 being scalar zero on path (1)
|
||||
* and with r1 being some other scalar on path (2).
|
||||
* Check that the verifier does not use checkpoints created
|
||||
* on path (1) to prune path (2).
|
||||
*/
|
||||
SEC("?raw_tp")
|
||||
__flag(BPF_F_TEST_STATE_FREQ)
|
||||
__failure __msg("R1 type=scalar expected=fp")
|
||||
__naked int null_mem_arg_global_subprog(void)
|
||||
{
|
||||
asm volatile (
|
||||
"call %[bpf_get_prandom_u32];"
|
||||
"r1 = 42;"
|
||||
"if r0 > 42 goto 1f;"
|
||||
"r1 = 0;"
|
||||
"1:"
|
||||
"call subprog_mem_arg;"
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_get_prandom_u32)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
/* Same as above, check that path with r3 == 0 does not prune the path with r3 != 0 */
|
||||
SEC("?tc")
|
||||
__flag(BPF_F_TEST_STATE_FREQ)
|
||||
__failure __msg("R3 type=scalar expected=fp")
|
||||
int null_kfunc_arg_dynptr_slice(struct __sk_buff *skb)
|
||||
{
|
||||
struct bpf_dynptr ptr;
|
||||
|
||||
bpf_dynptr_from_skb(skb, 0, &ptr);
|
||||
asm volatile (
|
||||
"call %[bpf_get_prandom_u32];"
|
||||
"r3 = 42;"
|
||||
"if r0 > 42 goto 1f;"
|
||||
"r3 = 0;"
|
||||
"1:"
|
||||
"r1 = %[ptr];"
|
||||
"r2 = 0;"
|
||||
"r4 = 8;"
|
||||
"call %[bpf_dynptr_slice];"
|
||||
:
|
||||
: __imm_ptr(ptr),
|
||||
__imm(bpf_get_prandom_u32),
|
||||
__imm(bpf_dynptr_slice)
|
||||
: __clobber_common);
|
||||
return 0;
|
||||
}
|
||||
|
||||
void __kfunc_btf_root(void)
|
||||
{
|
||||
bpf_dynptr_slice(0, 0, 0, 0);
|
||||
}
|
||||
|
||||
char _license[] SEC("license") = "GPL";
|
||||
|
|
|
|||
|
|
@ -287,9 +287,9 @@ __msg("17: (b7) r0 = 0")
|
|||
__msg("18: (95) exit")
|
||||
__msg("returning from callee:")
|
||||
__msg("to caller at 9:")
|
||||
__msg("frame 0: propagating r1,r4")
|
||||
__msg("frame 0: propagating r1,r3,r4")
|
||||
__msg("mark_precise: frame0: last_idx 9 first_idx 9 subseq_idx -1")
|
||||
__msg("mark_precise: frame0: regs=r1,r4 stack= before 18: (95) exit")
|
||||
__msg("mark_precise: frame0: regs=r1,r3,r4 stack= before 18: (95) exit")
|
||||
__msg("from 18 to 9: safe")
|
||||
__naked int callback_result_precise(void)
|
||||
{
|
||||
|
|
@ -419,9 +419,9 @@ __msg("to caller at 9:")
|
|||
/* r1, r4 are always precise for bpf_loop(),
|
||||
* r6 was marked before backtracking to callback body.
|
||||
*/
|
||||
__msg("frame 0: propagating r1,r4,r6")
|
||||
__msg("frame 0: propagating r1,r3,r4,r6")
|
||||
__msg("mark_precise: frame0: last_idx 9 first_idx 9 subseq_idx -1")
|
||||
__msg("mark_precise: frame0: regs=r1,r4,r6 stack= before 16: (95) exit")
|
||||
__msg("mark_precise: frame0: regs=r1,r3,r4,r6 stack= before 16: (95) exit")
|
||||
__msg("mark_precise: frame1: regs= stack= before 15: (b7) r0 = 0")
|
||||
__msg("mark_precise: frame1: regs= stack= before 9: (85) call bpf_loop")
|
||||
__msg("mark_precise: frame0: parent state regs= stack=:")
|
||||
|
|
@ -575,9 +575,9 @@ __msg("to caller at 10:")
|
|||
/* r1, r4 are always precise for bpf_loop(),
|
||||
* fp-8 was marked before backtracking to callback body.
|
||||
*/
|
||||
__msg("frame 0: propagating r1,r4,fp-8")
|
||||
__msg("frame 0: propagating r1,r3,r4,fp-8")
|
||||
__msg("mark_precise: frame0: last_idx 10 first_idx 10 subseq_idx -1")
|
||||
__msg("mark_precise: frame0: regs=r1,r4 stack=-8 before 18: (95) exit")
|
||||
__msg("mark_precise: frame0: regs=r1,r3,r4 stack=-8 before 18: (95) exit")
|
||||
__msg("mark_precise: frame1: regs= stack= before 17: (b7) r0 = 0")
|
||||
__msg("mark_precise: frame1: regs= stack= before 10: (85) call bpf_loop#181")
|
||||
__msg("mark_precise: frame0: parent state regs= stack=:")
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user