mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
bpf: Consolidate nullable argument validation
check_kfunc_args() rejects a NULL or possibly-NULL register passed to a non-nullable pointer argument up front. Helpers only do so inside check_reg_type() for PTR_TO_BTF_ID arguments, leaving other pointer kinds to fail indirectly through compatible_reg_types[]. Give both paths the same policy and factor it into check_func_arg_nullability(). Gate it on arg_type_is_scalar() rather than on the BTF shape of the kfunc parameter. This keeps a zero passed to a size argument from being interpreted as a NULL pointer. Taking nullability from the argument classification also avoids an is_kfunc_arg_nullable() lookup for every kfunc argument on every verification. Keep release-argument nullability separate from ownership checking. A possibly-NULL release argument must be rejected even if it is otherwise nullable, while the ownership check must still permit a literal NULL for bpf_kptr_xchg(). Use call-neutral structured diagnostics and retain the expected BTF type when it is available for a kfunc argument. Resolve that type lazily from the call metadata only when reporting an error. Helpers now report NULL-ness rather than a register-type mismatch for non-BTF-ID pointer arguments and literal NULL passed to a non-nullable pointer argument. Update the affected selftest expectations. Signed-off-by: Amery Hung <ameryhung@gmail.com> Link: https://lore.kernel.org/r/20260911220415.1396439-16-ameryhung@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
parent
2e418779a0
commit
750f377080
|
|
@ -8180,6 +8180,25 @@ static bool arg_type_is_dynptr(enum bpf_arg_type type)
|
|||
return base_type(type) == ARG_PTR_TO_DYNPTR;
|
||||
}
|
||||
|
||||
/*
|
||||
* An argument that only ever takes a scalar, so a zero register passed to it
|
||||
* is a value rather than a NULL pointer.
|
||||
*/
|
||||
static bool arg_type_is_scalar(enum bpf_arg_type type)
|
||||
{
|
||||
switch (base_type(type)) {
|
||||
case ARG_SCALAR:
|
||||
case ARG_CONST_SCALAR:
|
||||
case ARG_MEM_SIZE:
|
||||
case ARG_MEM_SIZE_OR_ZERO:
|
||||
case ARG_CONST_MEM_SIZE:
|
||||
case ARG_CONST_ALLOC_SIZE_OR_ZERO:
|
||||
return true;
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* A kfunc is named by a BTF ID, which can take the same numeric value as an
|
||||
* enum bpf_func_id. Only test meta->func_id against a BPF_FUNC_* once the call
|
||||
|
|
@ -8382,6 +8401,34 @@ __printf(6, 7) static void bpf_diag_call_arg_fmt(struct bpf_verifier_env *env, u
|
|||
bpf_diag_call_arg(env, insn_idx, argno, call_name, reason, suggestion);
|
||||
}
|
||||
|
||||
static int check_func_arg_nullability(struct bpf_verifier_env *env,
|
||||
struct bpf_reg_state *reg, argno_t argno,
|
||||
enum bpf_arg_type arg_type,
|
||||
struct bpf_call_arg_meta *meta, int insn_idx)
|
||||
{
|
||||
const char *expected_type = "pointer";
|
||||
|
||||
if (arg_type_is_scalar(arg_type) || type_may_be_null(arg_type) ||
|
||||
(!bpf_register_is_null(reg) && !type_may_be_null(reg->type)))
|
||||
return 0;
|
||||
|
||||
if (meta->btf) {
|
||||
u32 arg_btf_id;
|
||||
|
||||
arg_btf_id = btf_params(meta->func_proto)[arg_idx_from_argno(argno)].type;
|
||||
expected_type = bpf_diag_fmt(env, "value of type %s",
|
||||
bpf_diag_fmt_btf_type(env, meta->btf, arg_btf_id));
|
||||
}
|
||||
|
||||
verbose(env, "Possibly NULL pointer passed to trusted %s\n",
|
||||
reg_arg_name(env, argno));
|
||||
bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name,
|
||||
"Add a NULL check and make the call only on the non-NULL path.",
|
||||
"the pointer may be NULL, but this call requires a non-NULL %s",
|
||||
expected_type);
|
||||
return -EACCES;
|
||||
}
|
||||
|
||||
static const char *bpf_diag_expected_reg_types(struct bpf_verifier_env *env,
|
||||
const enum bpf_reg_type *types, int count)
|
||||
{
|
||||
|
|
@ -8494,17 +8541,6 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re
|
|||
bool strict_type_match = arg_type_is_release(arg_type) &&
|
||||
!is_helper_call(meta, BPF_FUNC_sk_release);
|
||||
|
||||
if (type_may_be_null(reg->type) &&
|
||||
(!type_may_be_null(arg_type) || arg_type_is_release(arg_type))) {
|
||||
verbose(env, "Possibly NULL pointer passed to helper %s\n",
|
||||
reg_arg_name(env, argno));
|
||||
bpf_diag_call_arg(
|
||||
env, env->insn_idx, argno, meta->func_name,
|
||||
"the pointer may be NULL, but this call requires a non-NULL pointer",
|
||||
"Add a NULL check and make the call only on the non-NULL path.");
|
||||
return -EACCES;
|
||||
}
|
||||
|
||||
if (!arg_btf_id) {
|
||||
if (!compatible->btf_id) {
|
||||
verifier_bug(env, "missing arg compatible BTF ID");
|
||||
|
|
@ -8892,6 +8928,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
|
|||
goto skip_type_check;
|
||||
}
|
||||
|
||||
err = check_func_arg_nullability(env, reg, argno, arg_type, meta, insn_idx);
|
||||
if (err)
|
||||
return err;
|
||||
|
||||
/* arg_btf_id and arg_size are in a union. */
|
||||
if (base_type(arg_type) == ARG_PTR_TO_BTF_ID ||
|
||||
base_type(arg_type) == ARG_PTR_TO_SPIN_LOCK)
|
||||
|
|
@ -8906,15 +8946,28 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
|
|||
return err;
|
||||
|
||||
skip_type_check:
|
||||
if (arg_type_is_release(arg_type) && !arg_type_is_dynptr(arg_type) &&
|
||||
!reg_is_referenced(env, reg) && !bpf_register_is_null(reg)) {
|
||||
verbose(env, "release helper %s expects referenced PTR_TO_BTF_ID passed to %s\n",
|
||||
meta->func_name, reg_arg_name(env, argno));
|
||||
bpf_diag_call_arg(
|
||||
env, insn_idx, argno, meta->func_name,
|
||||
"release helpers require a value that owns a live resource returned by a matching acquire helper",
|
||||
"Pass the resource-owning pointer returned by the matching acquire helper, and avoid calling the release helper after ownership has already been transferred or released.");
|
||||
return -EINVAL;
|
||||
if (arg_type_is_release(arg_type)) {
|
||||
if (type_may_be_null(reg->type)) {
|
||||
verbose(env, "Possibly NULL pointer passed to trusted %s\n",
|
||||
reg_arg_name(env, argno));
|
||||
bpf_diag_call_arg(
|
||||
env, insn_idx, argno, meta->func_name,
|
||||
"the pointer may be NULL, but this call requires a non-NULL pointer",
|
||||
"Add a NULL check and make the call only on the non-NULL path.");
|
||||
return -EACCES;
|
||||
}
|
||||
|
||||
if (!arg_type_is_dynptr(arg_type) &&
|
||||
!reg_is_referenced(env, reg) && !bpf_register_is_null(reg)) {
|
||||
verbose(env,
|
||||
"release helper %s expects referenced PTR_TO_BTF_ID passed to %s\n",
|
||||
meta->func_name, reg_arg_name(env, argno));
|
||||
bpf_diag_call_arg(
|
||||
env, insn_idx, argno, meta->func_name,
|
||||
"release helpers require a value that owns a live resource returned by a matching acquire helper",
|
||||
"Pass the resource-owning pointer returned by the matching acquire helper, and avoid calling the release helper after ownership has already been transferred or released.");
|
||||
return -EINVAL;
|
||||
}
|
||||
}
|
||||
|
||||
if (reg_is_referenced(env, reg))
|
||||
|
|
@ -13074,20 +13127,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
|
|||
ref_tname = btf_name_by_offset(btf, ref_t->name_off);
|
||||
}
|
||||
|
||||
if (btf_type_is_ptr(t) &&
|
||||
(bpf_register_is_null(reg) || type_may_be_null(reg->type)) &&
|
||||
!type_may_be_null(arg_type)) {
|
||||
const char *expected_type;
|
||||
|
||||
expected_type = bpf_diag_fmt_btf_type(env, btf, args[i].type);
|
||||
verbose(env, "Possibly NULL pointer passed to trusted %s\n",
|
||||
reg_arg_name(env, argno));
|
||||
bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name,
|
||||
"Add a NULL check and call the kfunc only on the non-NULL path.",
|
||||
"the pointer may be NULL, but this kfunc requires a non-NULL value of type %s",
|
||||
expected_type);
|
||||
return -EACCES;
|
||||
}
|
||||
ret = check_func_arg_nullability(env, reg, argno, arg_type, meta, insn_idx);
|
||||
if (ret < 0)
|
||||
return ret;
|
||||
|
||||
if (regno == meta->release_regno && !is_kfunc_arg_dynptr(meta->btf, &args[i]) &&
|
||||
!reg_is_referenced(env, reg) && !bpf_register_is_null(reg)) {
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ struct {
|
|||
const char *err_msg;
|
||||
} cb_refs_tests[] = {
|
||||
{ "underflow_prog", "release kfunc bpf_kfunc_call_test_release expects referenced PTR_TO_BTF_ID passed to R1" },
|
||||
{ "leak_prog", "Possibly NULL pointer passed to helper R2" },
|
||||
{ "leak_prog", "Possibly NULL pointer passed to trusted R2" },
|
||||
{ "nested_cb", "Unreleased reference id=4 alloc_insn=2" }, /* alloc_insn=2{4,5} */
|
||||
{ "non_cb_transfer_ref", "Unreleased reference id=4 alloc_insn=1" }, /* alloc_insn=1{1,2} */
|
||||
};
|
||||
|
|
|
|||
|
|
@ -183,7 +183,7 @@ int BPF_PROG(test_global_mask_no_null_check, struct task_struct *task, u64 clone
|
|||
}
|
||||
|
||||
SEC("tp_btf/task_newtask")
|
||||
__failure __msg("Possibly NULL pointer passed to helper R2")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R2")
|
||||
int BPF_PROG(test_global_mask_rcu_no_null_check, struct task_struct *task, u64 clone_flags)
|
||||
{
|
||||
struct bpf_cpumask *prev, *curr;
|
||||
|
|
|
|||
|
|
@ -135,7 +135,7 @@ int iter_ret_rcu_test_protected(const void *ctx)
|
|||
}
|
||||
|
||||
SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
|
||||
__failure __msg("R1 type=rcu_ptr_or_null_ expected=")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
int iter_ret_rcu_test_type(const void *ctx)
|
||||
{
|
||||
struct task_struct *p;
|
||||
|
|
@ -158,7 +158,7 @@ int iter_ret_rcu_test_protected_nostruct(const void *ctx)
|
|||
}
|
||||
|
||||
SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
|
||||
__failure __msg("R1 type=rdonly_rcu_mem_or_null expected=")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
int iter_ret_rcu_test_type_nostruct(const void *ctx)
|
||||
{
|
||||
void *p;
|
||||
|
|
|
|||
|
|
@ -149,7 +149,7 @@ int reject_bad_type_match(struct __sk_buff *ctx)
|
|||
}
|
||||
|
||||
SEC("?tc")
|
||||
__failure __msg("R1 type=untrusted_ptr_or_null_ expected=percpu_ptr_")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
int marked_as_untrusted_or_null(struct __sk_buff *ctx)
|
||||
{
|
||||
struct map_value *v;
|
||||
|
|
@ -217,7 +217,7 @@ int reject_kptr_xchg_on_unref(struct __sk_buff *ctx)
|
|||
}
|
||||
|
||||
SEC("?tc")
|
||||
__failure __msg("R1 type=rcu_ptr_or_null_ expected=percpu_ptr_")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
int mark_ref_as_untrusted_or_null(struct __sk_buff *ctx)
|
||||
{
|
||||
struct map_value *v;
|
||||
|
|
@ -364,7 +364,7 @@ int kptr_xchg_ref_state(struct __sk_buff *ctx)
|
|||
}
|
||||
|
||||
SEC("?tc")
|
||||
__failure __msg("Possibly NULL pointer passed to helper R2")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R2")
|
||||
int kptr_xchg_possibly_null(struct __sk_buff *ctx)
|
||||
{
|
||||
struct prog_test_ref_kfunc *p;
|
||||
|
|
|
|||
|
|
@ -208,7 +208,7 @@ __naked void null_check_7_ctx_bind(void)
|
|||
|
||||
SEC("cgroup/post_bind4")
|
||||
__description("pass ctx or null check, 8: null (bind)")
|
||||
__failure __msg("R1 type=scalar expected=ctx")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
__naked void null_check_8_null_bind(void)
|
||||
{
|
||||
asm volatile (" \
|
||||
|
|
|
|||
|
|
@ -621,7 +621,7 @@ l0_%=: exit; \
|
|||
|
||||
SEC("tracepoint")
|
||||
__description("helper access to variable memory: size = 0 not allowed on NULL (!ARG_PTR_TO_MEM_OR_NULL)")
|
||||
__failure __msg("R1 type=scalar expected=fp")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
__naked void ptr_to_mem_or_null_8(void)
|
||||
{
|
||||
asm volatile (" \
|
||||
|
|
@ -637,7 +637,7 @@ __naked void ptr_to_mem_or_null_8(void)
|
|||
|
||||
SEC("tracepoint")
|
||||
__description("helper access to variable memory: size > 0 not allowed on NULL (!ARG_PTR_TO_MEM_OR_NULL)")
|
||||
__failure __msg("R1 type=scalar expected=fp")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
__naked void ptr_to_mem_or_null_9(void)
|
||||
{
|
||||
asm volatile (" \
|
||||
|
|
|
|||
|
|
@ -246,7 +246,7 @@ static __used __naked void read_first_param2(void)
|
|||
SEC("socket")
|
||||
__flag(BPF_F_TEST_STATE_FREQ)
|
||||
__failure
|
||||
__msg("R1 type=scalar expected=map_ptr")
|
||||
__msg("Possibly NULL pointer passed to trusted R1")
|
||||
__naked void caller_stack_pruning_callback(void)
|
||||
{
|
||||
asm volatile (
|
||||
|
|
|
|||
|
|
@ -154,8 +154,7 @@ l0_%=: r0 = 0; \
|
|||
|
||||
SEC("socket")
|
||||
__description("forgot null checking on the inner map pointer")
|
||||
__failure __msg("R1 type=map_ptr_or_null expected=map_ptr")
|
||||
__msg("map_ptr_or_null, but this argument accepts map_ptr")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
__failure_unpriv
|
||||
__naked void on_the_inner_map_pointer(void)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -58,7 +58,7 @@ int mapofmaps_value_as_helper_mem_buf(struct __sk_buff *skb)
|
|||
}
|
||||
|
||||
SEC("?tc")
|
||||
__failure __msg("type=map_ptr_or_null expected=fp")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
int mapofmaps_value_as_helper_fixed_mem(struct __sk_buff *skb)
|
||||
{
|
||||
char th[sizeof(struct tcphdr)] = {};
|
||||
|
|
|
|||
|
|
@ -344,7 +344,7 @@ __naked void potential_reference_to_system_key(void)
|
|||
|
||||
SEC("tc")
|
||||
__description("reference tracking: release reference without check")
|
||||
__failure __msg("type=sock_or_null expected=sock")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
__naked void tracking_release_reference_without_check(void)
|
||||
{
|
||||
asm volatile (
|
||||
|
|
@ -363,7 +363,7 @@ __naked void tracking_release_reference_without_check(void)
|
|||
|
||||
SEC("tc")
|
||||
__description("reference tracking: release reference to sock_common without check")
|
||||
__failure __msg("type=sock_common_or_null expected=sock")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
__naked void to_sock_common_without_check(void)
|
||||
{
|
||||
asm volatile (
|
||||
|
|
|
|||
|
|
@ -110,7 +110,7 @@ l0_%=: r0 = *(u32*)(r1 + %[bpf_sock_type]); \
|
|||
|
||||
SEC("cgroup/skb")
|
||||
__description("bpf_sk_fullsock(skb->sk): no !skb->sk check")
|
||||
__failure __msg("type=sock_common_or_null expected=sock_common")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
__failure_unpriv
|
||||
__naked void sk_no_skb_sk_check_1(void)
|
||||
{
|
||||
|
|
@ -466,7 +466,7 @@ l1_%=: r0 = *(u32*)(r0 + %[bpf_sock_rx_queue_mapping__end]);\
|
|||
|
||||
SEC("cgroup/skb")
|
||||
__description("bpf_tcp_sock(skb->sk): no !skb->sk check")
|
||||
__failure __msg("type=sock_common_or_null expected=sock_common")
|
||||
__failure __msg("Possibly NULL pointer passed to trusted R1")
|
||||
__failure_unpriv
|
||||
__naked void sk_no_skb_sk_check_2(void)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -311,7 +311,7 @@
|
|||
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
|
||||
.fixup_map_kptr = { 1 },
|
||||
.result = REJECT,
|
||||
.errstr = "R1 type=rcu_ptr_or_null_ expected=percpu_ptr_",
|
||||
.errstr = "Possibly NULL pointer passed to trusted R1",
|
||||
},
|
||||
{
|
||||
"map_kptr: ref: reject off != 0",
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user