Helper, kfunc, and global subprogram argument paths supply callee identity and state differently. check_reg_type() receives the call name separately from bpf_call_arg_meta, while process_dynptr_func() receives the call name, referenced-object state, and dynptr state as separate arguments. Populate meta->func_name for helper and global subprogram calls, keep one metadata object for the entire global subprogram argument check, and have both shared checking functions obtain their name and state from the metadata. This gives dynptr validation one interface for every call kind in preparation for unifying their argument-checking paths. No functional change. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 67 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 9e79750e2480..a45e0478f23c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -7663,11 +7663,11 @@ __printf(6, 7) static void bpf_diag_call_arg_fmt(struct bpf_verifier_env *env, u /* * Validate dynptr arguments for helper, kfunc and subprog. * - * @dynptr is both input and output. It is populated when the argument is - * tagged with MEM_UNINIT (i.e., the dynptr argument that will be constructed) - * and consumed when the argument is expecting to be an initialized dynptr. - * @parent_id is used to track the referenced parent object (e.g., file or skb in - * qdisc program) when constructing a dynptr. + * @meta carries the dynptr and referenced-object state. The dynptr is populated + * when the argument is tagged with MEM_UNINIT (i.e., the dynptr argument that + * will be constructed) and consumed when the argument is expected to be an + * initialized dynptr. The reference tracks the parent object (e.g., file or skb + * in qdisc program) when constructing a dynptr. * * There are two register types representing a bpf_dynptr, one is PTR_TO_STACK * which points to a stack slot, and the other is CONST_PTR_TO_DYNPTR. @@ -7684,9 +7684,8 @@ __printf(6, 7) static void bpf_diag_call_arg_fmt(struct bpf_verifier_env *env, u * and checked dynamically during runtime. */ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_state *reg, - argno_t argno, int insn_idx, const char *call_name, - enum bpf_arg_type arg_type, - struct ref_obj_desc *ref_obj, struct bpf_dynptr_desc *dynptr) + argno_t argno, int insn_idx, enum bpf_arg_type arg_type, + struct bpf_call_arg_meta *meta) { int spi, err = 0; @@ -7695,7 +7694,7 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat "%s expected pointer to stack or const struct bpf_dynptr\n", reg_arg_name(env, argno)); bpf_diag_call_arg_fmt( - env, insn_idx, argno, call_name, + env, insn_idx, argno, meta->func_name, "Pass the address of a stack dynptr object, or use a const dynptr pointer returned by the verifier-supported path.", "a dynptr argument must be a pointer to a dynptr stack slot or a verifier-provided const struct bpf_dynptr, but %s is %s", reg_arg_name(env, argno), bpf_diag_reg_type_plain(env, reg->type)); @@ -7736,7 +7735,8 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat return err; } - err = mark_stack_slots_dynptr(env, reg, arg_type, insn_idx, ref_obj, dynptr); + err = mark_stack_slots_dynptr(env, reg, arg_type, insn_idx, + &meta->ref_obj, &meta->dynptr); } else /* OBJ_RELEASE and None case from above */ { /* For the reg->type == PTR_TO_STACK case, bpf_dynptr is never const */ if (reg->type == CONST_PTR_TO_DYNPTR && (arg_type & OBJ_RELEASE)) { @@ -7766,7 +7766,7 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat verbose(env, "Expected a dynptr of type %s as %s\n", dynptr_type_str(expected_type), reg_arg_name(env, argno)); bpf_diag_call_arg_fmt( - env, insn_idx, argno, call_name, + env, insn_idx, argno, meta->func_name, "Use a dynptr constructor that matches this operation, or call an operation that accepts the dynptr's current type.", "the dynptr is initialized with backing object type %s, but this operation expects dynptr type %s", dynptr_type_str(actual_type), dynptr_type_str(expected_type)); @@ -7785,11 +7785,9 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat reg = &state->stack[spi].spilled_ptr; } - if (dynptr) { - dynptr->type = reg->dynptr.type; - dynptr->id = reg->id; - dynptr->parent_id = reg->parent_id; - } + meta->dynptr.type = reg->dynptr.type; + meta->dynptr.id = reg->id; + meta->dynptr.parent_id = reg->parent_id; } return err; } @@ -8382,7 +8380,7 @@ static const char *bpf_diag_expected_reg_types(struct bpf_verifier_env *env, static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, enum bpf_arg_type arg_type, const u32 *arg_btf_id, - struct bpf_call_arg_meta *meta, const char *call_name) + struct bpf_call_arg_meta *meta) { enum bpf_reg_type expected, type = reg->type; const struct bpf_reg_types *compatible; @@ -8435,7 +8433,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re verbose(env, "%s\n", reg_type_str(env, compatible->types[j])); actual = bpf_diag_fmt(env, "%s", reg_type_str(env, reg->type)); accepted = bpf_diag_expected_reg_types(env, compatible->types, i); - bpf_diag_call_arg_fmt(env, env->insn_idx, argno, call_name, + bpf_diag_call_arg_fmt(env, env->insn_idx, argno, meta->func_name, "Pass a value with one of the accepted pointer or scalar types for this call.", "it has type %s, but this argument accepts %s", actual, accepted); @@ -8449,7 +8447,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re if (!(arg_type & MEM_RDONLY)) { verbose(env, "%s() may write into memory pointed by %s type=%s\n", - func_id_name(meta->func_id), + meta->func_name, reg_arg_name(env, argno), reg_type_str(env, reg->type)); return -EACCES; } @@ -8476,7 +8474,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re verbose(env, "Possibly NULL pointer passed to helper %s\n", reg_arg_name(env, argno)); bpf_diag_call_arg( - env, env->insn_idx, argno, call_name, + 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; @@ -8869,8 +8867,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, base_type(arg_type) == ARG_PTR_TO_SPIN_LOCK) arg_btf_id = fn->arg_btf_id[arg]; - err = check_reg_type(env, reg, argno, arg_type, arg_btf_id, meta, - func_id_name(meta->func_id)); + err = check_reg_type(env, reg, argno, arg_type, arg_btf_id, meta); if (err) return err; @@ -8882,9 +8879,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 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", - func_id_name(meta->func_id), reg_arg_name(env, argno)); + meta->func_name, reg_arg_name(env, argno)); bpf_diag_call_arg( - env, insn_idx, argno, func_id_name(meta->func_id), + 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; @@ -9015,8 +9012,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, true, meta, NULL); break; case ARG_PTR_TO_DYNPTR: - err = process_dynptr_func(env, reg, argno, insn_idx, func_id_name(meta->func_id), - arg_type, &meta->ref_obj, &meta->dynptr); + err = process_dynptr_func(env, reg, argno, insn_idx, arg_type, meta); if (err) return err; break; @@ -9763,12 +9759,16 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, struct bpf_subprog_info *sub = subprog_info(env, subprog); struct bpf_func_state *caller = cur_func(env); struct bpf_verifier_log *log = &env->log; - struct ref_obj_desc ref_obj = {}; const struct btf_param *args; const struct btf_type *func, *func_proto; + struct bpf_call_arg_meta meta; u32 i; int ret, err; + /* Leave btf and func_id zero: this is neither a helper nor a kfunc. */ + memset(&meta, 0, sizeof(meta)); + meta.func_name = bpf_subprog_name(env, subprog); + ret = btf_prepare_func_args(env, subprog); if (ret) { if (bpf_in_stack_arg_cnt(sub) > 0) { @@ -9852,12 +9852,10 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, return ret; ret = process_dynptr_func(env, reg, argno, env->insn_idx, - bpf_subprog_name(env, subprog), arg->arg_type, - &ref_obj, NULL); + arg->arg_type, &meta); if (ret) return ret; } else if (base_type(arg->arg_type) == ARG_PTR_TO_BTF_ID) { - struct bpf_call_arg_meta meta; int err; if (bpf_register_is_null(reg) && type_may_be_null(arg->arg_type)) { @@ -9867,9 +9865,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, 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, - bpf_subprog_name(env, subprog)); + err = check_reg_type(env, reg, argno, arg->arg_type, &arg->btf_id, &meta); err = err ?: check_func_arg_reg_off(env, reg, argno, arg->arg_type); if (err) return err; @@ -11008,6 +11004,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn env->insn_aux_data[insn_idx].non_sleepable = true; meta.func_id = func_id; + meta.func_name = func_id_name(func_id); meta.fn = fn; /* check args */ for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { @@ -13177,8 +13174,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me dynptr_arg_type |= (unsigned int)get_dynptr_type_flag(parent_type); } - ret = process_dynptr_func(env, reg, argno, insn_idx, func_name, - dynptr_arg_type, &meta->ref_obj, &meta->dynptr); + ret = process_dynptr_func(env, reg, argno, insn_idx, + dynptr_arg_type, meta); if (ret < 0) return ret; break; -- 2.52.0 check_func_arg() derives every argument from BPF_REG_1 + arg. That works for helpers, which have at most five arguments, but kfunc arguments beyond the fifth are passed on the stack. Switch to the argno_t addressing used by check_kfunc_args(): obtain argument state with get_func_arg_reg(), use argno for diagnostics, and call check_reg_arg() only when the argument is held in a register. This prepares check_func_arg() to be shared with kfunc verification. There is no functional change for helpers. reg_arg_name() renders their register arguments as R%d, so the updated diagnostics remain unchanged. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index a45e0478f23c..d5e885435bb6 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8815,10 +8815,12 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, int insn_idx) { const struct bpf_func_proto *fn = meta->fn; - u32 regno = BPF_REG_1 + arg; - struct bpf_reg_state *reg = reg_state(env, regno); + struct bpf_func_state *caller = cur_func(env); + struct bpf_reg_state *regs = cur_regs(env); + argno_t argno = argno_from_arg(arg + 1); + struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, arg); enum bpf_arg_type arg_type = fn->arg_type[arg]; - argno_t argno = argno_from_reg(regno); + int regno = reg_from_argno(argno); enum bpf_reg_type type = reg->type; u32 *arg_btf_id = NULL; u32 key_size; @@ -8827,14 +8829,16 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, if (arg_type == ARG_DONTCARE) return 0; - err = check_reg_arg(env, regno, SRC_OP); - if (err) - return err; + if (regno >= 0) { + err = check_reg_arg(env, regno, SRC_OP); + if (err) + return err; + } if (arg_type == ARG_ANYTHING) { - if (is_pointer_value(env, regno)) { - verbose(env, "R%d leaks addr into helper function\n", - regno); + if (__is_pointer_value(env->allow_ptr_leaks, reg)) { + verbose(env, "%s leaks addr into helper function\n", + reg_arg_name(env, argno)); return -EACCES; } return 0; @@ -8954,7 +8958,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, break; case ARG_PTR_TO_PERCPU_BTF_ID: if (!reg->btf_id) { - verbose(env, "Helper has invalid btf_id in R%d\n", regno); + verbose(env, "Helper has invalid btf_id in %s\n", reg_arg_name(env, argno)); return -EACCES; } meta->ret_btf = reg->btf; @@ -8991,7 +8995,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, * next is_mem_size argument below. */ if (arg_type & MEM_FIXED_SIZE) { - err = check_mem_reg(env, reg, argno_from_reg(regno), fn->arg_size[arg], + err = check_mem_reg(env, reg, argno, fn->arg_size[arg], arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ, meta, NULL); if (err) return err; @@ -9000,14 +9004,14 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, } break; case ARG_MEM_SIZE: - err = check_mem_size_reg(env, reg_state(env, regno - 1), reg, - argno_from_reg(regno - 1), argno, + err = check_mem_size_reg(env, get_func_arg_reg(caller, regs, arg - 1), reg, + argno_from_arg(arg), argno, fn->arg_type[arg - 1] & MEM_WRITE ? BPF_WRITE : BPF_READ, false, meta, NULL); break; case ARG_MEM_SIZE_OR_ZERO: - err = check_mem_size_reg(env, reg_state(env, regno - 1), reg, - argno_from_reg(regno - 1), argno, + err = check_mem_size_reg(env, get_func_arg_reg(caller, regs, arg - 1), reg, + argno_from_arg(arg), argno, fn->arg_type[arg - 1] & MEM_WRITE ? BPF_WRITE : BPF_READ, true, meta, NULL); break; -- 2.52.0 Helpers are identified by enum bpf_func_id, while kfuncs are identified by a BTF ID. Both values are stored in bpf_call_arg_meta::func_id, and a kfunc BTF ID can have the same numeric value as a BPF_FUNC_* constant. Later patches extend check_reg_type() and check_func_arg() to kfuncs. A bare func_id comparison in those common paths could then mistake a kfunc for a helper. Introduce is_helper_call(), which first excludes kfunc metadata through meta->btf, and use it for every BPF_FUNC_* comparison. This keeps helper-specific behavior out of the shared path from the start. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index d5e885435bb6..5bf31fbdaac1 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8176,6 +8176,16 @@ static bool arg_type_is_dynptr(enum bpf_arg_type type) return base_type(type) == ARG_PTR_TO_DYNPTR; } +/* + * 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 + * is known to be to a helper; meta->btf is set only for a kfunc. + */ +static bool is_helper_call(const struct bpf_call_arg_meta *meta, enum bpf_func_id func_id) +{ + return !meta->btf && meta->func_id == func_id; +} + static int resolve_map_arg_type(struct bpf_verifier_env *env, const struct bpf_call_arg_meta *meta, enum bpf_arg_type *arg_type) @@ -8197,7 +8207,7 @@ static int resolve_map_arg_type(struct bpf_verifier_env *env, } break; case BPF_MAP_TYPE_BLOOM_FILTER: - if (meta->func_id == BPF_FUNC_map_peek_elem) + if (is_helper_call(meta, BPF_FUNC_map_peek_elem)) *arg_type = ARG_PTR_TO_MAP_VALUE; break; default: @@ -8413,7 +8423,8 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re type &= ~DYNPTR_TYPE_FLAG_MASK; /* Local kptr types are allowed as the source argument of bpf_kptr_xchg */ - if (meta->func_id == BPF_FUNC_kptr_xchg && type_is_alloc(type) && reg_from_argno(argno) == BPF_REG_2) { + if (is_helper_call(meta, BPF_FUNC_kptr_xchg) && type_is_alloc(type) && + reg_from_argno(argno) == BPF_REG_2) { type &= ~MEM_ALLOC; type &= ~MEM_PERCPU; } @@ -8467,7 +8478,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re * allows bpf_sk_release to work for multiple socket types. */ bool strict_type_match = arg_type_is_release(arg_type) && - meta->func_id != BPF_FUNC_sk_release; + !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))) { @@ -8488,7 +8499,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re arg_btf_id = compatible->btf_id; } - if (meta->func_id == BPF_FUNC_kptr_xchg) { + if (is_helper_call(meta, BPF_FUNC_kptr_xchg)) { if (map_kptr_match_type(env, meta->kptr_field, reg, reg_from_argno(argno))) return -EACCES; } else { @@ -8519,13 +8530,14 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re case PTR_TO_BTF_ID | MEM_PERCPU | MEM_ALLOC: case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF: case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU: - if (meta->func_id != BPF_FUNC_spin_lock && meta->func_id != BPF_FUNC_spin_unlock && - meta->func_id != BPF_FUNC_kptr_xchg) { + if (!is_helper_call(meta, BPF_FUNC_spin_lock) && + !is_helper_call(meta, BPF_FUNC_spin_unlock) && + !is_helper_call(meta, BPF_FUNC_kptr_xchg)) { verifier_bug(env, "unimplemented handling of MEM_ALLOC"); return -EFAULT; } /* Check if local kptr in src arg matches kptr in dst arg */ - if (meta->func_id == BPF_FUNC_kptr_xchg) { + if (is_helper_call(meta, BPF_FUNC_kptr_xchg)) { int regno = reg_from_argno(argno); if (regno == BPF_REG_2 && @@ -8948,7 +8960,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, * Disable raw mode for bpf_map_peek_elem() on a bloom filter. The helper reads * the value buffer as an input rather than filling it. */ - if (meta->func_id == BPF_FUNC_map_peek_elem && + if (is_helper_call(meta, BPF_FUNC_map_peek_elem) && meta->map.ptr->map_type == BPF_MAP_TYPE_BLOOM_FILTER) meta->arg_raw_mem.regno = 0; @@ -8969,11 +8981,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, verbose(env, "can't spin_{lock,unlock} in rbtree cb\n"); return -EACCES; } - if (meta->func_id == BPF_FUNC_spin_lock) { + if (is_helper_call(meta, BPF_FUNC_spin_lock)) { err = process_spin_lock(env, reg, argno, PROCESS_SPIN_LOCK); if (err) return err; - } else if (meta->func_id == BPF_FUNC_spin_unlock) { + } else if (is_helper_call(meta, BPF_FUNC_spin_unlock)) { err = process_spin_lock(env, reg, argno, 0); if (err) return err; -- 2.52.0 Helpers are identified by enum bpf_func_id, while kfuncs are identified by a BTF ID. Both values are stored in bpf_call_arg_meta::func_id, and a helper ID can have the same numeric value as a kfunc BTF ID. Introduce is_kfunc_call(), which first confirms that the metadata is for a kfunc through meta->btf, and use it for comparisons against the special kfunc BTF ID list. This complements is_helper_call() before later patches move these checks into paths shared by helpers and kfuncs. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 68 +++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 5bf31fbdaac1..42d89db5df07 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8186,6 +8186,11 @@ static bool is_helper_call(const struct bpf_call_arg_meta *meta, enum bpf_func_i return !meta->btf && meta->func_id == func_id; } +static bool is_kfunc_call(const struct bpf_call_arg_meta *meta, u32 btf_id) +{ + return meta->btf && meta->func_id == btf_id; +} + static int resolve_map_arg_type(struct bpf_verifier_env *env, const struct bpf_call_arg_meta *meta, enum bpf_arg_type *arg_type) @@ -12137,27 +12142,27 @@ static bool is_kfunc_ret_null(struct bpf_call_arg_meta *meta) static bool is_kfunc_bpf_rcu_read_lock(struct bpf_call_arg_meta *meta) { - return meta->func_id == special_kfunc_list[KF_bpf_rcu_read_lock]; + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_rcu_read_lock]); } static bool is_kfunc_bpf_rcu_read_unlock(struct bpf_call_arg_meta *meta) { - return meta->func_id == special_kfunc_list[KF_bpf_rcu_read_unlock]; + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_rcu_read_unlock]); } static bool is_kfunc_bpf_preempt_disable(struct bpf_call_arg_meta *meta) { - return meta->func_id == special_kfunc_list[KF_bpf_preempt_disable]; + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_preempt_disable]); } static bool is_kfunc_bpf_preempt_enable(struct bpf_call_arg_meta *meta) { - return meta->func_id == special_kfunc_list[KF_bpf_preempt_enable]; + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_preempt_enable]); } bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta) { - return meta->func_id == special_kfunc_list[KF_bpf_xdp_pull_data]; + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_xdp_pull_data]); } static int @@ -12198,9 +12203,9 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, * type to our caller. When a set of conditions hold in the BTF type of * arguments, we resolve it to a known kfunc_ptr_arg_type. */ - if (meta->func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx] || - meta->func_id == special_kfunc_list[KF_bpf_session_is_return] || - meta->func_id == special_kfunc_list[KF_bpf_session_cookie]) + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_cast_to_kern_ctx]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_session_is_return]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_session_cookie])) arg_type = KF_ARG_PTR_TO_CTX; else if (btf_is_prog_ctx_type(&env->log, meta->btf, t, resolve_prog_type(env->prog), arg)) arg_type = KF_ARG_PTR_TO_CTX; @@ -12411,15 +12416,15 @@ static int process_irq_flag(struct bpf_verifier_env *env, struct bpf_reg_state * int err, spi, kfunc_class = IRQ_NATIVE_KFUNC; bool irq_save; - if (meta->func_id == special_kfunc_list[KF_bpf_local_irq_save] || - meta->func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave]) { + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_local_irq_save]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock_irqsave])) { irq_save = true; - if (meta->func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave]) + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock_irqsave])) kfunc_class = IRQ_LOCK_KFUNC; - } else if (meta->func_id == special_kfunc_list[KF_bpf_local_irq_restore] || - meta->func_id == special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore]) { + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_local_irq_restore]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore])) { irq_save = false; - if (meta->func_id == special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore]) + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore])) kfunc_class = IRQ_LOCK_KFUNC; } else { verifier_bug(env, "unknown irq flags kfunc"); @@ -13111,7 +13116,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return -EINVAL; } - if (meta->func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx]) { + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_cast_to_kern_ctx])) { ret = get_kern_ctx_btf_id(&env->log, resolve_prog_type(env->prog)); if (ret < 0) return -EINVAL; @@ -13168,17 +13173,17 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (is_kfunc_arg_uninit(btf, &args[i])) dynptr_arg_type |= MEM_UNINIT; - if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) { + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_skb])) { dynptr_arg_type |= DYNPTR_TYPE_SKB; - } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_xdp]) { + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_xdp])) { dynptr_arg_type |= DYNPTR_TYPE_XDP; - } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb_meta]) { + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_skb_meta])) { dynptr_arg_type |= DYNPTR_TYPE_SKB_META; - } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_file]) { + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_file])) { dynptr_arg_type |= DYNPTR_TYPE_FILE; - } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_file_discard]) { + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_file_discard])) { dynptr_arg_type |= DYNPTR_TYPE_FILE | OBJ_RELEASE; - } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_clone] && + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_clone]) && (dynptr_arg_type & MEM_UNINIT)) { enum bpf_dynptr_type parent_type = meta->dynptr.type; @@ -13197,7 +13202,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me break; } case KF_ARG_PTR_TO_ITER: - if (meta->func_id == special_kfunc_list[KF_bpf_iter_css_task_new]) { + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_iter_css_task_new])) { if (!check_css_task_iter_allowlist(env)) { verbose(env, "css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs\n"); return -EINVAL; @@ -13578,11 +13583,12 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (!is_bpf_res_spin_lock_kfunc(meta->func_id)) return -EFAULT; - if (meta->func_id == special_kfunc_list[KF_bpf_res_spin_lock] || - meta->func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave]) + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock_irqsave])) flags |= PROCESS_SPIN_LOCK; - if (meta->func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave] || - meta->func_id == special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore]) + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock_irqsave]) || + is_kfunc_call(meta, + special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore])) flags |= PROCESS_LOCK_IRQ; ret = process_spin_lock(env, reg, argno, flags); if (ret < 0) @@ -13925,12 +13931,12 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_call_arg struct btf_field *field = meta->arg_rbtree_root.field; mark_reg_graph_node(regs, BPF_REG_0, &field->graph_root); - } else if (meta->func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx]) { + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_cast_to_kern_ctx])) { mark_reg_known_zero(env, regs, BPF_REG_0); regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_TRUSTED; regs[BPF_REG_0].btf = desc_btf; regs[BPF_REG_0].btf_id = meta->ret_btf_id; - } else if (meta->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) { + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_rdonly_cast])) { ret_t = btf_type_by_id(desc_btf, meta->arg_constant.value); if (!ret_t) { verbose(env, "Unknown type ID %lld passed to kfunc bpf_rdonly_cast\n", @@ -13950,8 +13956,8 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_call_arg "kfunc bpf_rdonly_cast type ID argument must be of a struct or void\n"); return -EINVAL; } - } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_slice] || - meta->func_id == special_kfunc_list[KF_bpf_dynptr_slice_rdwr]) { + } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_slice]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_slice_rdwr])) { enum bpf_type_flag type_flag = get_dynptr_type_flag(meta->dynptr.type); mark_reg_known_zero(env, regs, BPF_REG_0); @@ -13966,7 +13972,7 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_call_arg /* PTR_MAYBE_NULL will be added when is_kfunc_ret_null is checked */ regs[BPF_REG_0].type = PTR_TO_MEM | type_flag; - if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_slice]) { + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_slice])) { regs[BPF_REG_0].type |= MEM_RDONLY; } else { /* this will set env->seen_direct_write to true */ -- 2.52.0 ARG_DONTCARE is the zero-valued terminator for the fixed-size helper argument array rather than an argument whose value is ignored. Rename it to ARG_UNUSED and stop helper argument iteration explicitly when it is encountered. Commit c3fd8e5fd100 ("bpf: Reject non-scalar bpf_loop iteration counts") introduced ARG_SCALAR for arguments that must be scalars. Reuse it for integer and enum global subprogram arguments, which already require SCALAR_VALUE despite being classified as ARG_ANYTHING, and for exception callback arguments. This leaves ARG_ANYTHING with its legacy helper behavior of accepting pointers when pointer leaks are allowed. Also pass ARG_PTR_TO_MEM when checking a global subprogram memory argument instead of using the prototype terminator as a placeholder. No functional change. Signed-off-by: Amery Hung --- include/linux/bpf.h | 2 +- kernel/bpf/btf.c | 2 +- kernel/bpf/verifier.c | 25 ++++++++++++------------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index e80963971f68..dcbf8cea45d6 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -874,7 +874,7 @@ enum bpf_type_flag { /* function argument constraints */ enum bpf_arg_type { - ARG_DONTCARE = 0, /* unused argument in helper function */ + ARG_UNUSED = 0, /* unused argument; terminates argument iteration */ /* the following constraints used to prototype * bpf_map_lookup/update/delete_elem() functions diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 31057c8f3a7c..122a4101ce94 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -8244,7 +8244,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) return -EINVAL; } if (btf_type_is_int(t) || btf_is_any_enum(t)) { - sub->args[i].arg_type = ARG_ANYTHING; + sub->args[i].arg_type = ARG_SCALAR; continue; } if (!is_global) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 42d89db5df07..5d21bee45805 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8843,9 +8843,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, u32 key_size; int err = 0; - if (arg_type == ARG_DONTCARE) - return 0; - if (regno >= 0) { err = check_reg_arg(env, regno, SRC_OP); if (err) @@ -9356,7 +9353,7 @@ static bool check_raw_mode_ok(const struct bpf_func_proto *fn, struct bpf_call_a int i; for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) { - if (fn->arg_type[i] == ARG_DONTCARE) + if (fn->arg_type[i] == ARG_UNUSED) break; if (!arg_type_is_raw_mem(fn->arg_type[i])) continue; @@ -9406,7 +9403,7 @@ static bool check_btf_id_ok(const struct bpf_func_proto *fn) int i; for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) { - if (fn->arg_type[i] == ARG_DONTCARE) + if (fn->arg_type[i] == ARG_UNUSED) break; if (base_type(fn->arg_type[i]) == ARG_PTR_TO_BTF_ID) return !!fn->arg_btf_id[i]; @@ -9429,7 +9426,7 @@ static bool check_mem_arg_rw_flag_ok(const struct bpf_func_proto *fn) for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) { enum bpf_arg_type arg_type = fn->arg_type[i]; - if (arg_type == ARG_DONTCARE) + if (arg_type == ARG_UNUSED) break; if (base_type(arg_type) != ARG_PTR_TO_MEM) continue; @@ -9447,7 +9444,7 @@ static bool check_proto_release_reg(const struct bpf_func_proto *fn, struct bpf_ for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) { enum bpf_arg_type arg_type = fn->arg_type[i]; - if (arg_type == ARG_DONTCARE) + if (arg_type == ARG_UNUSED) break; if (arg_type_is_release(arg_type)) { if (meta->release_regno) @@ -9818,7 +9815,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i); struct bpf_subprog_arg_info *arg = &sub->args[i]; - if (arg->arg_type == ARG_ANYTHING) { + if (arg->arg_type == ARG_SCALAR) { if (reg->type != SCALAR_VALUE) { bpf_log(log, "%s is not a scalar\n", reg_arg_name(env, argno)); return -EINVAL; @@ -9842,7 +9839,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, return -EINVAL; } } else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) { - ret = check_func_arg_reg_off(env, reg, argno, ARG_DONTCARE); + ret = check_func_arg_reg_off(env, reg, argno, ARG_PTR_TO_MEM); if (ret < 0) return ret; if (check_mem_reg(env, reg, argno, arg->mem_size, BPF_READ | BPF_WRITE, NULL, @@ -11029,6 +11026,8 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn meta.fn = fn; /* check args */ for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { + if (fn->arg_type[i] == ARG_UNUSED) + break; err = check_func_arg(env, i, &meta, insn_idx); if (err) return err; @@ -12924,7 +12923,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me for (i = 0; i < nargs; i++) { struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i); const struct btf_type *t, *ref_t, *resolve_ret; - enum bpf_arg_type arg_type = ARG_DONTCARE; + enum bpf_arg_type arg_type = ARG_UNUSED; argno_t argno = argno_from_arg(i + 1); int regno = reg_from_argno(argno); bool btf_id_fixed_off_ok = true; @@ -17991,7 +17990,7 @@ bool bpf_get_call_summary(struct bpf_verifier_env *env, struct bpf_insn *call, cs->is_void = fn->ret_type == RET_VOID; cs->num_params = 0; for (i = 0; i < ARRAY_SIZE(fn->arg_type); ++i) { - if (fn->arg_type[i] == ARG_DONTCARE) + if (fn->arg_type[i] == ARG_UNUSED) break; cs->num_params++; } @@ -19795,7 +19794,7 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) } /* Also ensure the callback only has a single scalar argument. */ - if (sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_ANYTHING) { + if (sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_SCALAR) { verbose(env, "exception cb only supports single integer argument\n"); ret = -EINVAL; goto out; @@ -19808,7 +19807,7 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) if (arg->arg_type == ARG_PTR_TO_CTX) { reg->type = PTR_TO_CTX; mark_reg_known_zero(env, regs, i); - } else if (arg->arg_type == ARG_ANYTHING) { + } else if (arg->arg_type == ARG_SCALAR) { reg->type = SCALAR_VALUE; mark_reg_unknown(env, regs, i); } else if (arg->arg_type == ARG_PTR_TO_DYNPTR) { -- 2.52.0 check_kfunc_args() classifies arguments with enum kfunc_ptr_arg_type while check_func_arg() uses enum bpf_arg_type, yet both classifications are stored in bpf_func_proto::arg_type. The overlapping namespaces force the kfunc path to translate argument kinds before calling shared checks. Fold the kfunc kinds into enum bpf_arg_type. Reuse ARG_SCALAR and the existing pointer and memory kinds where their semantics match, map kfunc callbacks to ARG_PTR_TO_FUNC, and add enumerators for the remaining kfunc-only kinds. check_kfunc_args() can then carry one classification throughout verification. Preserving the original argument kind also lets check_func_arg_reg_off() derive the zero-offset requirement for ARG_PTR_TO_REFCOUNTED_KPTR directly. Remove its separate btf_id_fixed_off_ok parameter and wrapper, along with the now-empty translation switch in check_kfunc_args(). Keep the PTR_TO_CTX offset decision based on the base argument kind. The generated kind may carry flags such as PTR_MAYBE_NULL, while the removed translation discarded them. This retains the zero-offset requirement for context arguments regardless of such flags. No functional change. Signed-off-by: Amery Hung --- include/linux/bpf.h | 14 +++ include/linux/bpf_verifier.h | 2 +- kernel/bpf/verifier.c | 223 ++++++++++++----------------------- 3 files changed, 89 insertions(+), 150 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index dcbf8cea45d6..ebfdf9d209f3 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -909,6 +909,20 @@ enum bpf_arg_type { ARG_PTR_TO_TIMER, /* pointer to bpf_timer */ ARG_KPTR_XCHG_DEST, /* pointer to destination that kptrs are bpf_kptr_xchg'd into */ ARG_PTR_TO_DYNPTR, /* pointer to bpf_dynptr. See bpf_type_flag for dynptr type */ + + ARG_CONST_SCALAR, /* scalar known at verification time */ + ARG_CONST_MEM_SIZE, /* ARG_MEM_SIZE that must be constant */ + ARG_PTR_TO_ALLOC_BTF_ID, /* pointer to an allocated object */ + ARG_PTR_TO_REFCOUNTED_KPTR, /* pointer to a refcounted local kptr */ + ARG_PTR_TO_ITER, /* pointer to an iterator */ + ARG_PTR_TO_LIST_HEAD, /* pointer to bpf_list_head */ + ARG_PTR_TO_LIST_NODE, /* pointer to bpf_list_node */ + ARG_PTR_TO_RB_ROOT, /* pointer to bpf_rb_root */ + ARG_PTR_TO_RB_NODE, /* pointer to bpf_rb_node */ + ARG_PTR_TO_WORKQUEUE, /* pointer to bpf_wq */ + ARG_PTR_TO_TASK_WORK, /* pointer to bpf_task_work */ + ARG_PTR_TO_IRQ_FLAG, /* pointer to saved IRQ flags on the stack */ + ARG_PTR_TO_RES_SPIN_LOCK, /* pointer to bpf_res_spin_lock */ __BPF_ARG_TYPE_MAX, /* Extended arg_types. */ diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 9727df5af83a..6b973b94ee75 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1591,7 +1591,7 @@ struct bpf_call_arg_meta { * verification logic * bpf_obj_drop/bpf_percpu_obj_drop * Record the local kptr type to be drop'd - * bpf_refcount_acquire (via KF_ARG_PTR_TO_REFCOUNTED_KPTR arg type) + * bpf_refcount_acquire (via ARG_PTR_TO_REFCOUNTED_KPTR arg type) * Record the local kptr type to be refcount_incr'd and use * arg_owning_ref to determine whether refcount_acquire should be * fallible diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 5d21bee45805..dc849e670876 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8579,10 +8579,9 @@ reg_find_field_offset(const struct bpf_reg_state *reg, s32 off, u32 fields) return field; } -static int __check_func_arg_reg_off(struct bpf_verifier_env *env, - const struct bpf_reg_state *reg, argno_t argno, - enum bpf_arg_type arg_type, - bool btf_id_fixed_off_ok) +static int check_func_arg_reg_off(struct bpf_verifier_env *env, + const struct bpf_reg_state *reg, argno_t argno, + enum bpf_arg_type arg_type) { u32 type = reg->type; @@ -8638,12 +8637,15 @@ static int __check_func_arg_reg_off(struct bpf_verifier_env *env, case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF: case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU: /* When referenced PTR_TO_BTF_ID is passed to release function, - * its fixed offset must be 0. In the other cases, fixed offset - * can be non-zero unless the caller requires otherwise. - * var_off always must be 0 for PTR_TO_BTF_ID, hence we still - * need to do checks instead of returning. + * its fixed offset must be 0. bpf_refcount_acquire() returns the + * pointer it was given while incrementing the refcount at the + * refcount field offset, so it needs a zero offset too. In the + * other cases, fixed offset can be non-zero. var_off always must + * be 0 for PTR_TO_BTF_ID, hence we still need to do checks + * instead of returning. */ - return __check_ptr_off_reg(env, reg, argno, btf_id_fixed_off_ok); + return __check_ptr_off_reg(env, reg, argno, + base_type(arg_type) != ARG_PTR_TO_REFCOUNTED_KPTR); case PTR_TO_CTX: /* * Allow fixed and variable offsets for syscall context, but @@ -8651,7 +8653,7 @@ static int __check_func_arg_reg_off(struct bpf_verifier_env *env, * otherwise we may get modified ctx in tail called programs and * global subprogs (that may act as extension prog hooks). */ - if (arg_type != ARG_PTR_TO_CTX && is_var_ctx_off_allowed(env->prog)) + if (base_type(arg_type) != ARG_PTR_TO_CTX && is_var_ctx_off_allowed(env->prog)) return 0; fallthrough; default: @@ -8659,13 +8661,6 @@ static int __check_func_arg_reg_off(struct bpf_verifier_env *env, } } -static int check_func_arg_reg_off(struct bpf_verifier_env *env, - const struct bpf_reg_state *reg, argno_t argno, - enum bpf_arg_type arg_type) -{ - return __check_func_arg_reg_off(env, reg, argno, arg_type, true); -} - static int check_arg_const_str(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno) { @@ -11889,34 +11884,6 @@ static void btf_member_path_str(const struct btf *btf, const struct btf_member_p } } -enum kfunc_ptr_arg_type { - KF_ARG_CONST_MEM_SIZE, - KF_ARG_MEM_SIZE, - KF_ARG_CONST, - KF_ARG_CONST_ALLOC_SIZE_OR_ZERO, - KF_ARG_ANYTHING, - KF_ARG_PTR_TO_CTX, - KF_ARG_PTR_TO_ALLOC_BTF_ID, /* Allocated object */ - KF_ARG_PTR_TO_REFCOUNTED_KPTR, /* Refcounted local kptr */ - KF_ARG_PTR_TO_DYNPTR, - KF_ARG_PTR_TO_ITER, - KF_ARG_PTR_TO_LIST_HEAD, - KF_ARG_PTR_TO_LIST_NODE, - KF_ARG_PTR_TO_BTF_ID, /* Also covers reg2btf_ids conversions */ - KF_ARG_PTR_TO_MEM, - KF_ARG_PTR_TO_CALLBACK, - KF_ARG_PTR_TO_RB_ROOT, - KF_ARG_PTR_TO_RB_NODE, - KF_ARG_PTR_TO_CONST_STR, - KF_ARG_CONST_MAP_PTR, - KF_ARG_PTR_TO_TIMER, - KF_ARG_PTR_TO_WORKQUEUE, - KF_ARG_PTR_TO_IRQ_FLAG, - KF_ARG_PTR_TO_RES_SPIN_LOCK, - KF_ARG_PTR_TO_TASK_WORK, - KF_ARG_PTR_TO_ARENA, -}; - enum special_kfunc_type { KF_bpf_obj_new_impl, KF_bpf_obj_new, @@ -12178,15 +12145,15 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, /* Scalar arguments are classified from their BTF suffix/name alone. */ if (btf_type_is_scalar(t)) { if (is_kfunc_arg_constant(meta->btf, &args[arg])) - return KF_ARG_CONST; + return ARG_CONST_SCALAR; if (is_kfunc_arg_const_mem_size(meta->btf, &args[arg])) - return KF_ARG_CONST_MEM_SIZE; + return ARG_CONST_MEM_SIZE; if (is_kfunc_arg_mem_size(meta->btf, &args[arg])) - return KF_ARG_MEM_SIZE; + return ARG_MEM_SIZE; if (is_kfunc_arg_scalar_with_name(meta->btf, &args[arg], "rdonly_buf_size") || is_kfunc_arg_scalar_with_name(meta->btf, &args[arg], "rdwr_buf_size")) - return KF_ARG_CONST_ALLOC_SIZE_OR_ZERO; - return KF_ARG_ANYTHING; + return ARG_CONST_ALLOC_SIZE_OR_ZERO; + return ARG_SCALAR; } if (!btf_type_is_ptr(t)) { @@ -12200,48 +12167,48 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, /* In this function, we verify the kfunc's BTF as per the argument type, * leaving the rest of the verification with respect to the register * type to our caller. When a set of conditions hold in the BTF type of - * arguments, we resolve it to a known kfunc_ptr_arg_type. + * arguments, we resolve it to a known bpf_arg_type. */ if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_cast_to_kern_ctx]) || is_kfunc_call(meta, special_kfunc_list[KF_bpf_session_is_return]) || is_kfunc_call(meta, special_kfunc_list[KF_bpf_session_cookie])) - arg_type = KF_ARG_PTR_TO_CTX; + arg_type = ARG_PTR_TO_CTX; else if (btf_is_prog_ctx_type(&env->log, meta->btf, t, resolve_prog_type(env->prog), arg)) - arg_type = KF_ARG_PTR_TO_CTX; + arg_type = ARG_PTR_TO_CTX; else if (is_kfunc_arg_alloc_obj(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_ALLOC_BTF_ID; + arg_type = ARG_PTR_TO_ALLOC_BTF_ID; else if (is_kfunc_arg_refcounted_kptr(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_REFCOUNTED_KPTR; + arg_type = ARG_PTR_TO_REFCOUNTED_KPTR; else if (is_kfunc_arg_dynptr(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_DYNPTR; + arg_type = ARG_PTR_TO_DYNPTR; else if (is_kfunc_arg_iter(meta, arg, &args[arg])) - arg_type = KF_ARG_PTR_TO_ITER; + arg_type = ARG_PTR_TO_ITER; else if (is_kfunc_arg_list_head(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_LIST_HEAD; + arg_type = ARG_PTR_TO_LIST_HEAD; else if (is_kfunc_arg_list_node(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_LIST_NODE; + arg_type = ARG_PTR_TO_LIST_NODE; else if (is_kfunc_arg_rbtree_root(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_RB_ROOT; + arg_type = ARG_PTR_TO_RB_ROOT; else if (is_kfunc_arg_rbtree_node(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_RB_NODE; + arg_type = ARG_PTR_TO_RB_NODE; else if (is_kfunc_arg_const_str(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_CONST_STR; + arg_type = ARG_PTR_TO_CONST_STR; else if (is_kfunc_arg_const_map(meta->btf, &args[arg])) - arg_type = KF_ARG_CONST_MAP_PTR; + arg_type = ARG_CONST_MAP_PTR; else if (is_kfunc_arg_map(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_BTF_ID; + arg_type = ARG_PTR_TO_BTF_ID; else if (is_kfunc_arg_wq(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_WORKQUEUE; + arg_type = ARG_PTR_TO_WORKQUEUE; else if (is_kfunc_arg_timer(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_TIMER; + arg_type = ARG_PTR_TO_TIMER; else if (is_kfunc_arg_task_work(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_TASK_WORK; + arg_type = ARG_PTR_TO_TASK_WORK; else if (is_kfunc_arg_irq_flag(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_IRQ_FLAG; + arg_type = ARG_PTR_TO_IRQ_FLAG; else if (is_kfunc_arg_res_spin_lock(meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_RES_SPIN_LOCK; + arg_type = ARG_PTR_TO_RES_SPIN_LOCK; else if (is_kfunc_arg_callback(env, meta->btf, &args[arg])) - arg_type = KF_ARG_PTR_TO_CALLBACK; + arg_type = ARG_PTR_TO_FUNC; else if (is_kfunc_arg_arena(meta->btf, &args[arg])) { if (!bpf_jit_supports_arena_args()) { verbose(env, "JIT does not support kfunc %s() with arena pointer arguments\n", @@ -12264,7 +12231,7 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, * whether the JIT rebases it to the arena base or preserves NULL. * The common nullable path below records that verifier property. */ - arg_type = KF_ARG_PTR_TO_ARENA; + arg_type = ARG_PTR_TO_ARENA; } else if (arg + 1 < nargs && (is_kfunc_arg_mem_size(meta->btf, &args[arg + 1]) || is_kfunc_arg_const_mem_size(meta->btf, &args[arg + 1]))) { @@ -12274,10 +12241,10 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, reg_arg_name(env, argno), btf_type_str(ref_t), ref_tname); return -EINVAL; } - arg_type = KF_ARG_PTR_TO_MEM; + arg_type = ARG_PTR_TO_MEM; } else if (btf_type_is_struct(ref_t)) - /* A pointer to a struct without a size argument is classified as KF_ARG_PTR_TO_BTF_ID */ - arg_type = KF_ARG_PTR_TO_BTF_ID; + /* A pointer to a struct without a size argument is classified as ARG_PTR_TO_BTF_ID */ + arg_type = ARG_PTR_TO_BTF_ID; else { /* * Otherwise this is a fixed-size memory buffer supported by @@ -12290,7 +12257,7 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, reg_arg_name(env, argno), btf_type_str(ref_t), ref_tname); return -EINVAL; } - arg_type = KF_ARG_PTR_TO_MEM | MEM_FIXED_SIZE; + arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE; } if (is_kfunc_arg_nullable(meta->btf, &args[arg])) @@ -12922,13 +12889,11 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me */ for (i = 0; i < nargs; i++) { struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i); + enum bpf_arg_type arg_type = meta->fn->arg_type[i]; const struct btf_type *t, *ref_t, *resolve_ret; - enum bpf_arg_type arg_type = ARG_UNUSED; argno_t argno = argno_from_arg(i + 1); int regno = reg_from_argno(argno); - bool btf_id_fixed_off_ok = true; u32 ref_id = args[i].type, type_size; - int kf_arg_type = meta->fn->arg_type[i]; if (is_kfunc_arg_prog_aux(btf, &args[i])) { /* Reject repeated use bpf_prog_aux */ @@ -12958,7 +12923,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (btf_type_is_ptr(t) && (bpf_register_is_null(reg) || type_may_be_null(reg->type)) && - !type_may_be_null(kf_arg_type)) { + !type_may_be_null(arg_type)) { const char *expected_type; expected_type = bpf_diag_fmt_btf_type(env, btf, args[i].type); @@ -12988,7 +12953,7 @@ 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(arg_type)) { ret = mark_arg_precision(env, argno); if (ret) return ret; @@ -13001,54 +12966,14 @@ 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); } - switch (base_type(kf_arg_type)) { - case KF_ARG_CONST: - case KF_ARG_CONST_MEM_SIZE: - case KF_ARG_MEM_SIZE: - case KF_ARG_ANYTHING: - case KF_ARG_CONST_ALLOC_SIZE_OR_ZERO: - case KF_ARG_PTR_TO_ALLOC_BTF_ID: - case KF_ARG_PTR_TO_BTF_ID: - case KF_ARG_CONST_MAP_PTR: - case KF_ARG_PTR_TO_ITER: - case KF_ARG_PTR_TO_LIST_HEAD: - case KF_ARG_PTR_TO_LIST_NODE: - case KF_ARG_PTR_TO_RB_ROOT: - case KF_ARG_PTR_TO_RB_NODE: - case KF_ARG_PTR_TO_MEM: - case KF_ARG_PTR_TO_CALLBACK: - case KF_ARG_PTR_TO_CONST_STR: - case KF_ARG_PTR_TO_WORKQUEUE: - case KF_ARG_PTR_TO_TIMER: - case KF_ARG_PTR_TO_TASK_WORK: - case KF_ARG_PTR_TO_IRQ_FLAG: - case KF_ARG_PTR_TO_RES_SPIN_LOCK: - case KF_ARG_PTR_TO_ARENA: - break; - case KF_ARG_PTR_TO_DYNPTR: - arg_type = ARG_PTR_TO_DYNPTR; - break; - case KF_ARG_PTR_TO_CTX: - arg_type = ARG_PTR_TO_CTX; - break; - case KF_ARG_PTR_TO_REFCOUNTED_KPTR: - arg_type = ARG_PTR_TO_BTF_ID; - btf_id_fixed_off_ok = false; - break; - default: - verifier_bug(env, "unknown kfunc arg type %d", kf_arg_type); - return -EFAULT; - } - if (regno == meta->release_regno) arg_type |= OBJ_RELEASE; - ret = __check_func_arg_reg_off(env, reg, argno, arg_type, - btf_id_fixed_off_ok); + ret = check_func_arg_reg_off(env, reg, argno, arg_type); if (ret < 0) return ret; - switch (base_type(kf_arg_type)) { - case KF_ARG_CONST: + switch (base_type(arg_type)) { + case ARG_CONST_SCALAR: if (reg->type != SCALAR_VALUE) { verbose(env, "%s is not a scalar\n", reg_arg_name(env, argno)); bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, @@ -13069,7 +12994,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return ret; } break; - case KF_ARG_ANYTHING: + case ARG_SCALAR: if (reg->type != SCALAR_VALUE) { verbose(env, "%s is not a scalar\n", reg_arg_name(env, argno)); bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, @@ -13080,7 +13005,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return -EINVAL; } break; - case KF_ARG_CONST_ALLOC_SIZE_OR_ZERO: + case ARG_CONST_ALLOC_SIZE_OR_ZERO: if (reg->type != SCALAR_VALUE) { verbose(env, "%s is not a scalar\n", reg_arg_name(env, argno)); bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, @@ -13103,7 +13028,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return ret; } break; - case KF_ARG_PTR_TO_CTX: + case ARG_PTR_TO_CTX: if (reg->type != PTR_TO_CTX) { verbose(env, "%s expected pointer to ctx, but got %s\n", reg_arg_name(env, argno), reg_type_str(env, reg->type)); @@ -13122,14 +13047,14 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me meta->ret_btf_id = ret; } break; - case KF_ARG_PTR_TO_ARENA: + case ARG_PTR_TO_ARENA: if (reg->type != PTR_TO_ARENA && reg->type != SCALAR_VALUE) { verbose(env, "%s is not a pointer to arena or scalar\n", reg_arg_name(env, argno)); return -EINVAL; } break; - case KF_ARG_PTR_TO_ALLOC_BTF_ID: + case ARG_PTR_TO_ALLOC_BTF_ID: if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC)) { if (!is_bpf_obj_drop_kfunc(meta->func_id)) { verbose(env, "%s expected for bpf_obj_drop()\n", @@ -13165,7 +13090,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me meta->arg_btf_id = reg->btf_id; } break; - case KF_ARG_PTR_TO_DYNPTR: + case ARG_PTR_TO_DYNPTR: { enum bpf_arg_type dynptr_arg_type = ARG_PTR_TO_DYNPTR; @@ -13200,7 +13125,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return ret; break; } - case KF_ARG_PTR_TO_ITER: + case ARG_PTR_TO_ITER: if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_iter_css_task_new])) { if (!check_css_task_iter_allowlist(env)) { verbose(env, "css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs\n"); @@ -13211,7 +13136,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_LIST_HEAD: + case ARG_PTR_TO_LIST_HEAD: if (reg->type != PTR_TO_MAP_VALUE && reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { verbose(env, "%s expected pointer to map value or allocated object\n", @@ -13227,7 +13152,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_RB_ROOT: + case ARG_PTR_TO_RB_ROOT: if (reg->type != PTR_TO_MAP_VALUE && reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { verbose(env, "%s expected pointer to map value or allocated object\n", @@ -13243,7 +13168,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_LIST_NODE: + case ARG_PTR_TO_LIST_NODE: if (is_kfunc_arg_nonown_allowed(btf, &args[i]) && type_is_non_owning_ref(reg->type) && !reg_is_referenced(env, reg)) { /* Allow bpf_list_front/back return value for @@ -13265,7 +13190,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_RB_NODE: + case ARG_PTR_TO_RB_NODE: if (is_bpf_rbtree_add_kfunc(meta->func_id)) { if (reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { verbose(env, "%s expected pointer to allocated object\n", @@ -13292,7 +13217,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_CONST_MAP_PTR: + case ARG_CONST_MAP_PTR: if (base_type(reg->type) != CONST_PTR_TO_MAP || type_may_be_null(reg->type)) { verbose(env, "pointer in %s isn't map pointer\n", @@ -13303,7 +13228,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_BTF_ID: + case ARG_PTR_TO_BTF_ID: /* Only base_type is checked, further checks are done here */ if (base_type(reg->type) == PTR_TO_BTF_ID || reg2btf_ids[base_type(reg->type)]) { @@ -13368,10 +13293,10 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me * If the register does not contain btf id but the argument type is a pointer to * scalar-only struct, allow verifying it as a fixed size memory. */ - kf_arg_type = KF_ARG_PTR_TO_MEM | MEM_FIXED_SIZE; + arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE; fallthrough; - case KF_ARG_PTR_TO_MEM: - if (kf_arg_type & MEM_FIXED_SIZE) { + case ARG_PTR_TO_MEM: + if (arg_type & MEM_FIXED_SIZE) { bool known_memory; resolve_ret = btf_resolve_size(btf, ref_t, &type_size); @@ -13405,7 +13330,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me } } break; - case KF_ARG_CONST_MEM_SIZE: + case ARG_CONST_MEM_SIZE: ret = process_const_arg(env, reg, argno, meta); if (ret < 0) { if (ret == -EINVAL) @@ -13416,7 +13341,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return ret; } fallthrough; - case KF_ARG_MEM_SIZE: + case ARG_MEM_SIZE: { struct bpf_reg_state *buff_reg = get_func_arg_reg(caller, regs, i - 1); struct bpf_reg_state *size_reg = reg; @@ -13469,14 +13394,14 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me } break; } - case KF_ARG_PTR_TO_CALLBACK: + case ARG_PTR_TO_FUNC: if (reg->type != PTR_TO_FUNC) { verbose(env, "%s expected pointer to func\n", reg_arg_name(env, argno)); return -EINVAL; } meta->subprogno = reg->subprogno; break; - case KF_ARG_PTR_TO_REFCOUNTED_KPTR: + case ARG_PTR_TO_REFCOUNTED_KPTR: if (!type_is_ptr_alloc_obj(reg->type)) { verbose(env, "%s is neither owning or non-owning ref\n", reg_arg_name(env, argno)); @@ -13505,7 +13430,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me meta->arg_btf = reg->btf; meta->arg_btf_id = reg->btf_id; break; - case KF_ARG_PTR_TO_CONST_STR: + case ARG_PTR_TO_CONST_STR: if (reg->type != PTR_TO_MAP_VALUE) { verbose(env, "%s doesn't point to a const string\n", reg_arg_name(env, argno)); @@ -13520,7 +13445,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret) return ret; break; - case KF_ARG_PTR_TO_WORKQUEUE: + case ARG_PTR_TO_WORKQUEUE: if (reg->type != PTR_TO_MAP_VALUE) { verbose(env, "%s doesn't point to a map value\n", reg_arg_name(env, argno)); @@ -13530,7 +13455,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_TIMER: + case ARG_PTR_TO_TIMER: if (reg->type != PTR_TO_MAP_VALUE) { verbose(env, "%s doesn't point to a map value\n", reg_arg_name(env, argno)); @@ -13540,7 +13465,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_TASK_WORK: + case ARG_PTR_TO_TASK_WORK: if (reg->type != PTR_TO_MAP_VALUE) { verbose(env, "%s doesn't point to a map value\n", reg_arg_name(env, argno)); @@ -13550,7 +13475,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_IRQ_FLAG: + case ARG_PTR_TO_IRQ_FLAG: if (reg->type != PTR_TO_STACK) { verbose(env, "%s doesn't point to an irq flag on stack\n", reg_arg_name(env, argno)); @@ -13565,7 +13490,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (ret < 0) return ret; break; - case KF_ARG_PTR_TO_RES_SPIN_LOCK: + case ARG_PTR_TO_RES_SPIN_LOCK: { int flags = PROCESS_RES_LOCK; -- 2.52.0 The verifier does not inspect __ign arguments or implicit arguments, except that bpf_prog_aux is handled specially. get_kfunc_arg_type() leaves the ignored arguments unclassified, so gen_kfunc_arg_proto() skips them and check_kfunc_args() repeats the same BTF predicates for every call. Add ARG_IGNORE and classify ignored arguments in get_kfunc_arg_type(). The generated prototype can then record them, and the call site can consume the recorded type without deriving the classification again. Keep bpf_prog_aux on its existing BTF-based path for now. No functional change. Signed-off-by: Amery Hung --- include/linux/bpf.h | 1 + kernel/bpf/verifier.c | 13 ++++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index ebfdf9d209f3..54af5562ec95 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -923,6 +923,7 @@ enum bpf_arg_type { ARG_PTR_TO_TASK_WORK, /* pointer to bpf_task_work */ ARG_PTR_TO_IRQ_FLAG, /* pointer to saved IRQ flags on the stack */ ARG_PTR_TO_RES_SPIN_LOCK, /* pointer to bpf_res_spin_lock */ + ARG_IGNORE, /* argument the verifier does not check at all */ __BPF_ARG_TYPE_MAX, /* Extended arg_types. */ diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index dc849e670876..09e96d04801d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8838,6 +8838,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, u32 key_size; int err = 0; + if (arg_type == ARG_IGNORE) + return 0; + if (regno >= 0) { err = check_reg_arg(env, regno, SRC_OP); if (err) @@ -12140,6 +12143,9 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, const char *ref_tname = NULL; int arg_type; + if (is_kfunc_arg_ignore(meta->btf, &args[arg]) || is_kfunc_arg_implicit(meta, arg)) + return ARG_IGNORE; + t = btf_type_skip_modifiers(meta->btf, args[arg].type, NULL); /* Scalar arguments are classified from their BTF suffix/name alone. */ @@ -12269,7 +12275,6 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, static int gen_kfunc_arg_proto(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, struct bpf_func_proto *proto) { - const struct btf *btf = meta->btf; const struct btf_param *args; u32 i, nargs; int arg_type; @@ -12288,9 +12293,7 @@ static int gen_kfunc_arg_proto(struct bpf_verifier_env *env, struct bpf_call_arg } for (i = 0; i < nargs; i++) { - if (is_kfunc_arg_prog_aux(btf, &args[i]) || - is_kfunc_arg_ignore(btf, &args[i]) || - is_kfunc_arg_implicit(meta, i)) + if (is_kfunc_arg_prog_aux(meta->btf, &args[i])) continue; arg_type = get_kfunc_arg_type(env, meta, args, i, nargs); @@ -12911,7 +12914,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me continue; } - if (is_kfunc_arg_ignore(btf, &args[i]) || is_kfunc_arg_implicit(meta, i)) + if (arg_type == ARG_IGNORE) continue; t = btf_type_skip_modifiers(btf, args[i].type, NULL); -- 2.52.0 The verifier supplies a bpf_prog_aux argument to both bpf_timer_set_callback() and kfuncs rather than reading it from the BPF program. The helper prototype leaves its third argument unused, while the kfunc path identifies the argument from BTF at every call. Add ARG_PTR_TO_PROG_AUX, record it in the helper prototype, and classify the kfunc argument when its prototype is generated. Because other ignored and implicit arguments are already represented by ARG_IGNORE, the generated prototype has no internal ARG_UNUSED gaps. Validate in one place that it contains at most one prog-aux argument and that the argument is register-passed, as required by the BPF_LD_IMM64 fixup. Record the argument register in the per-instruction metadata for both call kinds. Drive the helper fixup from that metadata instead of the helper ID and its hard-coded R3, matching the existing kfunc fixup. bpf_call_arg_meta::arg_prog is then no longer needed. No functional change beyond reporting invalid kfunc signatures when the call is added rather than when it is verified. Signed-off-by: Amery Hung --- include/linux/bpf.h | 1 + include/linux/bpf_verifier.h | 1 - kernel/bpf/fixups.c | 5 +-- kernel/bpf/helpers.c | 1 + kernel/bpf/verifier.c | 66 +++++++++++++++++++++++++----------- 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 54af5562ec95..aa4d3bb5e8cc 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -923,6 +923,7 @@ enum bpf_arg_type { ARG_PTR_TO_TASK_WORK, /* pointer to bpf_task_work */ ARG_PTR_TO_IRQ_FLAG, /* pointer to saved IRQ flags on the stack */ ARG_PTR_TO_RES_SPIN_LOCK, /* pointer to bpf_res_spin_lock */ + ARG_PTR_TO_PROG_AUX, /* pointer to the caller's bpf_prog_aux */ ARG_IGNORE, /* argument the verifier does not check at all */ __BPF_ARG_TYPE_MAX, diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 6b973b94ee75..1e7593e8d5c5 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1599,7 +1599,6 @@ struct bpf_call_arg_meta { struct btf *arg_btf; u32 arg_btf_id; bool arg_owning_ref; - bool arg_prog; struct { struct btf_field *field; diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index fcf68cfb91e9..2add8001c3ec 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -2020,7 +2020,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) goto next_insn; } - if (insn->imm == BPF_FUNC_timer_set_callback) { + aux = &env->insn_aux_data[i + delta]; + if (aux->arg_prog) { /* The verifier will process callback_fn as many times as necessary * with different maps and the register states prepared by * set_timer_callback_state will be accurate. @@ -2035,7 +2036,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) * bpf_timer_set_callback-ed will return -EINVAL. */ struct bpf_insn ld_addrs[2] = { - BPF_LD_IMM64(BPF_REG_3, (long)prog->aux), + BPF_LD_IMM64(aux->arg_prog, (long)prog->aux), }; insn_buf[0] = ld_addrs[0]; diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index b3cc5c8fc875..051b6654e57c 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1510,6 +1510,7 @@ static const struct bpf_func_proto bpf_timer_set_callback_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_TIMER, .arg2_type = ARG_PTR_TO_FUNC, + .arg3_type = ARG_PTR_TO_PROG_AUX, }; static bool defer_timer_wq_op(void) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 09e96d04801d..c7822addfc64 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8783,6 +8783,7 @@ static int get_constant_map_key(struct bpf_verifier_env *env, } static bool can_elide_value_nullness(const struct bpf_map *map); +static struct bpf_insn_aux_data *cur_aux(const struct bpf_verifier_env *env); static int process_map_ptr_arg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, struct bpf_call_arg_meta *meta) @@ -8838,6 +8839,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, u32 key_size; int err = 0; + if (arg_type == ARG_PTR_TO_PROG_AUX) { + cur_aux(env)->arg_prog = regno; + return 0; + } + if (arg_type == ARG_IGNORE) return 0; @@ -9454,9 +9460,42 @@ static bool check_proto_release_reg(const struct bpf_func_proto *fn, struct bpf_ return true; } -static int check_func_proto(const struct bpf_func_proto *fn, struct bpf_call_arg_meta *meta) +static bool check_arg_prog_aux(struct bpf_verifier_env *env, + const struct bpf_func_proto *proto) +{ + bool seen = false; + argno_t argno; + u32 i; + + for (i = 0; i < ARRAY_SIZE(proto->arg_type); i++) { + if (proto->arg_type[i] == ARG_UNUSED) + break; + if (proto->arg_type[i] != ARG_PTR_TO_PROG_AUX) + continue; + + if (seen) { + verifier_bug(env, "Only 1 prog->aux argument supported"); + return false; + } + + argno = argno_from_arg(i + 1); + if (reg_from_argno(argno) < 0) { + verbose(env, "%s prog->aux cannot be a stack argument\n", + reg_arg_name(env, argno)); + return false; + } + + seen = true; + } + + return true; +} + +static int check_func_proto(struct bpf_verifier_env *env, const struct bpf_func_proto *fn, + struct bpf_call_arg_meta *meta) { - return check_raw_mode_ok(fn, meta) && + return check_arg_prog_aux(env, fn) && + check_raw_mode_ok(fn, meta) && check_arg_pair_ok(fn) && check_mem_arg_rw_flag_ok(fn) && check_proto_release_reg(fn, meta) && @@ -10999,7 +11038,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn memset(&meta, 0, sizeof(meta)); - err = check_func_proto(fn, &meta); + err = check_func_proto(env, fn, &meta); if (err) { verifier_bug(env, "incorrect func proto %s#%d", func_id_name(func_id), func_id); return err; @@ -12143,6 +12182,9 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, const char *ref_tname = NULL; int arg_type; + if (is_kfunc_arg_prog_aux(meta->btf, &args[arg])) + return ARG_PTR_TO_PROG_AUX; + if (is_kfunc_arg_ignore(meta->btf, &args[arg]) || is_kfunc_arg_implicit(meta, arg)) return ARG_IGNORE; @@ -12293,9 +12335,6 @@ static int gen_kfunc_arg_proto(struct bpf_verifier_env *env, struct bpf_call_arg } for (i = 0; i < nargs; i++) { - if (is_kfunc_arg_prog_aux(meta->btf, &args[i])) - continue; - arg_type = get_kfunc_arg_type(env, meta, args, i, nargs); if (arg_type < 0) return arg_type; @@ -12303,7 +12342,7 @@ static int gen_kfunc_arg_proto(struct bpf_verifier_env *env, struct bpf_call_arg proto->arg_type[i] = arg_type; } - return 0; + return check_arg_prog_aux(env, proto) ? 0 : -EINVAL; } static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env, @@ -12898,18 +12937,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me int regno = reg_from_argno(argno); u32 ref_id = args[i].type, type_size; - if (is_kfunc_arg_prog_aux(btf, &args[i])) { - /* Reject repeated use bpf_prog_aux */ - if (meta->arg_prog) { - verifier_bug(env, "Only 1 prog->aux argument supported per-kfunc"); - return -EFAULT; - } - if (regno < 0) { - verbose(env, "%s prog->aux cannot be a stack argument\n", - reg_arg_name(env, argno)); - return -EINVAL; - } - meta->arg_prog = true; + if (arg_type == ARG_PTR_TO_PROG_AUX) { cur_aux(env)->arg_prog = regno; continue; } -- 2.52.0 check_kfunc_args() ORs OBJ_RELEASE into arg_type on every verification of every argument of every call, from a comparison against meta->release_regno. Neither side of that comparison depends on the call site. bpf_fetch_kfunc_arg_meta() sets release_regno to BPF_REG_1 when the kfunc is KF_RELEASE and leaves it zero otherwise, and regno is derived from the argument index, so the test is true exactly for argument 0 of a KF_RELEASE kfunc. Set the flag in get_kfunc_arg_type() instead, where the rest of the classification is built, and drop the comparison. The release handling in check_func_proto() is helper-only, so an OBJ_RELEASE in a generated kfunc prototype does not feed back into meta->release_regno. meta->release_regno itself stays: it is still what tells the referenced PTR_TO_BTF_ID check and release_reg() which register to act on. No functional change. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index c7822addfc64..20e7da6674a1 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -12311,6 +12311,13 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, if (is_kfunc_arg_nullable(meta->btf, &args[arg])) arg_type |= PTR_MAYBE_NULL; + /* + * Only the first argument of a KF_RELEASE kfunc releases anything, and + * bpf_fetch_kfunc_arg_meta() only ever records BPF_REG_1 for it. + */ + if (is_kfunc_release(meta) && arg == 0) + arg_type |= OBJ_RELEASE; + return arg_type; } @@ -12997,8 +13004,6 @@ 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 (regno == meta->release_regno) - arg_type |= OBJ_RELEASE; ret = check_func_arg_reg_off(env, reg, argno, arg_type); if (ret < 0) return ret; -- 2.52.0 The dynptr flavour a kfunc argument expects is rebuilt on every verification of the call: check_kfunc_args() starts from ARG_PTR_TO_DYNPTR, adds MEM_UNINIT from the __uninit suffix, and then walks a chain of func_id comparisons to add DYNPTR_TYPE_SKB, _XDP, _SKB_META or _FILE. All of that is fixed by the kfunc identity and BTF, so move it into get_kfunc_arg_type() and let the generated prototype carry it. Inheriting the classification also means PTR_MAYBE_NULL reaches process_dynptr_func() for a __nullable dynptr argument, where it did not before, and is_dynptr_type_expected() tested for any type of dynptr with an exact arg_type == ARG_PTR_TO_DYNPTR comparison. Test the DYNPTR_TYPE_FLAG_MASK bits instead, which is what the comment there already claims to mean and does not care about unrelated flags. What cannot move is the KF_bpf_dynptr_clone arm, which takes its type from meta->dynptr.type, recorded while verifying the parent dynptr argument earlier in the same call. That stays at the call site, applied on top of the cached classification. No functional change. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 47 ++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 20e7da6674a1..c602007bf583 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -960,8 +960,12 @@ static enum bpf_dynptr_type dynptr_reg_type(struct bpf_verifier_env *env, struct static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg_state *reg, enum bpf_arg_type arg_type) { - /* ARG_PTR_TO_DYNPTR takes any type of dynptr */ - if (arg_type == ARG_PTR_TO_DYNPTR) + /* + * ARG_PTR_TO_DYNPTR without a type flag takes any type of dynptr. + * Test the flags rather than the whole arg_type, which may carry + * unrelated ones such as PTR_MAYBE_NULL. + */ + if (!(arg_type & DYNPTR_TYPE_FLAG_MASK)) return true; return dynptr_reg_type(env, reg) == arg_to_dynptr_type(arg_type); @@ -12227,9 +12231,20 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, arg_type = ARG_PTR_TO_ALLOC_BTF_ID; else if (is_kfunc_arg_refcounted_kptr(meta->btf, &args[arg])) arg_type = ARG_PTR_TO_REFCOUNTED_KPTR; - else if (is_kfunc_arg_dynptr(meta->btf, &args[arg])) + else if (is_kfunc_arg_dynptr(meta->btf, &args[arg])) { arg_type = ARG_PTR_TO_DYNPTR; - else if (is_kfunc_arg_iter(meta, arg, &args[arg])) + + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_skb])) + arg_type |= DYNPTR_TYPE_SKB; + else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_xdp])) + arg_type |= DYNPTR_TYPE_XDP; + else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_skb_meta])) + arg_type |= DYNPTR_TYPE_SKB_META; + else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_file]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_file_discard])) + /* OBJ_RELEASE for the latter comes from KF_RELEASE below */ + arg_type |= DYNPTR_TYPE_FILE; + } else if (is_kfunc_arg_iter(meta, arg, &args[arg])) arg_type = ARG_PTR_TO_ITER; else if (is_kfunc_arg_list_head(meta->btf, &args[arg])) arg_type = ARG_PTR_TO_LIST_HEAD; @@ -12308,6 +12323,9 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE; } + if (is_kfunc_arg_uninit(meta->btf, &args[arg])) + arg_type |= MEM_UNINIT; + if (is_kfunc_arg_nullable(meta->btf, &args[arg])) arg_type |= PTR_MAYBE_NULL; @@ -13128,23 +13146,10 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me break; case ARG_PTR_TO_DYNPTR: { - enum bpf_arg_type dynptr_arg_type = ARG_PTR_TO_DYNPTR; - - if (is_kfunc_arg_uninit(btf, &args[i])) - dynptr_arg_type |= MEM_UNINIT; - - if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_skb])) { - dynptr_arg_type |= DYNPTR_TYPE_SKB; - } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_xdp])) { - dynptr_arg_type |= DYNPTR_TYPE_XDP; - } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_skb_meta])) { - dynptr_arg_type |= DYNPTR_TYPE_SKB_META; - } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_from_file])) { - dynptr_arg_type |= DYNPTR_TYPE_FILE; - } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_file_discard])) { - dynptr_arg_type |= DYNPTR_TYPE_FILE | OBJ_RELEASE; - } else if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_clone]) && - (dynptr_arg_type & MEM_UNINIT)) { + enum bpf_arg_type dynptr_arg_type = arg_type; + + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_clone]) && + (dynptr_arg_type & MEM_UNINIT)) { enum bpf_dynptr_type parent_type = meta->dynptr.type; if (parent_type == BPF_DYNPTR_TYPE_INVALID) { -- 2.52.0 The BTF ID argument of a kfunc has to be referenced or trusted unless the kfunc is KF_RCU, in which case an RCU-protected pointer is accepted too. check_kfunc_args() reads that off meta->kfunc_flags on every verification of the call, which is the last thing keeping the argument policy of ARG_PTR_TO_BTF_ID out of the generated prototype. Set MEM_RCU in get_kfunc_arg_type() for ARG_PTR_TO_BTF_ID arguments of KF_RCU kfuncs, the way it already sets OBJ_RELEASE for release arguments, and test the generated flag instead. MEM_RCU represents the KF_RCU relaxation applied by the ARG_PTR_TO_BTF_ID trust check; other argument kinds have their own provenance requirements. No functional change intended. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index c602007bf583..39f7f633b4d6 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -12336,6 +12336,14 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, if (is_kfunc_release(meta) && arg == 0) arg_type |= OBJ_RELEASE; + /* + * A KF_RCU kfunc accepts an RCU-protected pointer where it would + * otherwise demand a referenced or trusted one. Other argument kinds + * have their own provenance requirements and must not inherit MEM_RCU. + */ + if (base_type(arg_type) == ARG_PTR_TO_BTF_ID && is_kfunc_rcu(meta)) + arg_type |= MEM_RCU; + return arg_type; } @@ -13275,7 +13283,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me reg2btf_ids[base_type(reg->type)]) { if (!is_trusted_reg(env, reg) || bpf_type_has_unsafe_modifiers(reg->type)) { - if (!is_kfunc_rcu(meta)) { + if (!(arg_type & MEM_RCU)) { const char *expected_type; expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); -- 2.52.0 check_kfunc_args() walks the kfunc's BTF on every verification of a call to work out which BTF ID an ARG_PTR_TO_BTF_ID argument expects. Helpers name theirs in bpf_func_proto::arg_btf_id[], as a pointer to a BTF ID that resolve_btfids fills in at build time. The ID of a kfunc argument's referent is already stored in the immutable BTF records that describe its pointer and modifier chain. Make arg_btf_id[] point to the BTF field containing the resolved ID. A __map argument instead uses the existing vmlinux BTF ID pointer. Produce this metadata alongside the argument classification in get_kfunc_arg_type(). Declare the argument BTF ID pointers const, since the verifier only reads through them. The BTF object owns their storage and remains alive while the generated prototype is used, so the pointers remain valid when the kfunc descriptor array is reallocated or sorted. Both helper and kfunc callers can now read the expected BTF ID through the same bpf_func_proto field, which lets check_func_arg() take over the ARG_PTR_TO_BTF_ID case. arg_btf_id[] shares a union with arg_size[], so an argument cannot store both. The scalar-struct memory fallback keeps resolving its byte size at verification time, as it does today. No functional change. Signed-off-by: Amery Hung --- include/linux/bpf.h | 12 ++++++------ kernel/bpf/verifier.c | 42 ++++++++++++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index aa4d3bb5e8cc..d0066d744ceb 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1021,13 +1021,13 @@ struct bpf_func_proto { }; union { struct { - u32 *arg1_btf_id; - u32 *arg2_btf_id; - u32 *arg3_btf_id; - u32 *arg4_btf_id; - u32 *arg5_btf_id; + const u32 *arg1_btf_id; + const u32 *arg2_btf_id; + const u32 *arg3_btf_id; + const u32 *arg4_btf_id; + const u32 *arg5_btf_id; }; - u32 *arg_btf_id[MAX_BPF_FUNC_ARGS]; + const u32 *arg_btf_id[MAX_BPF_FUNC_ARGS]; struct { size_t arg1_size; size_t arg2_size; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 39f7f633b4d6..0547fbeeebbe 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8839,7 +8839,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, enum bpf_arg_type arg_type = fn->arg_type[arg]; int regno = reg_from_argno(argno); enum bpf_reg_type type = reg->type; - u32 *arg_btf_id = NULL; + const u32 *arg_btf_id = NULL; u32 key_size; int err = 0; @@ -12179,13 +12179,18 @@ bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta) static int get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, - const struct btf_param *args, int arg, int nargs) + const struct btf_param *args, int arg, int nargs, + struct bpf_func_proto *proto) { const struct btf_type *t, *ref_t = NULL; + const u32 *ref_id_ptr = NULL; argno_t argno = argno_from_arg(arg + 1); const char *ref_tname = NULL; + u32 ref_id; int arg_type; + proto->arg_btf_id[arg] = NULL; + if (is_kfunc_arg_prog_aux(meta->btf, &args[arg])) return ARG_PTR_TO_PROG_AUX; @@ -12213,7 +12218,11 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, reg_arg_name(env, argno), btf_type_str(t)); return -EINVAL; } - ref_t = btf_type_skip_modifiers(meta->btf, t->type, NULL); + /* Keep a pointer to the BTF field containing the resolved referent ID. */ + ref_id_ptr = &t->type; + ref_t = btf_type_skip_modifiers(meta->btf, *ref_id_ptr, &ref_id); + while (*ref_id_ptr != ref_id) + ref_id_ptr = &btf_type_by_id(meta->btf, *ref_id_ptr)->type; ref_tname = btf_name_by_offset(meta->btf, ref_t->name_off); /* In this function, we verify the kfunc's BTF as per the argument type, @@ -12336,13 +12345,20 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, if (is_kfunc_release(meta) && arg == 0) arg_type |= OBJ_RELEASE; - /* - * A KF_RCU kfunc accepts an RCU-protected pointer where it would - * otherwise demand a referenced or trusted one. Other argument kinds - * have their own provenance requirements and must not inherit MEM_RCU. - */ - if (base_type(arg_type) == ARG_PTR_TO_BTF_ID && is_kfunc_rcu(meta)) - arg_type |= MEM_RCU; + if (base_type(arg_type) == ARG_PTR_TO_BTF_ID) { + if (is_kfunc_arg_map(meta->btf, &args[arg])) + proto->arg_btf_id[arg] = reg2btf_ids[CONST_PTR_TO_MAP]; + else + proto->arg_btf_id[arg] = ref_id_ptr; + + /* + * A KF_RCU kfunc accepts an RCU-protected pointer where it would + * otherwise demand a referenced or trusted one. Other argument kinds + * have their own provenance requirements and must not inherit MEM_RCU. + */ + if (is_kfunc_rcu(meta)) + arg_type |= MEM_RCU; + } return arg_type; } @@ -12368,7 +12384,7 @@ static int gen_kfunc_arg_proto(struct bpf_verifier_env *env, struct bpf_call_arg } for (i = 0; i < nargs; i++) { - arg_type = get_kfunc_arg_type(env, meta, args, i, nargs); + arg_type = get_kfunc_arg_type(env, meta, args, i, nargs, proto); if (arg_type < 0) return arg_type; @@ -13024,8 +13040,10 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me continue; } + if (base_type(arg_type) == ARG_PTR_TO_BTF_ID) + ref_id = *meta->fn->arg_btf_id[i]; + if (is_kfunc_arg_map(btf, &args[i])) { - ref_id = *reg2btf_ids[CONST_PTR_TO_MAP]; ref_t = btf_type_by_id(btf_vmlinux, ref_id); ref_tname = btf_name_by_offset(btf, ref_t->name_off); } -- 2.52.0 A generated kfunc prototype classifies a pointer to scalar memory without an adjacent size argument as ARG_PTR_TO_MEM | MEM_FIXED_SIZE. The kfunc path currently walks BTF and resolves the pointee size each time it verifies a call. The common check_func_arg() path instead expects that size in bpf_func_proto::arg_size[]. Resolve the size when generating the prototype and teach the existing kfunc path to consume the cached value. This prepares fixed-size memory arguments to move to the common checker. arg_size[] shares storage with arg_btf_id[]. An ARG_PTR_TO_BTF_ID argument that falls back to scalar-struct memory therefore keeps its BTF ID pointer and continues resolving the size when that fallback is taken. No functional change. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 0547fbeeebbe..615a5667e569 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -12182,11 +12182,11 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, const struct btf_param *args, int arg, int nargs, struct bpf_func_proto *proto) { - const struct btf_type *t, *ref_t = NULL; + const struct btf_type *t, *ref_t = NULL, *resolve_ret; const u32 *ref_id_ptr = NULL; argno_t argno = argno_from_arg(arg + 1); const char *ref_tname = NULL; - u32 ref_id; + u32 ref_id, type_size; int arg_type; proto->arg_btf_id[arg] = NULL; @@ -12329,6 +12329,15 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, reg_arg_name(env, argno), btf_type_str(ref_t), ref_tname); return -EINVAL; } + resolve_ret = btf_resolve_size(meta->btf, ref_t, &type_size); + if (IS_ERR(resolve_ret)) { + verbose(env, + "%s reference type('%s %s') size cannot be determined: %ld\n", + reg_arg_name(env, argno), btf_type_str(ref_t), + ref_tname, PTR_ERR(resolve_ret)); + return -EINVAL; + } + proto->arg_size[arg] = type_size; arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE; } @@ -12984,7 +12993,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me const struct btf_type *t, *ref_t, *resolve_ret; argno_t argno = argno_from_arg(i + 1); int regno = reg_from_argno(argno); - u32 ref_id = args[i].type, type_size; + u32 ref_id = args[i].type; + u32 arg_size = arg_type & MEM_FIXED_SIZE ? meta->fn->arg_size[i] : 0; if (arg_type == ARG_PTR_TO_PROG_AUX) { cur_aux(env)->arg_prog = regno; @@ -13360,20 +13370,21 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me * If the register does not contain btf id but the argument type is a pointer to * scalar-only struct, allow verifying it as a fixed size memory. */ + resolve_ret = btf_resolve_size(btf, ref_t, &arg_size); + if (IS_ERR(resolve_ret)) { + verbose(env, + "%s reference type('%s %s') size cannot be determined: %ld\n", + reg_arg_name(env, argno), btf_type_str(ref_t), + ref_tname, PTR_ERR(resolve_ret)); + return -EINVAL; + } arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE; fallthrough; case ARG_PTR_TO_MEM: if (arg_type & MEM_FIXED_SIZE) { bool known_memory; - resolve_ret = btf_resolve_size(btf, ref_t, &type_size); - if (IS_ERR(resolve_ret)) { - verbose(env, "%s reference type('%s %s') size cannot be determined: %ld\n", - reg_arg_name(env, argno), btf_type_str(ref_t), - ref_tname, PTR_ERR(resolve_ret)); - return -EINVAL; - } - ret = check_mem_reg(env, reg, argno, type_size, BPF_READ | BPF_WRITE, + ret = check_mem_reg(env, reg, argno, arg_size, BPF_READ | BPF_WRITE, meta, &known_memory); if (ret < 0) { const char *expected_type; @@ -13384,14 +13395,14 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me env, insn_idx, argno, func_name, "Pass memory with at least the required number of accessible bytes and suitable read and write access.", "the kfunc expects %u bytes of memory for %s, but the verifier cannot prove that %s provides a readable and writable range of that size", - type_size, expected_type, + arg_size, expected_type, bpf_diag_reg_type_plain(env, reg->type)); else bpf_diag_call_arg_fmt( env, insn_idx, argno, func_name, "Pass stack, map, context, or other verifier-known memory of the expected type and size, not an integer cast to a pointer.", "the kfunc expects %u bytes of memory for %s, but it is %s and not verifier-known memory", - type_size, expected_type, + arg_size, expected_type, bpf_diag_reg_type_plain(env, reg->type)); return ret; } -- 2.52.0 A function argument type can require call-site refinement before register admission. Helpers refine map-value arguments according to the recorded map type, while kfuncs reinterpret a pointer to a scalar-only BTF struct as fixed-size memory when the register does not carry a BTF ID. Add resolve_func_arg_type() as the common entry point. It delegates helper map-value refinement to the existing resolve_map_arg_type(), then handles the kfunc BTF-to-memory fallback. It updates the effective argument type and size consumed by the subsequent checks. Preserve PTR_MAYBE_NULL when changing the base type so later nullability validation observes the BTF contract. Keep OBJ_RELEASE arguments in their original pointer class. The latter is not a bug fix: existing kfunc checking already validates ownership before this resolver. It preserves release metadata for the later shared checking order, where ownership validation follows register admission. Moving the kfunc fallback before admission lets the resulting fixed-size memory argument enter the compatibility check directly. This is another step toward routing both call types through check_func_arg(). Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 194 ++++++++++++++++++++++++------------------ 1 file changed, 109 insertions(+), 85 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 615a5667e569..4e7230cb5791 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8196,8 +8196,8 @@ static bool is_kfunc_call(const struct bpf_call_arg_meta *meta, u32 btf_id) } static int resolve_map_arg_type(struct bpf_verifier_env *env, - const struct bpf_call_arg_meta *meta, - enum bpf_arg_type *arg_type) + const struct bpf_call_arg_meta *meta, + enum bpf_arg_type *arg_type) { if (!meta->map.ptr) { /* kernel subsystem misconfigured verifier */ @@ -8225,6 +8225,11 @@ static int resolve_map_arg_type(struct bpf_verifier_env *env, return 0; } +static int resolve_func_arg_type(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, u32 arg, + struct bpf_call_arg_meta *meta, int insn_idx, + enum bpf_arg_type *arg_type, u32 *arg_size); + struct bpf_reg_types { const enum bpf_reg_type types[10]; u32 *btf_id; @@ -8840,6 +8845,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, int regno = reg_from_argno(argno); enum bpf_reg_type type = reg->type; const u32 *arg_btf_id = NULL; + u32 arg_size = arg_type & MEM_FIXED_SIZE ? fn->arg_size[arg] : 0; u32 key_size; int err = 0; @@ -8872,11 +8878,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return -EACCES; } - if (base_type(arg_type) == ARG_PTR_TO_MAP_VALUE) { - err = resolve_map_arg_type(env, meta, &arg_type); - if (err) - return err; - } + err = resolve_func_arg_type(env, reg, arg, meta, insn_idx, &arg_type, &arg_size); + if (err) + return err; if (bpf_register_is_null(reg) && type_may_be_null(arg_type)) { /* A NULL register has a SCALAR_VALUE type, so skip @@ -9017,12 +9021,12 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, * next is_mem_size argument below. */ if (arg_type & MEM_FIXED_SIZE) { - err = check_mem_reg(env, reg, argno, fn->arg_size[arg], + err = check_mem_reg(env, reg, argno, arg_size, arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ, meta, NULL); if (err) return err; if (arg_type & MEM_ALIGNED) - err = check_ptr_alignment(env, reg, 0, fn->arg_size[arg], true); + err = check_ptr_alignment(env, reg, 0, arg_size, true); } break; case ARG_MEM_SIZE: @@ -11911,6 +11915,65 @@ static bool btf_type_is_scalar_struct(struct bpf_verifier_env *env, return btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR); } +static int resolve_func_arg_type(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, u32 arg, + struct bpf_call_arg_meta *meta, int insn_idx, + enum bpf_arg_type *arg_type, u32 *arg_size) +{ + argno_t argno = argno_from_arg(arg + 1); + const struct btf_param *args; + const struct btf_type *ref_t, *resolve_ret; + const struct btf *btf; + const char *ref_tname; + u32 ref_id; + + if (base_type(*arg_type) == ARG_PTR_TO_MAP_VALUE) + return resolve_map_arg_type(env, meta, arg_type); + + if (base_type(*arg_type) != ARG_PTR_TO_BTF_ID) + return 0; + + if (!meta->btf || arg_type_is_release(*arg_type) || + base_type(reg->type) == PTR_TO_BTF_ID || + reg2btf_ids[base_type(reg->type)]) + return 0; + args = btf_params(meta->func_proto); + ref_id = *meta->fn->arg_btf_id[arg]; + btf = is_kfunc_arg_map(meta->btf, &args[arg]) ? btf_vmlinux : meta->btf; + ref_t = btf_type_skip_modifiers(btf, ref_id, &ref_id); + ref_tname = btf_name_by_offset(btf, ref_t->name_off); + + if (!btf_type_is_scalar_struct(env, btf, ref_t)) { + enum bpf_reg_type reg2btf_type = lookup_reg2btf_ids(ref_id); + const char *expected_type; + + verbose(env, "%s is %s expected %s %s", + reg_arg_name(env, argno), reg_type_str(env, reg->type), + btf_type_str(ref_t), ref_tname); + if (reg2btf_type != NOT_INIT) + verbose(env, " or %s", reg_type_str(env, reg2btf_type)); + verbose(env, "\n"); + expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Pass a verifier-tracked pointer to the expected kernel object type, not a pointer to stack storage or another memory buffer.", + "the function expects a pointer to %s, but this argument is %s and cannot be used as that kernel object pointer", + expected_type, + bpf_diag_reg_type_plain(env, reg->type)); + return -EINVAL; + } + + resolve_ret = btf_resolve_size(btf, ref_t, arg_size); + if (IS_ERR(resolve_ret)) { + verbose(env, "%s reference type('%s %s') size cannot be determined: %ld\n", + reg_arg_name(env, argno), btf_type_str(ref_t), ref_tname, + PTR_ERR(resolve_ret)); + return -EINVAL; + } + *arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE | (*arg_type & PTR_MAYBE_NULL); + + return 0; +} + static void btf_member_path_str(const struct btf *btf, const struct btf_member_path *path, char *buf, size_t buf_sz) { @@ -12990,7 +13053,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me for (i = 0; i < nargs; i++) { struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i); enum bpf_arg_type arg_type = meta->fn->arg_type[i]; - const struct btf_type *t, *ref_t, *resolve_ret; + const struct btf_type *t, *ref_t; argno_t argno = argno_from_arg(i + 1); int regno = reg_from_argno(argno); u32 ref_id = args[i].type; @@ -13053,11 +13116,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (base_type(arg_type) == ARG_PTR_TO_BTF_ID) ref_id = *meta->fn->arg_btf_id[i]; - if (is_kfunc_arg_map(btf, &args[i])) { - ref_t = btf_type_by_id(btf_vmlinux, ref_id); - ref_tname = btf_name_by_offset(btf, ref_t->name_off); - } - + ret = resolve_func_arg_type(env, reg, i, meta, insn_idx, &arg_type, &arg_size); + if (ret < 0) + return ret; ret = check_func_arg_reg_off(env, reg, argno, arg_type); if (ret < 0) return ret; @@ -13306,80 +13367,43 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return ret; break; case ARG_PTR_TO_BTF_ID: - /* Only base_type is checked, further checks are done here */ - if (base_type(reg->type) == PTR_TO_BTF_ID || - reg2btf_ids[base_type(reg->type)]) { - if (!is_trusted_reg(env, reg) || - bpf_type_has_unsafe_modifiers(reg->type)) { - if (!(arg_type & MEM_RCU)) { - const char *expected_type; - - expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); - verbose(env, "%s must be referenced or trusted\n", - reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass a pointer acquired from a verifier-tracked source, or call this kfunc only inside the required protection if it accepts RCU pointers.", - "the kfunc requires a trusted or resource-owning pointer to %s, but %s is %s", - expected_type, - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } - if (!is_rcu_reg(reg)) { - const char *expected_type; - - expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); - verbose(env, "%s must be a rcu pointer\n", - reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Use this kfunc with a pointer that is valid in an RCU read lock region.", - "the kfunc requires an RCU-protected pointer to %s, but %s is %s", - expected_type, - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } - } + if (!is_trusted_reg(env, reg) || + bpf_type_has_unsafe_modifiers(reg->type)) { + if (!(arg_type & MEM_RCU)) { + const char *actual_type, *arg_name, *expected_type; - ret = process_kf_arg_ptr_to_btf_id(env, reg, ref_t, ref_tname, ref_id, meta, i, argno); - if (ret < 0) - return ret; - break; - } + expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); + verbose(env, "%s must be referenced or trusted\n", + reg_arg_name(env, argno)); + arg_name = reg_arg_name(env, argno); + actual_type = bpf_diag_reg_type_plain(env, reg->type); + bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, + "Pass a pointer acquired from a verifier-tracked source, or call this kfunc only inside the required protection if it accepts RCU pointers.", + "the kfunc requires a trusted or resource-owning pointer to %s, but %s is %s", + expected_type, arg_name, actual_type); + return -EINVAL; + } + if (!is_rcu_reg(reg)) { + const char *actual_type, *arg_name, *expected_type; - if (!btf_type_is_scalar_struct(env, meta->btf, ref_t)) { - enum bpf_reg_type reg2btf_type = lookup_reg2btf_ids(ref_id); - const char *expected_type; - - verbose(env, "%s is %s expected %s %s", - reg_arg_name(env, argno), reg_type_str(env, reg->type), - btf_type_str(ref_t), ref_tname); - if (reg2btf_type != NOT_INIT) - verbose(env, " or %s", reg_type_str(env, reg2btf_type)); - verbose(env, "\n"); - expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass a verifier-tracked pointer to the expected kernel object type, not a pointer to stack storage or another memory buffer.", - "the kfunc expects a pointer to %s, but this argument is %s and cannot be used as that kernel object pointer", - expected_type, - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; + expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); + verbose(env, "%s must be a rcu pointer\n", + reg_arg_name(env, argno)); + arg_name = reg_arg_name(env, argno); + actual_type = bpf_diag_reg_type_plain(env, reg->type); + bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, + "Use this kfunc with a pointer that is valid in an RCU read lock region.", + "the kfunc requires an RCU-protected pointer to %s, but %s is %s", + expected_type, arg_name, actual_type); + return -EINVAL; + } } - /* - * If the register does not contain btf id but the argument type is a pointer to - * scalar-only struct, allow verifying it as a fixed size memory. - */ - resolve_ret = btf_resolve_size(btf, ref_t, &arg_size); - if (IS_ERR(resolve_ret)) { - verbose(env, - "%s reference type('%s %s') size cannot be determined: %ld\n", - reg_arg_name(env, argno), btf_type_str(ref_t), - ref_tname, PTR_ERR(resolve_ret)); - return -EINVAL; - } - arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE; - fallthrough; + ret = process_kf_arg_ptr_to_btf_id(env, reg, ref_t, ref_tname, + ref_id, meta, i, argno); + if (ret < 0) + return ret; + break; case ARG_PTR_TO_MEM: if (arg_type & MEM_FIXED_SIZE) { bool known_memory; -- 2.52.0 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 --- kernel/bpf/verifier.c | 110 ++++++++++++------ .../selftests/bpf/prog_tests/cb_refs.c | 2 +- .../selftests/bpf/progs/cpumask_failure.c | 2 +- .../selftests/bpf/progs/iters_testmod.c | 4 +- .../selftests/bpf/progs/map_kptr_fail.c | 6 +- .../selftests/bpf/progs/verifier_ctx.c | 2 +- .../progs/verifier_helper_access_var_len.c | 4 +- .../selftests/bpf/progs/verifier_live_stack.c | 2 +- .../selftests/bpf/progs/verifier_map_in_map.c | 3 +- .../bpf/progs/verifier_map_lookup_refine.c | 2 +- .../bpf/progs/verifier_ref_tracking.c | 4 +- .../selftests/bpf/progs/verifier_sock.c | 4 +- .../testing/selftests/bpf/verifier/map_kptr.c | 2 +- 13 files changed, 94 insertions(+), 53 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 4e7230cb5791..541467f9101f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -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)) { diff --git a/tools/testing/selftests/bpf/prog_tests/cb_refs.c b/tools/testing/selftests/bpf/prog_tests/cb_refs.c index 78566b817fd7..50ea3d72d446 100644 --- a/tools/testing/selftests/bpf/prog_tests/cb_refs.c +++ b/tools/testing/selftests/bpf/prog_tests/cb_refs.c @@ -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} */ }; diff --git a/tools/testing/selftests/bpf/progs/cpumask_failure.c b/tools/testing/selftests/bpf/progs/cpumask_failure.c index 4628feb53d86..6d730535eb95 100644 --- a/tools/testing/selftests/bpf/progs/cpumask_failure.c +++ b/tools/testing/selftests/bpf/progs/cpumask_failure.c @@ -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; diff --git a/tools/testing/selftests/bpf/progs/iters_testmod.c b/tools/testing/selftests/bpf/progs/iters_testmod.c index 76012dbbdb41..5a3ff65e8234 100644 --- a/tools/testing/selftests/bpf/progs/iters_testmod.c +++ b/tools/testing/selftests/bpf/progs/iters_testmod.c @@ -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; diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c index eee35d203b66..0d7365b704b1 100644 --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c @@ -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; diff --git a/tools/testing/selftests/bpf/progs/verifier_ctx.c b/tools/testing/selftests/bpf/progs/verifier_ctx.c index 7856dad3d1f3..9d42ba824408 100644 --- a/tools/testing/selftests/bpf/progs/verifier_ctx.c +++ b/tools/testing/selftests/bpf/progs/verifier_ctx.c @@ -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 (" \ diff --git a/tools/testing/selftests/bpf/progs/verifier_helper_access_var_len.c b/tools/testing/selftests/bpf/progs/verifier_helper_access_var_len.c index 343fc08d9747..d1452ef6f2f9 100644 --- a/tools/testing/selftests/bpf/progs/verifier_helper_access_var_len.c +++ b/tools/testing/selftests/bpf/progs/verifier_helper_access_var_len.c @@ -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 (" \ diff --git a/tools/testing/selftests/bpf/progs/verifier_live_stack.c b/tools/testing/selftests/bpf/progs/verifier_live_stack.c index 401152b2b64f..bc3dfdc1a536 100644 --- a/tools/testing/selftests/bpf/progs/verifier_live_stack.c +++ b/tools/testing/selftests/bpf/progs/verifier_live_stack.c @@ -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 ( diff --git a/tools/testing/selftests/bpf/progs/verifier_map_in_map.c b/tools/testing/selftests/bpf/progs/verifier_map_in_map.c index d3be69a9a755..621248a02a1f 100644 --- a/tools/testing/selftests/bpf/progs/verifier_map_in_map.c +++ b/tools/testing/selftests/bpf/progs/verifier_map_in_map.c @@ -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) { diff --git a/tools/testing/selftests/bpf/progs/verifier_map_lookup_refine.c b/tools/testing/selftests/bpf/progs/verifier_map_lookup_refine.c index c01abf54923d..4b1eadddd89c 100644 --- a/tools/testing/selftests/bpf/progs/verifier_map_lookup_refine.c +++ b/tools/testing/selftests/bpf/progs/verifier_map_lookup_refine.c @@ -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)] = {}; diff --git a/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c b/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c index 199ad18f8eb5..f3fad911b5bc 100644 --- a/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c +++ b/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c @@ -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 ( diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c index 4f2f3209eec8..d59b2f905671 100644 --- a/tools/testing/selftests/bpf/progs/verifier_sock.c +++ b/tools/testing/selftests/bpf/progs/verifier_sock.c @@ -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) { diff --git a/tools/testing/selftests/bpf/verifier/map_kptr.c b/tools/testing/selftests/bpf/verifier/map_kptr.c index 4b39f8472f9b..1efaff296b7c 100644 --- a/tools/testing/selftests/bpf/verifier/map_kptr.c +++ b/tools/testing/selftests/bpf/verifier/map_kptr.c @@ -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", -- 2.52.0 check_reg_type() rejects a trusted PTR_TO_BTF_ID admitted by mem_types unless the argument carries MEM_RDONLY. This duplicates argument-specific memory access validation and is too conservative for map keys and input map values, whose read direction is not represented by MEM_RDONLY. Since commit fa75b7c85b0d ("bpf: Enforce write checks for BTF pointer helper access"), check_helper_mem_access() forwards the requested access type to check_ptr_to_btf_access(). Map keys always request read access. Map values request write access only when tagged with MEM_WRITE. ARG_PTR_TO_MEM prototypes must carry either MEM_RDONLY or MEM_WRITE, and size arguments use the direction of their associated memory argument. check_ptr_to_btf_access() therefore checks the effective access direction in every case. Drop the early rejection and keep check_reg_type() limited to register admission. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 10 +--------- tools/testing/selftests/bpf/progs/task_kfunc_failure.c | 2 +- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 541467f9101f..eb825fc3ce0d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8515,16 +8515,8 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re if (base_type(reg->type) != PTR_TO_BTF_ID) return 0; - if (compatible == &mem_types) { - if (!(arg_type & MEM_RDONLY)) { - verbose(env, - "%s() may write into memory pointed by %s type=%s\n", - meta->func_name, - reg_arg_name(env, argno), reg_type_str(env, reg->type)); - return -EACCES; - } + if (compatible == &mem_types) return 0; - } switch ((int)reg->type) { case PTR_TO_BTF_ID: diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c index f96b0c13ed1a..9979766d4d74 100644 --- a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c @@ -333,7 +333,7 @@ int BPF_PROG(task_access_comm2, struct task_struct *task, u64 clone_flags) } SEC("tp_btf/task_newtask") -__failure __msg("write into memory") +__failure __msg("only read is supported") int BPF_PROG(task_access_comm3, struct task_struct *task, u64 clone_flags) { bpf_probe_read_kernel(task->comm, 16, task->comm); -- 2.52.0 Keep check_reg_type() focused on register admission. Helpers currently match BTF-ID arguments there, while kfuncs use process_kf_arg_ptr_to_btf_id(). Both paths ultimately call btf_struct_ids_match(). Introduce process_arg_ptr_to_btf_id() for helpers, kfuncs, and global subprograms. Callers provide the expected BTF and ID and retain their call-specific metadata handling. Group the helper compatible-ID, poison, and bpf_kptr_xchg() handling in a helper-only block in the ARG_PTR_TO_BTF_ID case, leaving the common matcher outside it. Derive strict matching from the generated argument type. This limits KF_RELEASE strictness to the argument marked OBJ_RELEASE while preserving the bpf_sk_release() exception and kfunc no-cast-alias rule. Remove the post-admission BTF and nullability switch from check_reg_type(), leaving it responsible for register admission. The compatibility tables already limit helper MEM_ALLOC inputs to ARG_PTR_TO_SPIN_LOCK and ARG_KPTR_XCHG_DEST, while the kptr source is admitted only for bpf_kptr_xchg(). Drop the redundant helper-ID whitelist, pointer-offset check, and constant-offset assertion. The bpf_kptr_xchg() source match now follows offset validation, so a source within a referenced object reports the release zero-offset error before the kptr type error. Update the affected selftests and use call-neutral wording for BTF mismatch diagnostics. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 215 +++++++----------- .../testing/selftests/bpf/prog_tests/bpf_nf.c | 14 +- .../selftests/bpf/progs/map_kptr_fail.c | 2 +- .../selftests/bpf/progs/verifier_vfs_reject.c | 2 +- tools/testing/selftests/bpf/verifier/calls.c | 2 +- .../testing/selftests/bpf/verifier/map_kptr.c | 2 +- 6 files changed, 94 insertions(+), 143 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index eb825fc3ce0d..11d28c8d9c79 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8248,6 +8248,10 @@ static int resolve_func_arg_type(struct bpf_verifier_env *env, struct bpf_reg_state *reg, u32 arg, struct bpf_call_arg_meta *meta, int insn_idx, enum bpf_arg_type *arg_type, u32 *arg_size); +static int process_arg_ptr_to_btf_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + argno_t argno, enum bpf_arg_type arg_type, + const struct btf *arg_btf, u32 arg_btf_id, + struct bpf_call_arg_meta *meta, int insn_idx); struct bpf_reg_types { const enum bpf_reg_type types[10]; @@ -8450,13 +8454,12 @@ static const char *bpf_diag_expected_reg_types(struct bpf_verifier_env *env, } static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, - enum bpf_arg_type arg_type, const u32 *arg_btf_id, - struct bpf_call_arg_meta *meta) + enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta) { enum bpf_reg_type expected, type = reg->type; const struct bpf_reg_types *compatible; const char *actual, *accepted; - int i, j, err; + int i, j; compatible = compatible_reg_types[base_type(arg_type)]; if (!compatible) { @@ -8512,90 +8515,6 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re return -EACCES; found: - if (base_type(reg->type) != PTR_TO_BTF_ID) - return 0; - - if (compatible == &mem_types) - return 0; - - switch ((int)reg->type) { - case PTR_TO_BTF_ID: - case PTR_TO_BTF_ID | PTR_TRUSTED: - case PTR_TO_BTF_ID | PTR_TRUSTED | PTR_MAYBE_NULL: - case PTR_TO_BTF_ID | MEM_RCU: - case PTR_TO_BTF_ID | PTR_MAYBE_NULL: - case PTR_TO_BTF_ID | PTR_MAYBE_NULL | MEM_RCU: - { - /* For bpf_sk_release, it needs to match against first member - * 'struct sock_common', hence make an exception for it. This - * allows bpf_sk_release to work for multiple socket types. - */ - bool strict_type_match = arg_type_is_release(arg_type) && - !is_helper_call(meta, BPF_FUNC_sk_release); - - if (!arg_btf_id) { - if (!compatible->btf_id) { - verifier_bug(env, "missing arg compatible BTF ID"); - return -EFAULT; - } - arg_btf_id = compatible->btf_id; - } - - if (is_helper_call(meta, BPF_FUNC_kptr_xchg)) { - if (map_kptr_match_type(env, meta->kptr_field, reg, reg_from_argno(argno))) - return -EACCES; - } else { - if (arg_btf_id == BPF_PTR_POISON) { - verbose(env, "verifier internal error:"); - verbose(env, "%s has non-overwritten BPF_PTR_POISON type\n", - reg_arg_name(env, argno)); - return -EACCES; - } - - err = __check_ptr_off_reg(env, reg, argno, true); - if (err) - return err; - - if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, - reg->var_off.value, btf_vmlinux, *arg_btf_id, - strict_type_match, !type_is_alloc(reg->type))) { - verbose(env, "%s is of type %s but %s is expected\n", - reg_arg_name(env, argno), - btf_type_name(reg->btf, reg->btf_id), - btf_type_name(btf_vmlinux, *arg_btf_id)); - return -EACCES; - } - } - break; - } - case PTR_TO_BTF_ID | MEM_ALLOC: - case PTR_TO_BTF_ID | MEM_PERCPU | MEM_ALLOC: - case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF: - case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU: - if (!is_helper_call(meta, BPF_FUNC_spin_lock) && - !is_helper_call(meta, BPF_FUNC_spin_unlock) && - !is_helper_call(meta, BPF_FUNC_kptr_xchg)) { - verifier_bug(env, "unimplemented handling of MEM_ALLOC"); - return -EFAULT; - } - /* Check if local kptr in src arg matches kptr in dst arg */ - if (is_helper_call(meta, BPF_FUNC_kptr_xchg)) { - int regno = reg_from_argno(argno); - - if (regno == BPF_REG_2 && - map_kptr_match_type(env, meta->kptr_field, reg, regno)) - return -EACCES; - } - break; - case PTR_TO_BTF_ID | MEM_PERCPU: - case PTR_TO_BTF_ID | MEM_PERCPU | MEM_RCU: - case PTR_TO_BTF_ID | MEM_PERCPU | PTR_TRUSTED: - /* Handled by helper specific checks */ - break; - default: - verifier_bug(env, "invalid PTR_TO_BTF_ID register for type match"); - return -EFAULT; - } return 0; } @@ -8872,7 +8791,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, enum bpf_arg_type arg_type = fn->arg_type[arg]; int regno = reg_from_argno(argno); enum bpf_reg_type type = reg->type; - const u32 *arg_btf_id = NULL; u32 arg_size = arg_type & MEM_FIXED_SIZE ? fn->arg_size[arg] : 0; u32 key_size; int err = 0; @@ -8924,12 +8842,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 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) - arg_btf_id = fn->arg_btf_id[arg]; - - err = check_reg_type(env, reg, argno, arg_type, arg_btf_id, meta); + err = check_reg_type(env, reg, argno, arg_type, meta); if (err) return err; @@ -9027,6 +8940,43 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ, false, meta, NULL); break; + case ARG_PTR_TO_BTF_ID: + case ARG_PTR_TO_BTF_ID_SOCK_COMMON: + { + const u32 *arg_btf_id = fn->arg_btf_id[arg]; + + if (!meta->btf) { + const struct bpf_reg_types *compatible; + + if (base_type(reg->type) != PTR_TO_BTF_ID) + break; + + if (is_helper_call(meta, BPF_FUNC_kptr_xchg)) + return map_kptr_match_type(env, meta->kptr_field, reg, regno) ? + -EACCES : 0; + + if (!arg_btf_id) { + compatible = compatible_reg_types[base_type(arg_type)]; + if (!compatible->btf_id) { + verifier_bug(env, "missing arg compatible BTF ID"); + return -EFAULT; + } + arg_btf_id = compatible->btf_id; + } + if (arg_btf_id == BPF_PTR_POISON) { + verbose(env, "verifier internal error:"); + verbose(env, "%s has non-overwritten BPF_PTR_POISON type\n", + reg_arg_name(env, argno)); + return -EACCES; + } + } + + err = process_arg_ptr_to_btf_id(env, reg, argno, arg_type, btf_vmlinux, + *arg_btf_id, meta, insn_idx); + if (err < 0) + return err; + break; + } case ARG_PTR_TO_PERCPU_BTF_ID: if (!reg->btf_id) { verbose(env, "Helper has invalid btf_id in %s\n", reg_arg_name(env, argno)); @@ -9973,8 +9923,12 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, continue; } - err = check_reg_type(env, reg, argno, arg->arg_type, &arg->btf_id, &meta); + err = check_reg_type(env, reg, argno, arg->arg_type, &meta); err = err ?: check_func_arg_reg_off(env, reg, argno, arg->arg_type); + if (!err && base_type(reg->type) == PTR_TO_BTF_ID) + err = process_arg_ptr_to_btf_id(env, reg, argno, arg->arg_type, + btf_vmlinux, arg->btf_id, + &meta, env->insn_idx); if (err) return err; } else { @@ -12511,32 +12465,28 @@ static int gen_kfunc_arg_proto(struct bpf_verifier_env *env, struct bpf_call_arg return check_arg_prog_aux(env, proto) ? 0 : -EINVAL; } -static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env, - struct bpf_reg_state *reg, - const struct btf_type *ref_t, - const char *ref_tname, u32 ref_id, - struct bpf_call_arg_meta *meta, - int arg, argno_t argno) +static int process_arg_ptr_to_btf_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + argno_t argno, enum bpf_arg_type arg_type, + const struct btf *arg_btf, u32 arg_btf_id, + struct bpf_call_arg_meta *meta, int insn_idx) { - const struct btf_type *reg_ref_t; - bool strict_type_match = false; + bool taking_projection, struct_same, strict_type_match = false; + const struct btf_type *arg_t, *reg_t; + const char *arg_tname, *reg_tname; const struct btf *reg_btf; - const char *reg_ref_tname; - bool taking_projection; - bool struct_same; - u32 reg_ref_id; + u32 reg_btf_id; if (base_type(reg->type) == PTR_TO_BTF_ID) { reg_btf = reg->btf; - reg_ref_id = reg->btf_id; + reg_btf_id = reg->btf_id; } else { reg_btf = btf_vmlinux; - reg_ref_id = *reg2btf_ids[base_type(reg->type)]; + reg_btf_id = *reg2btf_ids[base_type(reg->type)]; } - /* Enforce strict type matching for calls to kfuncs that are acquiring - * or releasing a reference, or are no-cast aliases. We do _not_ - * enforce strict matching for kfuncs by default, + /* + * Enforce strict type matching for arguments that release a reference, + * or are no-cast aliases. We do _not_ enforce strict matching by default, * as we want to enable BPF programs to pass types that are bitwise * equivalent without forcing them to explicitly cast with something * like bpf_cast_to_kern_ctx(). @@ -12558,27 +12508,30 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env, * btf_struct_ids_match() to walk the struct at the 0th offset, and * resolve types. */ - if ((is_kfunc_release(meta) && reg_is_referenced(env, reg)) || - btf_type_ids_nocast_alias(&env->log, reg_btf, reg_ref_id, meta->btf, ref_id)) + if ((arg_type_is_release(arg_type) && !is_helper_call(meta, BPF_FUNC_sk_release)) || + (meta->btf && btf_type_ids_nocast_alias(&env->log, reg_btf, reg_btf_id, + arg_btf, arg_btf_id))) strict_type_match = true; - WARN_ON_ONCE(is_kfunc_release(meta) && !tnum_is_const(reg->var_off)); + arg_t = btf_type_skip_modifiers(arg_btf, arg_btf_id, &arg_btf_id); + arg_tname = btf_name_by_offset(arg_btf, arg_t->name_off); + reg_t = btf_type_skip_modifiers(reg_btf, reg_btf_id, ®_btf_id); + reg_tname = btf_name_by_offset(reg_btf, reg_t->name_off); + + struct_same = btf_struct_ids_match(&env->log, reg_btf, reg_btf_id, + reg->var_off.value, arg_btf, arg_btf_id, + strict_type_match, !type_is_alloc(reg->type)); - reg_ref_t = btf_type_skip_modifiers(reg_btf, reg_ref_id, ®_ref_id); - reg_ref_tname = btf_name_by_offset(reg_btf, reg_ref_t->name_off); - struct_same = btf_struct_ids_match(&env->log, reg_btf, reg_ref_id, reg->var_off.value, - meta->btf, ref_id, strict_type_match, - !type_is_alloc(reg->type)); /* If kfunc is accepting a projection type (ie. __sk_buff), it cannot * actually use it -- it must cast to the underlying type. So we allow * caller to pass in the underlying type. */ - taking_projection = btf_is_projection_of(ref_tname, reg_ref_tname); + taking_projection = meta->btf && btf_is_projection_of(arg_tname, reg_tname); if (!taking_projection && !struct_same) { - verbose(env, "kernel function %s %s expected pointer to %s %s but %s has a pointer to %s %s\n", + verbose(env, "%s %s expected pointer to %s %s but %s has a pointer to %s %s\n", meta->func_name, reg_arg_name(env, argno), - btf_type_str(ref_t), ref_tname, reg_arg_name(env, argno), - btf_type_str(reg_ref_t), reg_ref_tname); + btf_type_str(arg_t), arg_tname, + reg_arg_name(env, argno), btf_type_str(reg_t), reg_tname); return -EINVAL; } return 0; @@ -13076,7 +13029,7 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env) static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, int insn_idx) { - const char *func_name = meta->func_name, *ref_tname; + const char *func_name = meta->func_name; struct bpf_func_state *caller = cur_func(env); struct bpf_reg_state *regs = cur_regs(env); const struct btf *btf = meta->btf; @@ -13098,7 +13051,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me for (i = 0; i < nargs; i++) { struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i); enum bpf_arg_type arg_type = meta->fn->arg_type[i]; - const struct btf_type *t, *ref_t; + const struct btf_type *t; argno_t argno = argno_from_arg(i + 1); int regno = reg_from_argno(argno); u32 ref_id = args[i].type; @@ -13114,10 +13067,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me t = btf_type_skip_modifiers(btf, args[i].type, NULL); - if (btf_type_is_ptr(t)) { - ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); - ref_tname = btf_name_by_offset(btf, ref_t->name_off); - } + if (btf_type_is_ptr(t)) + btf_type_skip_modifiers(btf, t->type, &ref_id); ret = check_func_arg_nullability(env, reg, argno, arg_type, meta, insn_idx); if (ret < 0) @@ -13433,8 +13384,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me } } - ret = process_kf_arg_ptr_to_btf_id(env, reg, ref_t, ref_tname, - ref_id, meta, i, argno); + ret = process_arg_ptr_to_btf_id(env, reg, argno, arg_type, btf, + ref_id, meta, insn_idx); if (ret < 0) return ret; break; diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c index 14d4c1793aed..d74a9db54c9a 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c @@ -13,13 +13,13 @@ struct { const char *prog_name; const char *err_msg; } test_bpf_nf_fail_tests[] = { - { "alloc_release", "kernel function bpf_ct_release R1 expected pointer to STRUCT nf_conn but" }, - { "insert_insert", "kernel function bpf_ct_insert_entry R1 expected pointer to STRUCT nf_conn___init but" }, - { "lookup_insert", "kernel function bpf_ct_insert_entry R1 expected pointer to STRUCT nf_conn___init but" }, - { "set_timeout_after_insert", "kernel function bpf_ct_set_timeout R1 expected pointer to STRUCT nf_conn___init but" }, - { "set_status_after_insert", "kernel function bpf_ct_set_status R1 expected pointer to STRUCT nf_conn___init but" }, - { "change_timeout_after_alloc", "kernel function bpf_ct_change_timeout R1 expected pointer to STRUCT nf_conn but" }, - { "change_status_after_alloc", "kernel function bpf_ct_change_status R1 expected pointer to STRUCT nf_conn but" }, + { "alloc_release", "bpf_ct_release R1 expected pointer to STRUCT nf_conn but" }, + { "insert_insert", "bpf_ct_insert_entry R1 expected pointer to STRUCT nf_conn___init but" }, + { "lookup_insert", "bpf_ct_insert_entry R1 expected pointer to STRUCT nf_conn___init but" }, + { "set_timeout_after_insert", "bpf_ct_set_timeout R1 expected pointer to STRUCT nf_conn___init but" }, + { "set_status_after_insert", "bpf_ct_set_status R1 expected pointer to STRUCT nf_conn___init but" }, + { "change_timeout_after_alloc", "bpf_ct_change_timeout R1 expected pointer to STRUCT nf_conn but" }, + { "change_status_after_alloc", "bpf_ct_change_status R1 expected pointer to STRUCT nf_conn but" }, { "write_not_allowlisted_field", "no write support to nf_conn at off" }, { "lookup_null_bpf_tuple", "Possibly NULL pointer passed to trusted R2" }, { "lookup_null_bpf_opts", "Possibly NULL pointer passed to trusted R4" }, diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c index 0d7365b704b1..186b56357110 100644 --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c @@ -291,7 +291,7 @@ int reject_bad_type_xchg(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("invalid kptr access, R2 type=trusted_ptr_prog_test_ref_kfunc") +__failure __msg("R2 must have zero offset when passed to release func") int reject_member_of_ref_xchg(struct __sk_buff *ctx) { struct prog_test_ref_kfunc *ref_ptr; diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c index 8f0c45421f89..ff08aa75d6f7 100644 --- a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c +++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c @@ -128,7 +128,7 @@ int BPF_PROG(path_d_path_kfunc_untrusted_from_current) } SEC("lsm.s/file_open") -__failure __msg("kernel function bpf_path_d_path R1 expected pointer to STRUCT path but R1 has a pointer to STRUCT file") +__failure __msg("bpf_path_d_path R1 expected pointer to STRUCT path but R1 has a pointer to STRUCT file") int BPF_PROG(path_d_path_kfunc_type_mismatch, struct file *file) { bpf_path_d_path((struct path *)&file->f_task_work, buf, sizeof(buf)); diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c index eb6e3baef412..d730215e520b 100644 --- a/tools/testing/selftests/bpf/verifier/calls.c +++ b/tools/testing/selftests/bpf/verifier/calls.c @@ -152,7 +152,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "kernel function bpf_kfunc_call_memb1_release R1 expected pointer", + .errstr = "bpf_kfunc_call_memb1_release R1 expected pointer", .fixup_kfunc_btf_id = { { "bpf_kfunc_call_memb_acquire", 1 }, { "bpf_kfunc_call_memb1_release", 5 }, diff --git a/tools/testing/selftests/bpf/verifier/map_kptr.c b/tools/testing/selftests/bpf/verifier/map_kptr.c index 1efaff296b7c..345cecc722a3 100644 --- a/tools/testing/selftests/bpf/verifier/map_kptr.c +++ b/tools/testing/selftests/bpf/verifier/map_kptr.c @@ -342,7 +342,7 @@ .prog_type = BPF_PROG_TYPE_SCHED_CLS, .fixup_map_kptr = { 1 }, .result = REJECT, - .errstr = "invalid kptr access, R2 type=ptr_prog_test_ref_kfunc expected=ptr_prog_test_member", + .errstr = "R2 must have zero offset when passed to release func", }, { "map_kptr: ref: reference state created and released on xchg", -- 2.52.0 check_kfunc_args() open-codes exact register-type tests in most of its per-argument cases, duplicating what compatible_reg_types[] already expresses for helpers. This leaves two admission paths and prevents the helper and kfunc loops from converging. Runtime argument resolution now converts a scalar-struct BTF argument to fixed-size memory before register admission. Give the remaining kfunc-only argument kinds compatibility entries and run check_reg_type() once before the per-kind switch. Kfunc memory arguments already accept BPF-allocated objects. Normalize only the local comparison type to PTR_TO_MEM; subsequent memory checks still inspect the original register type. Keep allocated-object forms out of mem_types so helper calls continue through the existing type-mismatch path and retain its diagnostic. For ARG_PTR_TO_BTF_ID, let check_reg_type() admit BTF-backed register types and reject incompatible register classes with its standard diagnostic. Remove the now-unused lookup_reg2btf_ids(). Exact BTF identity and trust requirements remain checked later by process_arg_ptr_to_btf_id(). ARG_IGNORE and ARG_PTR_TO_PROG_AUX remain skipped because the verifier does not read those arguments from the program. Iterator arguments use the stack-pointer table. Graph nodes and ARG_PTR_TO_REFCOUNTED_KPTR share an allocated-object table. It admits owning and borrowed objects, including RCU-protected forms. Their switch cases retain API-specific ownership and BTF-record validation. ARG_PTR_TO_ALLOC_BTF_ID uses a separate table for object-drop arguments. Rename timer_types to map_value_types now that ARG_PTR_TO_WORKQUEUE and ARG_PTR_TO_TASK_WORK share it. Similarly, rename spin_lock_types to map_value_or_alloc_obj_types because graph roots and resource spin locks share its map-value-or-allocated-object admission. Moving admission checks into check_reg_type() must not discard the structured call-argument diagnostics emitted by the individual cases. Add bpf_diag_arg_type_plain() alongside bpf_diag_reg_type_plain() and use it to preserve the existing per-kind Pass suggestions where available. Other argument kinds retain the generic suggestion. The reason continues to report the actual register type and all accepted register types. Two behavior changes fall out of running admission first: - ARG_CONST_MEM_SIZE reaches process_const_arg(), and through it mark_chain_precision(), only after the register is known to be a scalar. Passing a pointer as a __szk argument used to reach backtrack_insn() with a non-scalar and trip the backtracking-misuse verifier bug. - ARG_CONST_MAP_PTR no longer needs its own type_may_be_null() test, because check_reg_type() compares whole register types. Every kfunc argument that is not explicitly ignored now passes through check_reg_type(), followed by the common register-offset check in the same order as a helper argument. Signed-off-by: Amery Hung --- kernel/bpf/diagnostics.c | 29 +++ kernel/bpf/diagnostics.h | 1 + kernel/bpf/verifier.c | 238 +++++------------- .../selftests/bpf/prog_tests/kfunc_call.c | 2 +- .../testing/selftests/bpf/progs/arena_kfunc.c | 2 +- .../selftests/bpf/progs/cgrp_kfunc_failure.c | 2 +- .../selftests/bpf/progs/cpumask_failure.c | 2 +- tools/testing/selftests/bpf/progs/irq.c | 4 +- tools/testing/selftests/bpf/progs/iters.c | 6 +- .../selftests/bpf/progs/iters_testmod.c | 3 +- .../bpf/progs/mem_rdonly_untrusted.c | 3 +- .../testing/selftests/bpf/progs/rbtree_fail.c | 4 +- .../bpf/progs/refcounted_kptr_fail.c | 9 +- .../selftests/bpf/progs/res_spin_lock_fail.c | 2 +- .../testing/selftests/bpf/progs/stream_fail.c | 2 +- .../selftests/bpf/progs/task_kfunc_failure.c | 2 +- .../selftests/bpf/progs/task_work_fail.c | 2 +- .../bpf/progs/test_kfunc_dynptr_param.c | 2 +- .../selftests/bpf/progs/verifier_vfs_reject.c | 2 +- .../testing/selftests/bpf/progs/wq_failures.c | 4 +- tools/testing/selftests/bpf/verifier/calls.c | 8 +- 21 files changed, 120 insertions(+), 209 deletions(-) diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c index 0abbbe177e31..a2cac59c6639 100644 --- a/kernel/bpf/diagnostics.c +++ b/kernel/bpf/diagnostics.c @@ -960,6 +960,35 @@ const char *bpf_diag_reg_type_plain(struct bpf_verifier_env *env, enum bpf_reg_t } } +const char *bpf_diag_arg_type_plain(enum bpf_arg_type type) +{ + switch (base_type(type)) { + case ARG_MEM_SIZE: + case ARG_CONST_MEM_SIZE: + return "an integer scalar length for this memory argument"; + case ARG_PTR_TO_CTX: + return "the original program context pointer or preserve it before modifying registers"; + case ARG_SCALAR: + case ARG_CONST_SCALAR: + case ARG_CONST_ALLOC_SIZE_OR_ZERO: + return "an integer scalar value for this argument, not a pointer or resource object"; + case ARG_PTR_TO_CONST_STR: + return "a constant string pointer that the verifier recognizes, such as a string stored in a read-only map value"; + case ARG_PTR_TO_DYNPTR: + return "the address of a stack dynptr object, or use a const dynptr pointer returned by the verifier-supported path"; + case ARG_PTR_TO_ALLOC_BTF_ID: + return "a pointer returned by the matching BPF object allocation path"; + case ARG_PTR_TO_REFCOUNTED_KPTR: + return "an owning or non-owning pointer to a BPF-managed object containing a bpf_refcount field"; + case ARG_PTR_TO_ITER: + return "the address of a stack iterator object for iterator new, next, and destroy calls"; + case ARG_PTR_TO_IRQ_FLAG: + return "the same stack slot used by bpf_local_irq_save() or bpf_res_spin_lock_irqsave()"; + default: + return "a value with one of the accepted pointer or scalar types for this call"; + } +} + static const char *diag_arg_ordinal(int argno) { switch (argno) { diff --git a/kernel/bpf/diagnostics.h b/kernel/bpf/diagnostics.h index d1b79945008a..a4102fb049ec 100644 --- a/kernel/bpf/diagnostics.h +++ b/kernel/bpf/diagnostics.h @@ -51,6 +51,7 @@ const char *bpf_diag_vfmt(struct bpf_verifier_env *env, const char *fmt, va_list const char *bpf_diag_fmt(struct bpf_verifier_env *env, const char *fmt, ...) __printf(2, 3); const char *bpf_diag_fmt_btf_type(struct bpf_verifier_env *env, const struct btf *btf, u32 type_id); const char *bpf_diag_reg_type_plain(struct bpf_verifier_env *env, enum bpf_reg_type type); +const char *bpf_diag_arg_type_plain(enum bpf_arg_type type); u64 bpf_diag_event_log_save(struct bpf_verifier_env *env); void bpf_diag_event_log_restore(struct bpf_verifier_env *env, u64 log_pos); u32 bpf_diag_irq_depth(const struct bpf_verifier_state *state); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 11d28c8d9c79..254458cc1737 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5166,18 +5166,6 @@ static u32 *reg2btf_ids[__BPF_REG_TYPE_MAX] = { [CONST_PTR_TO_MAP] = btf_bpf_map_id, }; -static enum bpf_reg_type lookup_reg2btf_ids(u32 ref_id) -{ - enum bpf_reg_type type; - - for (type = 0; type < __BPF_REG_TYPE_MAX; type++) { - if (reg2btf_ids[type] && *reg2btf_ids[type] == ref_id) - return type; - } - - return NOT_INIT; -} - static bool is_trusted_reg(struct bpf_verifier_env *env, const struct bpf_reg_state *reg) { /* A referenced register is always trusted. */ @@ -8246,7 +8234,7 @@ static int resolve_map_arg_type(struct bpf_verifier_env *env, static int resolve_func_arg_type(struct bpf_verifier_env *env, struct bpf_reg_state *reg, u32 arg, - struct bpf_call_arg_meta *meta, int insn_idx, + struct bpf_call_arg_meta *meta, enum bpf_arg_type *arg_type, u32 *arg_size); static int process_arg_ptr_to_btf_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, enum bpf_arg_type arg_type, @@ -8296,7 +8284,7 @@ static const struct bpf_reg_types mem_types = { }, }; -static const struct bpf_reg_types spin_lock_types = { +static const struct bpf_reg_types map_value_or_alloc_obj_types = { .types = { PTR_TO_MAP_VALUE, PTR_TO_BTF_ID | MEM_ALLOC, @@ -8325,7 +8313,30 @@ static const struct bpf_reg_types percpu_btf_ptr_types = { static const struct bpf_reg_types func_ptr_types = { .types = { PTR_TO_FUNC } }; static const struct bpf_reg_types stack_ptr_types = { .types = { PTR_TO_STACK } }; static const struct bpf_reg_types const_str_ptr_types = { .types = { PTR_TO_MAP_VALUE } }; -static const struct bpf_reg_types timer_types = { .types = { PTR_TO_MAP_VALUE } }; +static const struct bpf_reg_types map_value_types = { .types = { PTR_TO_MAP_VALUE } }; +static const struct bpf_reg_types arena_types = { + .types = { + PTR_TO_ARENA, + SCALAR_VALUE, + } +}; + +static const struct bpf_reg_types alloc_obj_drop_types = { + .types = { + PTR_TO_BTF_ID | MEM_ALLOC, + PTR_TO_BTF_ID | MEM_ALLOC | MEM_PERCPU, + } +}; + +static const struct bpf_reg_types alloc_obj_types = { + .types = { + PTR_TO_BTF_ID | MEM_ALLOC, + PTR_TO_BTF_ID | MEM_ALLOC | MEM_RCU, + PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF, + PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU, + } +}; + static const struct bpf_reg_types kptr_xchg_dest_types = { .types = { PTR_TO_MAP_VALUE, @@ -8356,16 +8367,30 @@ static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = { #endif [ARG_PTR_TO_SOCKET] = &fullsock_types, [ARG_PTR_TO_BTF_ID] = &btf_ptr_types, - [ARG_PTR_TO_SPIN_LOCK] = &spin_lock_types, + [ARG_PTR_TO_SPIN_LOCK] = &map_value_or_alloc_obj_types, [ARG_PTR_TO_MEM] = &mem_types, [ARG_PTR_TO_RINGBUF_MEM] = &ringbuf_mem_types, [ARG_PTR_TO_PERCPU_BTF_ID] = &percpu_btf_ptr_types, [ARG_PTR_TO_FUNC] = &func_ptr_types, [ARG_PTR_TO_STACK] = &stack_ptr_types, [ARG_PTR_TO_CONST_STR] = &const_str_ptr_types, - [ARG_PTR_TO_TIMER] = &timer_types, + [ARG_PTR_TO_TIMER] = &map_value_types, [ARG_KPTR_XCHG_DEST] = &kptr_xchg_dest_types, [ARG_PTR_TO_DYNPTR] = &dynptr_types, + [ARG_CONST_SCALAR] = &scalar_types, + [ARG_CONST_MEM_SIZE] = &scalar_types, + [ARG_PTR_TO_ALLOC_BTF_ID] = &alloc_obj_drop_types, + [ARG_PTR_TO_REFCOUNTED_KPTR] = &alloc_obj_types, + [ARG_PTR_TO_ITER] = &stack_ptr_types, + [ARG_PTR_TO_LIST_HEAD] = &map_value_or_alloc_obj_types, + [ARG_PTR_TO_LIST_NODE] = &alloc_obj_types, + [ARG_PTR_TO_RB_ROOT] = &map_value_or_alloc_obj_types, + [ARG_PTR_TO_RB_NODE] = &alloc_obj_types, + [ARG_PTR_TO_RES_SPIN_LOCK] = &map_value_or_alloc_obj_types, + [ARG_PTR_TO_WORKQUEUE] = &map_value_types, + [ARG_PTR_TO_TASK_WORK] = &map_value_types, + [ARG_PTR_TO_IRQ_FLAG] = &stack_ptr_types, + [ARG_PTR_TO_ARENA] = &arena_types, }; static void bpf_diag_call_arg(struct bpf_verifier_env *env, u32 insn_idx, argno_t argno, @@ -8466,6 +8491,9 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re verifier_bug(env, "unsupported arg type %d", arg_type); return -EFAULT; } + if (meta->btf && base_type(arg_type) == ARG_PTR_TO_BTF_ID && + (base_type(type) == PTR_TO_BTF_ID || reg2btf_ids[base_type(type)])) + goto found; /* ARG_PTR_TO_MEM + RDONLY is compatible with PTR_TO_MEM and PTR_TO_MEM + RDONLY, * but ARG_PTR_TO_MEM is compatible only with PTR_TO_MEM and NOT with PTR_TO_MEM + RDONLY @@ -8485,6 +8513,10 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re type &= ~PTR_MAYBE_NULL; if (base_type(arg_type) == ARG_PTR_TO_MEM) type &= ~DYNPTR_TYPE_FLAG_MASK; + /* Allow allocated memory for kfunc ARG_PTR_TO_MEM but not helper. */ + if (meta->btf && base_type(arg_type) == ARG_PTR_TO_MEM && + type_is_ptr_alloc_obj(type)) + type = PTR_TO_MEM; /* Local kptr types are allowed as the source argument of bpf_kptr_xchg */ if (is_helper_call(meta, BPF_FUNC_kptr_xchg) && type_is_alloc(type) && @@ -8509,7 +8541,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re actual = bpf_diag_fmt(env, "%s", reg_type_str(env, reg->type)); accepted = bpf_diag_expected_reg_types(env, compatible->types, i); bpf_diag_call_arg_fmt(env, env->insn_idx, argno, meta->func_name, - "Pass a value with one of the accepted pointer or scalar types for this call.", + bpf_diag_fmt(env, "Pass %s.", bpf_diag_arg_type_plain(arg_type)), "it has type %s, but this argument accepts %s", actual, accepted); return -EACCES; @@ -8824,7 +8856,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return -EACCES; } - err = resolve_func_arg_type(env, reg, arg, meta, insn_idx, &arg_type, &arg_size); + err = resolve_func_arg_type(env, reg, arg, meta, &arg_type, &arg_size); if (err) return err; @@ -11916,7 +11948,7 @@ static bool btf_type_is_scalar_struct(struct bpf_verifier_env *env, static int resolve_func_arg_type(struct bpf_verifier_env *env, struct bpf_reg_state *reg, u32 arg, - struct bpf_call_arg_meta *meta, int insn_idx, + struct bpf_call_arg_meta *meta, enum bpf_arg_type *arg_type, u32 *arg_size) { argno_t argno = argno_from_arg(arg + 1); @@ -11936,30 +11968,15 @@ static int resolve_func_arg_type(struct bpf_verifier_env *env, base_type(reg->type) == PTR_TO_BTF_ID || reg2btf_ids[base_type(reg->type)]) return 0; + args = btf_params(meta->func_proto); ref_id = *meta->fn->arg_btf_id[arg]; btf = is_kfunc_arg_map(meta->btf, &args[arg]) ? btf_vmlinux : meta->btf; ref_t = btf_type_skip_modifiers(btf, ref_id, &ref_id); ref_tname = btf_name_by_offset(btf, ref_t->name_off); - if (!btf_type_is_scalar_struct(env, btf, ref_t)) { - enum bpf_reg_type reg2btf_type = lookup_reg2btf_ids(ref_id); - const char *expected_type; - - verbose(env, "%s is %s expected %s %s", - reg_arg_name(env, argno), reg_type_str(env, reg->type), - btf_type_str(ref_t), ref_tname); - if (reg2btf_type != NOT_INIT) - verbose(env, " or %s", reg_type_str(env, reg2btf_type)); - verbose(env, "\n"); - expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); - bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, - "Pass a verifier-tracked pointer to the expected kernel object type, not a pointer to stack storage or another memory buffer.", - "the function expects a pointer to %s, but this argument is %s and cannot be used as that kernel object pointer", - expected_type, - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } + if (!btf_type_is_scalar_struct(env, btf, ref_t)) + return 0; resolve_ret = btf_resolve_size(btf, ref_t, arg_size); if (IS_ERR(resolve_ret)) { @@ -13101,25 +13118,20 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me if (base_type(arg_type) == ARG_PTR_TO_BTF_ID) ref_id = *meta->fn->arg_btf_id[i]; - ret = resolve_func_arg_type(env, reg, i, meta, insn_idx, &arg_type, &arg_size); + ret = resolve_func_arg_type(env, reg, i, meta, &arg_type, &arg_size); + if (ret < 0) + return ret; + + ret = check_reg_type(env, reg, argno, arg_type, meta); if (ret < 0) return ret; + ret = check_func_arg_reg_off(env, reg, argno, arg_type); if (ret < 0) return ret; switch (base_type(arg_type)) { case ARG_CONST_SCALAR: - if (reg->type != SCALAR_VALUE) { - verbose(env, "%s is not a scalar\n", reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass an integer scalar value for this argument, not a pointer or resource object.", - "the kfunc expects an integer scalar, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } - ret = process_const_arg(env, reg, argno, meta); if (ret < 0) { if (ret == -EINVAL) @@ -13131,27 +13143,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me } break; case ARG_SCALAR: - if (reg->type != SCALAR_VALUE) { - verbose(env, "%s is not a scalar\n", reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass an integer scalar value for this argument, not a pointer or resource object.", - "the kfunc expects an integer scalar, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } break; case ARG_CONST_ALLOC_SIZE_OR_ZERO: - if (reg->type != SCALAR_VALUE) { - verbose(env, "%s is not a scalar\n", reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass an integer scalar value for this argument, not a pointer or resource object.", - "the kfunc expects an integer scalar, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } - if (is_kfunc_arg_scalar_with_name(btf, &args[i], "rdonly_buf_size")) meta->r0_rdonly = true; ret = process_const_alloc_mem_size(env, reg, argno, &meta->ret_mem); @@ -13165,17 +13158,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me } break; case ARG_PTR_TO_CTX: - if (reg->type != PTR_TO_CTX) { - verbose(env, "%s expected pointer to ctx, but got %s\n", - reg_arg_name(env, argno), reg_type_str(env, reg->type)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass the original program context pointer or preserve it before modifying registers.", - "the kfunc expects a context pointer, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } - if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_cast_to_kern_ctx])) { ret = get_kern_ctx_btf_id(&env->log, resolve_prog_type(env->prog)); if (ret < 0) @@ -13184,11 +13166,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me } break; case ARG_PTR_TO_ARENA: - if (reg->type != PTR_TO_ARENA && reg->type != SCALAR_VALUE) { - verbose(env, "%s is not a pointer to arena or scalar\n", - reg_arg_name(env, argno)); - return -EINVAL; - } break; case ARG_PTR_TO_ALLOC_BTF_ID: if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC)) { @@ -13203,15 +13180,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me reg_arg_name(env, argno)); return -EINVAL; } - } else { - verbose(env, "%s expected pointer to allocated object\n", - reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass a pointer returned by the matching BPF object allocation path.", - "the kfunc expects an allocated object pointer, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; } if (!reg_is_referenced(env, reg)) { verbose(env, "allocated object must be referenced\n"); @@ -13260,12 +13228,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return ret; break; case ARG_PTR_TO_LIST_HEAD: - if (reg->type != PTR_TO_MAP_VALUE && - reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { - verbose(env, "%s expected pointer to map value or allocated object\n", - reg_arg_name(env, argno)); - return -EINVAL; - } if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC) && !reg_is_referenced(env, reg)) { verbose(env, "allocated object must be referenced\n"); @@ -13276,12 +13238,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return ret; break; case ARG_PTR_TO_RB_ROOT: - if (reg->type != PTR_TO_MAP_VALUE && - reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { - verbose(env, "%s expected pointer to map value or allocated object\n", - reg_arg_name(env, argno)); - return -EINVAL; - } if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC) && !reg_is_referenced(env, reg)) { verbose(env, "allocated object must be referenced\n"); @@ -13341,12 +13297,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me return ret; break; case ARG_CONST_MAP_PTR: - if (base_type(reg->type) != CONST_PTR_TO_MAP || - type_may_be_null(reg->type)) { - verbose(env, "pointer in %s isn't map pointer\n", - reg_arg_name(env, argno)); - return -EINVAL; - } ret = process_map_ptr_arg(env, reg, argno, meta); if (ret < 0) return ret; @@ -13435,16 +13385,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me argno_t buff_argno = argno_from_arg(i); enum bpf_mem_size_failure failure; - if (reg->type != SCALAR_VALUE) { - verbose(env, "%s is not a scalar\n", reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass an integer scalar length for this memory argument.", - "the kfunc expects a scalar memory size, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } - if (bpf_register_is_null(buff_reg)) break; @@ -13482,23 +13422,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me break; } case ARG_PTR_TO_FUNC: - if (reg->type != PTR_TO_FUNC) { - verbose(env, "%s expected pointer to func\n", reg_arg_name(env, argno)); - return -EINVAL; - } meta->subprogno = reg->subprogno; break; case ARG_PTR_TO_REFCOUNTED_KPTR: - if (!type_is_ptr_alloc_obj(reg->type)) { - verbose(env, "%s is neither owning or non-owning ref\n", - reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass an owning or non-owning pointer to a BPF-managed object containing a bpf_refcount field.", - "the kfunc expects a pointer to a BPF-managed refcounted object, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } if (!type_is_non_owning_ref(reg->type) && reg_is_referenced(env, reg)) meta->arg_owning_ref = true; @@ -13518,61 +13444,26 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me meta->arg_btf_id = reg->btf_id; break; case ARG_PTR_TO_CONST_STR: - if (reg->type != PTR_TO_MAP_VALUE) { - verbose(env, "%s doesn't point to a const string\n", - reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass a constant string pointer that the verifier recognizes, such as a string stored in a read-only map value.", - "the kfunc expects a pointer to a constant string stored in verifier-known memory, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } ret = check_arg_const_str(env, reg, argno); if (ret) return ret; break; case ARG_PTR_TO_WORKQUEUE: - if (reg->type != PTR_TO_MAP_VALUE) { - verbose(env, "%s doesn't point to a map value\n", - reg_arg_name(env, argno)); - return -EINVAL; - } ret = check_map_field_pointer(env, reg, argno, BPF_WORKQUEUE, &meta->map); if (ret < 0) return ret; break; case ARG_PTR_TO_TIMER: - if (reg->type != PTR_TO_MAP_VALUE) { - verbose(env, "%s doesn't point to a map value\n", - reg_arg_name(env, argno)); - return -EINVAL; - } ret = process_timer_func(env, reg, argno, &meta->map); if (ret < 0) return ret; break; case ARG_PTR_TO_TASK_WORK: - if (reg->type != PTR_TO_MAP_VALUE) { - verbose(env, "%s doesn't point to a map value\n", - reg_arg_name(env, argno)); - return -EINVAL; - } ret = check_map_field_pointer(env, reg, argno, BPF_TASK_WORK, &meta->map); if (ret < 0) return ret; break; case ARG_PTR_TO_IRQ_FLAG: - if (reg->type != PTR_TO_STACK) { - verbose(env, "%s doesn't point to an irq flag on stack\n", - reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass the same stack slot used by bpf_local_irq_save() or bpf_res_spin_lock_irqsave().", - "the kfunc expects a stack pointer to an IRQ flag slot, but %s is %s", - reg_arg_name(env, argno), - bpf_diag_reg_type_plain(env, reg->type)); - return -EINVAL; - } ret = process_irq_flag(env, reg, argno, meta); if (ret < 0) return ret; @@ -13585,13 +13476,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me verbose(env, "can't res_spin_{lock,unlock} in rbtree cb\n"); return -EACCES; } - - if (reg->type != PTR_TO_MAP_VALUE && reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { - verbose(env, "%s doesn't point to map value or allocated object\n", - reg_arg_name(env, argno)); - return -EINVAL; - } - if (!is_bpf_res_spin_lock_kfunc(meta->func_id)) return -EFAULT; if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock]) || diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c index 2b39cc1b09f9..0063e60d6f2f 100644 --- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c @@ -70,7 +70,7 @@ static struct kfunc_test_params kfunc_tests[] = { TC_FAIL(kfunc_call_test_get_mem_fail_oversized, 0, "allocation size exceeds u32 max"), TC_FAIL(kfunc_call_test_get_mem_fail_not_const, 0, "is not a const"), TC_FAIL(kfunc_call_test_mem_acquire_fail, 0, "acquire kernel function does not return PTR_TO_BTF_ID"), - TC_FAIL(kfunc_call_test_pointer_arg_type_mismatch, 0, "R1 expected pointer to ctx, but got scalar"), + TC_FAIL(kfunc_call_test_pointer_arg_type_mismatch, 0, "R1 type=scalar expected=ctx"), TC_FAIL(kfunc_call_test_spin_lock_unsafe, 0, "function calls are not allowed while holding a lock"), /* success cases */ diff --git a/tools/testing/selftests/bpf/progs/arena_kfunc.c b/tools/testing/selftests/bpf/progs/arena_kfunc.c index 50609f3b0564..6578cf12fa27 100644 --- a/tools/testing/selftests/bpf/progs/arena_kfunc.c +++ b/tools/testing/selftests/bpf/progs/arena_kfunc.c @@ -205,7 +205,7 @@ int arena_arg_no_arena(void *ctx) SEC("syscall") __arch_x86_64 __arch_arm64 -__failure __msg("is not a pointer to arena or scalar") +__failure __msg("R1 type=fp expected=arena, scalar") int arena_arg_bad_reg(void *ctx) { u64 buf = 0; diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c index efe7bcae70f8..a7c8c765a98d 100644 --- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c @@ -64,7 +64,7 @@ int BPF_PROG(cgrp_kfunc_acquire_no_null_check, struct cgroup *cgrp, const char * } SEC("tp_btf/cgroup_mkdir") -__failure __msg("R1 is fp expected STRUCT cgroup") +__failure __msg("R1 type=fp expected=ptr_, trusted_ptr_, rcu_ptr_") int BPF_PROG(cgrp_kfunc_acquire_fp, struct cgroup *cgrp, const char *path) { struct cgroup *acquired, *stack_cgrp = (struct cgroup *)&path; diff --git a/tools/testing/selftests/bpf/progs/cpumask_failure.c b/tools/testing/selftests/bpf/progs/cpumask_failure.c index 6d730535eb95..76a3cba6f23c 100644 --- a/tools/testing/selftests/bpf/progs/cpumask_failure.c +++ b/tools/testing/selftests/bpf/progs/cpumask_failure.c @@ -243,7 +243,7 @@ int BPF_PROG(test_populate_invalid_destination, struct task_struct *task, u64 cl } SEC("tp_btf/task_newtask") -__failure __msg("leads to invalid memory access") +__failure __msg("R2 type=scalar expected=fp") int BPF_PROG(test_populate_invalid_source, struct task_struct *task, u64 clone_flags) { void *garbage = (void *)0x123456; diff --git a/tools/testing/selftests/bpf/progs/irq.c b/tools/testing/selftests/bpf/progs/irq.c index a4a007866a33..53df6d248e26 100644 --- a/tools/testing/selftests/bpf/progs/irq.c +++ b/tools/testing/selftests/bpf/progs/irq.c @@ -15,7 +15,7 @@ struct bpf_res_spin_lock lockA __hidden SEC(".data.A"); struct bpf_res_spin_lock lockB __hidden SEC(".data.B"); SEC("?tc") -__failure __msg("R1 doesn't point to an irq flag on stack") +__failure __msg("R1 type=map_value expected=fp") int irq_save_bad_arg(struct __sk_buff *ctx) { bpf_local_irq_save(&global_flags); @@ -23,7 +23,7 @@ int irq_save_bad_arg(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("R1 doesn't point to an irq flag on stack") +__failure __msg("R1 type=map_value expected=fp") int irq_restore_bad_arg(struct __sk_buff *ctx) { bpf_local_irq_restore(&global_flags); diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c index c6699159dacd..65d4c6e01f93 100644 --- a/tools/testing/selftests/bpf/progs/iters.c +++ b/tools/testing/selftests/bpf/progs/iters.c @@ -1688,7 +1688,7 @@ int iter_subprog_check_stacksafe(const void *ctx) struct bpf_iter_num global_it; SEC("raw_tp") -__failure __msg("R1 expected pointer to an iterator on stack") +__failure __msg("R1 type=map_value expected=fp") int iter_new_bad_arg(const void *ctx) { bpf_iter_num_new(&global_it, 0, 1); @@ -1696,7 +1696,7 @@ int iter_new_bad_arg(const void *ctx) } SEC("raw_tp") -__failure __msg("R1 expected pointer to an iterator on stack") +__failure __msg("R1 type=map_value expected=fp") int iter_next_bad_arg(const void *ctx) { bpf_iter_num_next(&global_it); @@ -1704,7 +1704,7 @@ int iter_next_bad_arg(const void *ctx) } SEC("raw_tp") -__failure __msg("R1 expected pointer to an iterator on stack") +__failure __msg("R1 type=map_value expected=fp") int iter_destroy_bad_arg(const void *ctx) { bpf_iter_num_destroy(&global_it); diff --git a/tools/testing/selftests/bpf/progs/iters_testmod.c b/tools/testing/selftests/bpf/progs/iters_testmod.c index 5a3ff65e8234..f65cc9766633 100644 --- a/tools/testing/selftests/bpf/progs/iters_testmod.c +++ b/tools/testing/selftests/bpf/progs/iters_testmod.c @@ -105,8 +105,7 @@ int iter_next_rcu_not_trusted(const void *ctx) } SEC("raw_tp/sys_enter") -__failure __msg("R1 cannot write into rdonly_mem") -/* Message should not be 'R1 cannot write into rdonly_trusted_mem' */ +__failure __msg("R1 type=rdonly_mem expected=fp") int iter_next_ptr_mem_not_trusted(const void *ctx) { struct bpf_iter_num num_it; diff --git a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c index 3e0d4f687aaa..23019023511a 100644 --- a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c +++ b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c @@ -118,8 +118,7 @@ int atomic_rmw_not_ok(void *ctx) SEC("socket") __failure -__msg("invalid access to memory, mem_size=0 off=0 size=4") -__msg("R1 min value is outside of the allowed memory range") +__msg("R1 type=rdonly_untrusted_mem expected=fp") int kfunc_param_not_ok(void *ctx) { int *p; diff --git a/tools/testing/selftests/bpf/progs/rbtree_fail.c b/tools/testing/selftests/bpf/progs/rbtree_fail.c index 4504608196ab..08709f23ec0f 100644 --- a/tools/testing/selftests/bpf/progs/rbtree_fail.c +++ b/tools/testing/selftests/bpf/progs/rbtree_fail.c @@ -180,7 +180,7 @@ long rbtree_api_use_unchecked_remove_retval(void *ctx) } SEC("?tc") -__failure __msg("bpf_rbtree_remove can only take non-owning or refcounted bpf_rb_node pointer") +__failure __msg("R2 type=scalar expected=ptr_, rcu_ptr_, ptr_, rcu_ptr_") long rbtree_api_add_release_unlock_escape(void *ctx) { struct node_data *n; @@ -204,7 +204,7 @@ long rbtree_api_add_release_unlock_escape(void *ctx) } SEC("?tc") -__failure __msg("bpf_rbtree_remove can only take non-owning or refcounted bpf_rb_node pointer") +__failure __msg("R2 type=scalar expected=ptr_, rcu_ptr_, ptr_, rcu_ptr_") long rbtree_api_first_release_unlock_escape(void *ctx) { struct bpf_rb_node *res; diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c index 338e43822ffe..e80f78fae227 100644 --- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c @@ -118,8 +118,8 @@ long refcount_acquire_maybe_null(void *ctx) } SEC("?tc") -__failure __msg("R1 is neither owning or non-owning ref") -__msg("expects a pointer to a BPF-managed refcounted object, but R1 is a context pointer") +__failure __msg("R1 type=ctx expected=ptr_, rcu_ptr_, ptr_, rcu_ptr_") +__msg("type ctx, but this argument accepts ptr_, rcu_ptr_, ptr_, rcu_ptr_") long refcount_acquire_non_object(void *ctx) { return bpf_refcount_acquire(ctx) != NULL; @@ -159,8 +159,7 @@ long refcount_acquire_rcu_map_kptr_unchecked_drop(void *ctx) SEC("?syscall") __failure -__msg("bpf_rbtree_remove can only take non-owning or refcounted " - "bpf_rb_node pointer") +__msg("R2 type=untrusted_ptr_ expected=ptr_, rcu_ptr_, ptr_, rcu_ptr_") long rbtree_remove_after_rcu_unlock(void *ctx) { struct map_value_rcu_graph *mapval; @@ -190,7 +189,7 @@ long rbtree_remove_after_rcu_unlock(void *ctx) } SEC("?syscall") -__failure __msg("R1 is neither owning or non-owning ref") +__failure __msg("R1 type=untrusted_ptr_ expected=ptr_, rcu_ptr_, ptr_, rcu_ptr_") long refcount_acquire_after_rcu_unlock(void *ctx) { struct map_value_refcount_only *mapval; diff --git a/tools/testing/selftests/bpf/progs/res_spin_lock_fail.c b/tools/testing/selftests/bpf/progs/res_spin_lock_fail.c index 330682a88c16..8fd591bd1f6c 100644 --- a/tools/testing/selftests/bpf/progs/res_spin_lock_fail.c +++ b/tools/testing/selftests/bpf/progs/res_spin_lock_fail.c @@ -24,7 +24,7 @@ struct bpf_spin_lock lock __hidden SEC(".data.A"); struct bpf_res_spin_lock res_lock __hidden SEC(".data.B"); SEC("?tc") -__failure __msg("point to map value or allocated object") +__failure __msg("R1 type=untrusted_ptr_ expected=map_value, ptr_") int res_spin_lock_arg(struct __sk_buff *ctx) { struct arr_elem *elem; diff --git a/tools/testing/selftests/bpf/progs/stream_fail.c b/tools/testing/selftests/bpf/progs/stream_fail.c index 21428bb1ee59..10ebb4a7f105 100644 --- a/tools/testing/selftests/bpf/progs/stream_fail.c +++ b/tools/testing/selftests/bpf/progs/stream_fail.c @@ -23,7 +23,7 @@ int stream_vprintk_scalar_arg(void *ctx) } SEC("syscall") -__failure __msg("R2 doesn't point to a const string") +__failure __msg("R2 type=ctx expected=map_value") int stream_vprintk_string_arg(void *ctx) { bpf_stream_vprintk(BPF_STDOUT, ctx, NULL, 0); diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c index 9979766d4d74..404f7f9d7150 100644 --- a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c @@ -50,7 +50,7 @@ int BPF_PROG(task_kfunc_acquire_untrusted, struct task_struct *task, u64 clone_f } SEC("tp_btf/task_newtask") -__failure __msg("R1 is fp expected STRUCT task_struct") +__failure __msg("R1 type=fp expected=ptr_, trusted_ptr_, rcu_ptr_") int BPF_PROG(task_kfunc_acquire_fp, struct task_struct *task, u64 clone_flags) { struct task_struct *acquired, *stack_task = (struct task_struct *)&clone_flags; diff --git a/tools/testing/selftests/bpf/progs/task_work_fail.c b/tools/testing/selftests/bpf/progs/task_work_fail.c index 3186e7b4b24e..bc56bdaca780 100644 --- a/tools/testing/selftests/bpf/progs/task_work_fail.c +++ b/tools/testing/selftests/bpf/progs/task_work_fail.c @@ -58,7 +58,7 @@ int mismatch_map(struct pt_regs *args) } SEC("perf_event") -__failure __msg("R2 doesn't point to a map value") +__failure __msg("R2 type=fp expected=map_value") int no_map_task_work(struct pt_regs *args) { struct task_struct *task; diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c index bf48fc43c7ab..f7a83e502454 100644 --- a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c +++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c @@ -40,7 +40,7 @@ int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size, } SEC("?lsm.s/bpf") -__failure __msg("R1 expected pointer to stack or const struct bpf_dynptr") +__failure __msg("R1 type=map_value expected=fp, dynptr_ptr") int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size, bool kernel) { static struct bpf_dynptr val; diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c index ff08aa75d6f7..2cea3d9c3647 100644 --- a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c +++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c @@ -28,7 +28,7 @@ int BPF_PROG(get_task_exe_file_kfunc_null) } SEC("lsm.s/inode_getxattr") -__failure __msg("R1 is fp expected STRUCT task_struct") +__failure __msg("R1 type=fp expected=ptr_, trusted_ptr_, rcu_ptr_") int BPF_PROG(get_task_exe_file_kfunc_fp) { u64 x; diff --git a/tools/testing/selftests/bpf/progs/wq_failures.c b/tools/testing/selftests/bpf/progs/wq_failures.c index 32dc8827e128..bd30217579d4 100644 --- a/tools/testing/selftests/bpf/progs/wq_failures.c +++ b/tools/testing/selftests/bpf/progs/wq_failures.c @@ -48,7 +48,7 @@ __log_level(2) __flag(BPF_F_TEST_STATE_FREQ) __failure __msg(": (85) call bpf_wq_init#") /* anchor message */ -__msg("pointer in R2 isn't map pointer") +__msg("R2 type=fp expected=map_ptr") long test_wq_init_nomap(void *ctx) { struct bpf_wq *wq; @@ -98,7 +98,7 @@ __failure * is a correct bpf_wq pointer. */ __msg(": (85) call bpf_wq_set_callback#") /* anchor message */ -__msg("R1 doesn't point to a map value") +__msg("R1 type=fp expected=map_value") long test_wrong_wq_pointer(void *ctx) { int key = 0; diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c index d730215e520b..8b94b87135bc 100644 --- a/tools/testing/selftests/bpf/verifier/calls.c +++ b/tools/testing/selftests/bpf/verifier/calls.c @@ -31,7 +31,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "R1 is fp expected STRUCT prog_test_fail1", + .errstr = "R1 type=fp expected=ptr_, trusted_ptr_, rcu_ptr_", .fixup_kfunc_btf_id = { { "bpf_kfunc_call_test_fail1", 2 }, }, @@ -46,7 +46,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "max struct nesting depth exceeded\nR1 is fp expected STRUCT prog_test_fail2", + .errstr = "max struct nesting depth exceeded\nR1 type=fp expected=ptr_, trusted_ptr_, rcu_ptr_", .fixup_kfunc_btf_id = { { "bpf_kfunc_call_test_fail2", 2 }, }, @@ -61,7 +61,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "R1 is fp expected STRUCT prog_test_fail3", + .errstr = "R1 type=fp expected=ptr_, trusted_ptr_, rcu_ptr_", .fixup_kfunc_btf_id = { { "bpf_kfunc_call_test_fail3", 2 }, }, @@ -76,7 +76,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "R1 expected pointer to ctx, but got fp", + .errstr = "R1 type=fp expected=ctx", .fixup_kfunc_btf_id = { { "bpf_kfunc_call_test_pass_ctx", 2 }, }, -- 2.52.0 check_func_arg() checks whether a helper permits packet pointers before dispatching argument-specific memory validation. check_kfunc_args() has no equivalent check, even though kfunc memory arguments may be backed by packet data. Move packet-access validation to check_helper_mem_access(), where the access direction is known and helper, kfunc, and global-subprogram memory arguments converge. Pass call metadata there so helpers continue to require bpf_func_proto::pkt_access, while writes through kfunc and global subprogram arguments use the program-type policy and set env->seen_direct_write. Keep call metadata when variable-size memory disables raw mode by clearing arg_raw_mem.regno instead, and pass it through the map-key path as well. This also makes kfunc and global-subprogram packet writes request the required writable-packet prologue and rejects them for program types that only support direct packet reads. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 24 +++++++++---------- .../bpf/progs/verifier_helper_packet_access.c | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 254458cc1737..bb33d0755a61 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4881,7 +4881,7 @@ static int check_map_access(struct bpf_verifier_env *env, struct bpf_reg_state * } static bool may_access_direct_pkt_data(struct bpf_verifier_env *env, - const struct bpf_func_proto *fn, + const struct bpf_call_arg_meta *meta, enum bpf_access_type t) { enum bpf_prog_type prog_type = resolve_prog_type(env->prog); @@ -4905,10 +4905,11 @@ static bool may_access_direct_pkt_data(struct bpf_verifier_env *env, case BPF_PROG_TYPE_LWT_XMIT: case BPF_PROG_TYPE_SK_SKB: case BPF_PROG_TYPE_SK_MSG: - if (fn) - return fn->pkt_access; + if (meta && !meta->btf && meta->func_id) + return meta->fn->pkt_access; - env->seen_direct_write = true; + if (t == BPF_WRITE) + env->seen_direct_write = true; return true; case BPF_PROG_TYPE_CGROUP_SOCKOPT: @@ -7095,6 +7096,10 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, struct bpf_reg_ switch (base_type(reg->type)) { case PTR_TO_PACKET: case PTR_TO_PACKET_META: + if (!may_access_direct_pkt_data(env, meta, access_type)) { + verbose(env, "function access to the packet is not allowed\n"); + return -EACCES; + } return check_packet_access(env, reg, argno, 0, access_size, zero_size_allowed); case PTR_TO_MAP_KEY: @@ -7211,7 +7216,7 @@ static int check_mem_size_reg(struct bpf_verifier_env *env, * the memory that the helper could just partially fill up. */ if (!tnum_is_const(size_reg->var_off)) - meta = NULL; + meta->arg_raw_mem.regno = 0; if (reg_smin(size_reg) < 0) { verbose(env, "%s min value is negative, either use unsigned or 'var &= const'\n", @@ -8822,7 +8827,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, arg); enum bpf_arg_type arg_type = fn->arg_type[arg]; int regno = reg_from_argno(argno); - enum bpf_reg_type type = reg->type; u32 arg_size = arg_type & MEM_FIXED_SIZE ? fn->arg_size[arg] : 0; u32 key_size; int err = 0; @@ -8850,12 +8854,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return 0; } - if (type_is_pkt_pointer(type) && - !may_access_direct_pkt_data(env, fn, BPF_READ)) { - verbose(env, "helper access to the packet is not allowed\n"); - return -EACCES; - } - err = resolve_func_arg_type(env, reg, arg, meta, &arg_type, &arg_size); if (err) return err; @@ -8932,7 +8930,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return -EFAULT; } key_size = meta->map.ptr->key_size; - err = check_helper_mem_access(env, reg, argno, key_size, BPF_READ, false, NULL, + err = check_helper_mem_access(env, reg, argno, key_size, BPF_READ, false, meta, NULL); if (err) return err; diff --git a/tools/testing/selftests/bpf/progs/verifier_helper_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_helper_packet_access.c index 71cee3f58324..12786b72c694 100644 --- a/tools/testing/selftests/bpf/progs/verifier_helper_packet_access.c +++ b/tools/testing/selftests/bpf/progs/verifier_helper_packet_access.c @@ -258,7 +258,7 @@ l0_%=: r0 = 0; \ SEC("tc") __description("helper access to packet: test11, cls unsuitable helper 1") -__failure __msg("helper access to the packet") +__failure __msg("function access to the packet") __naked void test11_cls_unsuitable_helper_1(void) { asm volatile (" \ @@ -283,7 +283,7 @@ l0_%=: r0 = 0; \ SEC("tc") __description("helper access to packet: test12, cls unsuitable helper 2") -__failure __msg("helper access to the packet") +__failure __msg("function access to the packet") __naked void test12_cls_unsuitable_helper_2(void) { asm volatile (" \ -- 2.52.0 A kfunc memory argument may be backed by packet data. Exercise this with bpf_skb_ct_lookup(), which writes errors through its opts memory argument. Verify that the verifier marks the program as directly writing packet data, causing TC to emit its writable-skb prologue. Match the semantic prologue sequence while allowing architecture-specific zero extensions and the endian-specific cloned-bit mask. Signed-off-by: Amery Hung --- .../selftests/bpf/prog_tests/verifier.c | 2 + .../bpf/progs/verifier_kfunc_packet_access.c | 47 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/verifier_kfunc_packet_access.c diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c index f7f94ccebce2..b97381448248 100644 --- a/tools/testing/selftests/bpf/prog_tests/verifier.c +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c @@ -54,6 +54,7 @@ #include "verifier_iterating_callbacks.skel.h" #include "verifier_jeq_infer_not_null.skel.h" #include "verifier_jit_convergence.skel.h" +#include "verifier_kfunc_packet_access.skel.h" #include "verifier_ld_ind.skel.h" #include "verifier_ldsx.skel.h" #include "verifier_leak_ptr.skel.h" @@ -218,6 +219,7 @@ void test_verifier_int_ptr(void) { RUN(verifier_int_ptr); } void test_verifier_iterating_callbacks(void) { RUN(verifier_iterating_callbacks); } void test_verifier_jeq_infer_not_null(void) { RUN(verifier_jeq_infer_not_null); } void test_verifier_jit_convergence(void) { RUN(verifier_jit_convergence); } +void test_verifier_kfunc_packet_access(void) { RUN_TESTS(verifier_kfunc_packet_access); } void test_verifier_load_acquire(void) { RUN(verifier_load_acquire); } void test_verifier_ld_ind(void) { RUN(verifier_ld_ind); } void test_verifier_ldsx(void) { RUN(verifier_ldsx); } diff --git a/tools/testing/selftests/bpf/progs/verifier_kfunc_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_kfunc_packet_access.c new file mode 100644 index 000000000000..88009566d92f --- /dev/null +++ b/tools/testing/selftests/bpf/progs/verifier_kfunc_packet_access.c @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include "bpf_misc.h" + +struct nf_conn *bpf_skb_ct_lookup(struct __sk_buff *skb_ctx, + struct bpf_sock_tuple *bpf_tuple, + u32 tuple__sz, struct bpf_ct_opts *opts, + u32 opts__sz) __ksym; +void bpf_ct_release(struct nf_conn *nfct) __ksym; + +char _license[] SEC("license") = "GPL"; + +SEC("tc") +__description("kfunc packet write requests writable skb") +__success +/* bpf_unclone_prologue() */ +__xlated("r6 = *(u8 *)(r1 +{{[0-9]+}})") +__xlated("...") +__xlated("w6 &= {{(1|128)}}") +__xlated("...") +__xlated("if r6 == 0x0 goto") +__xlated("r6 = r1") +__xlated("r2 ^= r2") +__xlated("call") +__xlated("if r0 == 0x0 goto") +__xlated("w0 = 2") +__xlated("...") +__xlated("exit") +__xlated("r1 = r6") +int kfunc_packet_write(struct __sk_buff *skb) +{ + void *data_end = (void *)(long)skb->data_end; + void *data = (void *)(long)skb->data; + struct bpf_sock_tuple tuple = {}; + struct nf_conn *nfct; + + if (data + sizeof(struct bpf_ct_opts) > data_end) + return 0; + + /* An invalid tuple size makes bpf_skb_ct_lookup() write opts->error. */ + nfct = bpf_skb_ct_lookup(skb, &tuple, 1, data, sizeof(struct bpf_ct_opts)); + if (nfct) + bpf_ct_release(nfct); + return 0; +} -- 2.52.0 Helper and kfunc argument verification both require a live owning reference for non-dynptr release arguments. Whether that reference may be NULL is expressed by PTR_MAYBE_NULL in helper prototypes and the __nullable BTF suffix for kfuncs. Factor the shared ownership policy into check_func_arg_release(). Drop the helper-only rejection of maybe-null owning references. This allows bpf_kptr_xchg() to accept its explicitly nullable source directly, matching nullable kfunc release arguments. The verifier consumes the reference regardless of its runtime nullness. Update the tests so an owned nullable source succeeds while a nullable non-owning source remains rejected for lacking ownership. Preserve the identity of a nullable per-CPU allocation when bpf_kptr_xchg() transfers its ownership inside an RCU critical section. The converted MEM_RCU aliases no longer own a reference, but still need a non-zero ID so a NULL check on one allocation cannot refine another unrelated allocation. Use common legacy and structured diagnostic text for both call kinds. Tailor the structured suggestion to describe a pointer for helpers and include the expected BTF type for kfuncs. Resolve that type lazily from the kfunc BTF parameter only when reporting an ownership failure. The kfunc path now keys the check directly on the cached OBJ_RELEASE argument flag instead of comparing the argument register against meta->release_regno, and uses the cached argument kind for the dynptr exemption. This is another prerequisite for routing both call types through check_func_arg(). Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 82 ++++++++++--------- .../selftests/bpf/prog_tests/cb_refs.c | 4 +- .../selftests/bpf/progs/cgrp_kfunc_failure.c | 6 +- .../selftests/bpf/progs/cpumask_failure.c | 2 +- .../selftests/bpf/progs/map_kptr_fail.c | 4 +- .../selftests/bpf/progs/task_kfunc_failure.c | 6 +- .../bpf/progs/verifier_global_ptr_args.c | 2 +- .../bpf/progs/verifier_ref_tracking.c | 2 +- .../selftests/bpf/progs/verifier_sock.c | 6 +- .../selftests/bpf/progs/verifier_vfs_reject.c | 2 +- .../selftests/bpf/progs/wakeup_source_fail.c | 2 +- 11 files changed, 63 insertions(+), 55 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index bb33d0755a61..fe8a45303602 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8463,6 +8463,43 @@ static int check_func_arg_nullability(struct bpf_verifier_env *env, return -EACCES; } +static int check_func_arg_release(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_release(arg_type)) + return 0; + + if (arg_type_is_dynptr(arg_type) || reg_is_referenced(env, reg) || + bpf_register_is_null(reg)) + return 0; + + verbose(env, "release function %s expects referenced PTR_TO_BTF_ID passed to %s\n", + meta->func_name, reg_arg_name(env, argno)); + + if (meta->btf) { + const struct btf_param *btf_arg; + const struct btf_type *t; + u32 ref_id; + + btf_arg = &btf_params(meta->func_proto)[arg_idx_from_argno(argno)]; + ref_id = btf_arg->type; + t = btf_type_skip_modifiers(meta->btf, btf_arg->type, NULL); + if (btf_type_is_ptr(t)) + btf_type_skip_modifiers(meta->btf, t->type, &ref_id); + expected_type = bpf_diag_fmt(env, "value of type %s", + bpf_diag_fmt_btf_type(env, meta->btf, ref_id)); + } + + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + bpf_diag_fmt(env, "Pass the resource-owning %s returned by the matching acquire call, or avoid the release function after ownership has already been transferred or released.", + expected_type), + "release functions require a value that owns a live resource returned by a matching acquire function"); + return -EINVAL; +} + static const char *bpf_diag_expected_reg_types(struct bpf_verifier_env *env, const enum bpf_reg_type *types, int count) { @@ -8881,29 +8918,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return err; skip_type_check: - 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; - } - } + err = check_func_arg_release(env, reg, argno, arg_type, meta, insn_idx); + if (err) + return err; if (reg_is_referenced(env, reg)) update_ref_obj(&meta->ref_obj, reg); @@ -9748,7 +9765,8 @@ static int ref_convert_alloc_rcu_protected(struct bpf_verifier_env *env, u32 id) continue; if ((reg->type & MEM_ALLOC) && (reg->type & MEM_PERCPU)) { bpf_diag_mod_begin(env, reg, NULL, BPF_DIAG_MOD_WRITE); - reg->id = 0; + if (!type_may_be_null(reg->type)) + reg->id = 0; reg->type &= ~MEM_ALLOC; reg->type |= MEM_RCU; bpf_diag_mod_end(env); @@ -13089,19 +13107,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me 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)) { - const char *expected_type; - - expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); - verbose(env, "release kfunc %s expects referenced PTR_TO_BTF_ID passed to %s\n", - func_name, reg_arg_name(env, argno)); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass the resource-owning pointer returned by the matching acquire kfunc, and avoid calling the release kfunc after ownership has already been transferred or released.", - "release kfuncs require a resource-owning value of type %s returned by a matching acquire kfunc", - expected_type); - return -EINVAL; - } + ret = check_func_arg_release(env, reg, argno, arg_type, meta, insn_idx); + if (ret < 0) + return ret; if (reg_is_referenced(env, reg)) update_ref_obj(&meta->ref_obj, reg); diff --git a/tools/testing/selftests/bpf/prog_tests/cb_refs.c b/tools/testing/selftests/bpf/prog_tests/cb_refs.c index 50ea3d72d446..e415acc08b16 100644 --- a/tools/testing/selftests/bpf/prog_tests/cb_refs.c +++ b/tools/testing/selftests/bpf/prog_tests/cb_refs.c @@ -11,8 +11,8 @@ struct { const char *prog_name; 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 trusted R2" }, + { "underflow_prog", "release function bpf_kfunc_call_test_release expects referenced PTR_TO_BTF_ID passed to R1" }, + { "leak_prog", "Unreleased reference id=4 alloc_insn=3" }, /* alloc_insn=3{2,3} */ { "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} */ }; diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c index a7c8c765a98d..731eaa09db96 100644 --- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c @@ -154,7 +154,7 @@ int BPF_PROG(cgrp_kfunc_xchg_unreleased, struct cgroup *cgrp, const char *path) } SEC("tp_btf/cgroup_mkdir") -__failure __msg("release kfunc bpf_cgroup_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_cgroup_release expects referenced PTR_TO_BTF_ID passed to R1") int BPF_PROG(cgrp_kfunc_rcu_get_release, struct cgroup *cgrp, const char *path) { struct cgroup *kptr; @@ -191,7 +191,7 @@ int BPF_PROG(cgrp_kfunc_release_untrusted, struct cgroup *cgrp, const char *path } SEC("tp_btf/cgroup_mkdir") -__failure __msg("release kfunc bpf_cgroup_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_cgroup_release expects referenced PTR_TO_BTF_ID passed to R1") int BPF_PROG(cgrp_kfunc_release_fp, struct cgroup *cgrp, const char *path) { struct cgroup *acquired = (struct cgroup *)&path; @@ -237,7 +237,7 @@ int BPF_PROG(cgrp_kfunc_release_null, struct cgroup *cgrp, const char *path) } SEC("tp_btf/cgroup_mkdir") -__failure __msg("release kfunc bpf_cgroup_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_cgroup_release expects referenced PTR_TO_BTF_ID passed to R1") int BPF_PROG(cgrp_kfunc_release_unacquired, struct cgroup *cgrp, const char *path) { /* Cannot release trusted cgroup pointer which was not acquired. */ diff --git a/tools/testing/selftests/bpf/progs/cpumask_failure.c b/tools/testing/selftests/bpf/progs/cpumask_failure.c index 76a3cba6f23c..c89c88db39d1 100644 --- a/tools/testing/selftests/bpf/progs/cpumask_failure.c +++ b/tools/testing/selftests/bpf/progs/cpumask_failure.c @@ -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 trusted R2") +__failure __msg("release function bpf_kptr_xchg expects referenced PTR_TO_BTF_ID passed to R2") int BPF_PROG(test_global_mask_rcu_no_null_check, struct task_struct *task, u64 clone_flags) { struct bpf_cpumask *prev, *curr; diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c index 186b56357110..ac4003bfb8b0 100644 --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c @@ -252,7 +252,7 @@ int reject_untrusted_store_to_ref(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("release helper bpf_kptr_xchg expects referenced PTR_TO_BTF_ID passed to R2") +__failure __msg("release function bpf_kptr_xchg expects referenced PTR_TO_BTF_ID passed to R2") int reject_untrusted_xchg(struct __sk_buff *ctx) { struct prog_test_ref_kfunc *p; @@ -364,7 +364,7 @@ int kptr_xchg_ref_state(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("Possibly NULL pointer passed to trusted R2") +__success int kptr_xchg_possibly_null(struct __sk_buff *ctx) { struct prog_test_ref_kfunc *p; diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c index 404f7f9d7150..0ca2e1a38a94 100644 --- a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c @@ -179,7 +179,7 @@ int BPF_PROG(task_kfunc_release_untrusted, struct task_struct *task, u64 clone_f } SEC("tp_btf/task_newtask") -__failure __msg("release kfunc bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") int BPF_PROG(task_kfunc_release_fp, struct task_struct *task, u64 clone_flags) { struct task_struct *acquired = (struct task_struct *)&clone_flags; @@ -225,7 +225,7 @@ int BPF_PROG(task_kfunc_release_null, struct task_struct *task, u64 clone_flags) } SEC("tp_btf/task_newtask") -__failure __msg("release kfunc bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") int BPF_PROG(task_kfunc_release_unacquired, struct task_struct *task, u64 clone_flags) { /* Cannot release trusted task pointer which was not acquired. */ @@ -353,7 +353,7 @@ int BPF_PROG(task_access_comm4, struct task_struct *task, const char *buf, bool } SEC("tp_btf/task_newtask") -__failure __msg("release kfunc bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") int BPF_PROG(task_kfunc_release_in_map, struct task_struct *task, u64 clone_flags) { struct task_struct *local; diff --git a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c index a3d2af8dc839..b277b1efb8c7 100644 --- a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c +++ b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c @@ -177,7 +177,7 @@ __weak int subprog_trusted_destroy(struct task_struct *task __arg_trusted) SEC("?tp_btf/task_newtask") __failure __log_level(2) -__msg("release kfunc bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") +__msg("release function bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") int BPF_PROG(trusted_destroy_fail, struct task_struct *task, u64 clone_flags) { return subprog_trusted_destroy(task); diff --git a/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c b/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c index f3fad911b5bc..799db6f5713b 100644 --- a/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c +++ b/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c @@ -1288,7 +1288,7 @@ l1_%=: r1 = r6; \ SEC("tc") __description("reference tracking: bpf_sk_release(listen_sk)") -__failure __msg("release helper bpf_sk_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_sk_release expects referenced PTR_TO_BTF_ID passed to R1") __naked void bpf_sk_release_listen_sk(void) { asm volatile ( diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c index d59b2f905671..2a136c917680 100644 --- a/tools/testing/selftests/bpf/progs/verifier_sock.c +++ b/tools/testing/selftests/bpf/progs/verifier_sock.c @@ -603,7 +603,7 @@ l2_%=: r0 = *(u32*)(r0 + %[bpf_tcp_sock_snd_cwnd]); \ SEC("tc") __description("bpf_sk_release(skb->sk)") -__failure __msg("release helper bpf_sk_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_sk_release expects referenced PTR_TO_BTF_ID passed to R1") __naked void bpf_sk_release_skb_sk(void) { asm volatile (" \ @@ -620,7 +620,7 @@ l0_%=: r0 = 0; \ SEC("tc") __description("bpf_sk_release(bpf_sk_fullsock(skb->sk))") -__failure __msg("release helper bpf_sk_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_sk_release expects referenced PTR_TO_BTF_ID passed to R1") __naked void bpf_sk_fullsock_skb_sk(void) { asm volatile (" \ @@ -644,7 +644,7 @@ l1_%=: r1 = r0; \ SEC("tc") __description("bpf_sk_release(bpf_tcp_sock(skb->sk))") -__failure __msg("release helper bpf_sk_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_sk_release expects referenced PTR_TO_BTF_ID passed to R1") __naked void bpf_tcp_sock_skb_sk(void) { asm volatile (" \ diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c index 2cea3d9c3647..b5f456d57669 100644 --- a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c +++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c @@ -80,7 +80,7 @@ int BPF_PROG(get_task_exe_file_kfunc_unreleased) } SEC("lsm.s/file_open") -__failure __msg("release kfunc bpf_put_file expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_put_file expects referenced PTR_TO_BTF_ID passed to R1") int BPF_PROG(put_file_kfunc_unacquired, struct file *file) { /* Can't release an unacquired pointer. */ diff --git a/tools/testing/selftests/bpf/progs/wakeup_source_fail.c b/tools/testing/selftests/bpf/progs/wakeup_source_fail.c index d4d0f1610853..ff9ea5aa80ad 100644 --- a/tools/testing/selftests/bpf/progs/wakeup_source_fail.c +++ b/tools/testing/selftests/bpf/progs/wakeup_source_fail.c @@ -42,7 +42,7 @@ int wakeup_source_access_lock_fields(void *ctx) } SEC("syscall") -__failure __msg("release kfunc bpf_wakeup_sources_read_unlock expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("release function bpf_wakeup_sources_read_unlock expects referenced PTR_TO_BTF_ID passed to R1") int wakeup_source_unlock_no_lock(void *ctx) { struct bpf_ws_lock *lock = (void *)0x1; -- 2.52.0 Pass two unchecked bpf_percpu_obj_new() results to bpf_kptr_xchg() inside an RCU critical section. The exchanges consume ownership and leave nullable RCU-protected aliases. Checking the first alias must not make the second non-NULL. Verify that bpf_this_cpu_ptr() still rejects the unchecked second alias. Signed-off-by: Amery Hung --- .../selftests/bpf/progs/percpu_alloc_fail.c | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c index 3701f4ea58c7..57615b0f0c25 100644 --- a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c +++ b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c @@ -24,6 +24,7 @@ struct val_600b_t { struct elem { long sum; struct val_t __percpu_kptr *pc; + struct val_t __percpu_kptr *pc2; }; struct { @@ -46,6 +47,8 @@ struct { struct task_struct *bpf_task_from_pid(s32 pid) __ksym; void bpf_task_release(struct task_struct *p) __ksym; +void bpf_rcu_read_lock(void) __ksym; +void bpf_rcu_read_unlock(void) __ksym; long ret; @@ -123,6 +126,38 @@ int BPF_PROG(test_array_map_3) return 0; } +SEC("?fentry.s/bpf_fentry_test1") +__failure __msg("Possibly NULL pointer passed to trusted R1") +int BPF_PROG(reject_nullable_percpu_xchg_alias) +{ + struct val_t __percpu_kptr *p1, *p2, *old; + struct val_t *v; + struct elem *e; + int index = 0; + + e = bpf_map_lookup_elem(&array, &index); + if (!e) + return 0; + + p1 = bpf_percpu_obj_new(struct val_t); + p2 = bpf_percpu_obj_new(struct val_t); + + bpf_rcu_read_lock(); + old = bpf_kptr_xchg(&e->pc, p1); + if (old) + bpf_percpu_obj_drop(old); + old = bpf_kptr_xchg(&e->pc2, p2); + if (old) + bpf_percpu_obj_drop(old); + + if (p1) { + v = bpf_this_cpu_ptr(p2); + v->b = 1; + } + bpf_rcu_read_unlock(); + return 0; +} + SEC("?fentry.s/bpf_fentry_test1") __failure __msg("R1 expected for bpf_percpu_obj_drop()") int BPF_PROG(test_array_map_4) -- 2.52.0 Generated kfunc prototypes now carry the argument kinds and metadata needed by the helper argument checker, but kfunc calls still duplicate the per-argument loop and switch. Add check_func_args() to determine the argument count, validate outgoing kfunc stack arguments, and drive check_func_arg() for both call kinds. Move the remaining kfunc-only argument handlers, including the dynptr clone runtime refinement, into check_func_arg(), derive each kfunc BTF referent there once, and skip check_reg_arg() for stack-passed arguments that check_outgoing_stack_args() already validated. Keep special-kfunc ID checks behind small helpers so their table can remain in place. Keep the intentional differences selected by call metadata: helpers retain their privileged ARG_ANYTHING pointer-leak behavior, while kfunc scalars use strict ARG_SCALAR register admission. The shared ARG_PTR_TO_BTF_ID case retains helper-only compatible-ID and bpf_kptr_xchg() handling, performs kfunc trusted/RCU provenance checks, and selects the expected BTF based on call kind. Helper and kfunc memory arguments likewise retain their access modes and share fixed-memory and memory/size diagnostics. Treat an accepted nullable NULL as a completed argument check, matching the former kfunc continue path. The skipped helper-specific processing is either a no-op for NULL or deferred to the paired size argument. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 922 +++++++++--------- .../selftests/bpf/prog_tests/cb_refs.c | 2 +- .../selftests/bpf/progs/cgrp_kfunc_failure.c | 2 +- .../selftests/bpf/progs/task_kfunc_failure.c | 2 +- .../selftests/bpf/progs/wakeup_source_fail.c | 2 +- 5 files changed, 440 insertions(+), 490 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index fe8a45303602..617a277c3558 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -7719,7 +7719,7 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat verbose(env, "Dynptr has to be an uninitialized dynptr\n"); bpf_diag_res( env, insn_idx, "dynptr is already initialized", - "This kfunc constructs a dynptr and requires an uninitialized dynptr stack slot, but the selected slot already holds dynptr state.", + "This function constructs a dynptr and requires an uninitialized dynptr stack slot, but the selected slot already holds dynptr state.", "Use a fresh stack dynptr slot, or release/destroy the existing dynptr before reusing the slot."); return -EINVAL; } @@ -7848,8 +7848,8 @@ static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state * reg_arg_name(env, argno)); bpf_diag_call_arg( env, insn_idx, argno, meta->func_name, - "the kfunc expects a recognized iterator state pointer, but this argument does not match a valid iterator type", - "Pass the exact iterator state type expected by this kfunc."); + "the function expects a recognized iterator state pointer, but this argument does not match a valid iterator type", + "Pass the exact iterator state type expected by this function."); return -EINVAL; } t = btf_type_by_id(meta->btf, btf_id); @@ -8245,6 +8245,39 @@ static int process_arg_ptr_to_btf_id(struct bpf_verifier_env *env, struct bpf_re argno_t argno, enum bpf_arg_type arg_type, const struct btf *arg_btf, u32 arg_btf_id, struct bpf_call_arg_meta *meta, int insn_idx); +static bool is_kfunc_arg_nonown_allowed(const struct btf *btf, + const struct btf_param *arg); +static bool is_kfunc_arg_scalar_with_name(const struct btf *btf, + const struct btf_param *arg, + const char *name); +static bool is_bpf_cast_to_kern_ctx_kfunc(const struct bpf_call_arg_meta *meta); +static bool is_bpf_dynptr_clone_kfunc(const struct bpf_call_arg_meta *meta); +static bool is_bpf_iter_css_task_new_kfunc(const struct bpf_call_arg_meta *meta); +static bool is_bpf_obj_drop_kfunc(u32 func_id); +static bool is_bpf_percpu_obj_drop_kfunc(u32 func_id); +static bool is_bpf_rbtree_add_kfunc(u32 func_id); +static int get_bpf_res_spin_lock_kfunc_flags(const struct bpf_call_arg_meta *meta); +static struct bpf_insn_aux_data *cur_aux(const struct bpf_verifier_env *env); +static int process_irq_flag(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, argno_t argno, + struct bpf_call_arg_meta *meta); +static int process_kf_arg_ptr_to_list_head(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, + argno_t argno, + struct bpf_call_arg_meta *meta); +static int process_kf_arg_ptr_to_rbtree_root(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, + argno_t argno, + struct bpf_call_arg_meta *meta); +static int process_kf_arg_ptr_to_list_node(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, + argno_t argno, + struct bpf_call_arg_meta *meta); +static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, + argno_t argno, + struct bpf_call_arg_meta *meta); +static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env); struct bpf_reg_types { const enum bpf_reg_type types[10]; @@ -8813,7 +8846,6 @@ static int get_constant_map_key(struct bpf_verifier_env *env, } static bool can_elide_value_nullness(const struct bpf_map *map); -static struct bpf_insn_aux_data *cur_aux(const struct bpf_verifier_env *env); static int process_map_ptr_arg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, struct bpf_call_arg_meta *meta) @@ -8857,6 +8889,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, struct bpf_call_arg_meta *meta, int insn_idx) { + const struct btf_param *btf_arg = meta->btf ? &btf_params(meta->func_proto)[arg] : NULL; const struct bpf_func_proto *fn = meta->fn; struct bpf_func_state *caller = cur_func(env); struct bpf_reg_state *regs = cur_regs(env); @@ -8882,6 +8915,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return err; } + /* Preserve the legacy helper behavior for privileged pointer leaks. */ if (arg_type == ARG_ANYTHING) { if (__is_pointer_value(env->allow_ptr_leaks, reg)) { verbose(env, "%s leaks addr into helper function\n", @@ -8896,13 +8930,10 @@ 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)) { - /* A NULL register has a SCALAR_VALUE type, so skip - * type checking. - */ - err = mark_chain_precision(env, regno); + err = mark_arg_precision(env, argno); if (err) return err; - goto skip_type_check; + return 0; } err = check_func_arg_nullability(env, reg, argno, arg_type, meta, insn_idx); @@ -8917,7 +8948,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, if (err) return err; -skip_type_check: err = check_func_arg_release(env, reg, argno, arg_type, meta, insn_idx); if (err) return err; @@ -8926,6 +8956,19 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, update_ref_obj(&meta->ref_obj, reg); switch (base_type(arg_type)) { + case ARG_CONST_SCALAR: + err = process_const_arg(env, reg, argno, meta); + if (err < 0) { + if (err == -EINVAL) + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Pass a compile-time constant or a value the verifier can prove is constant at this call.", + "the function requires this scalar argument to be a verifier-known constant, but %s is variable on this path", + reg_arg_name(env, argno)); + return err; + } + break; + case ARG_SCALAR: + break; case ARG_CONST_MAP_PTR: /* bpf_map_xxx(map_ptr) call: remember that map_ptr */ err = process_map_ptr_arg(env, reg, argno, meta); @@ -8991,6 +9034,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, case ARG_PTR_TO_BTF_ID_SOCK_COMMON: { const u32 *arg_btf_id = fn->arg_btf_id[arg]; + const struct btf *arg_btf = meta->btf ?: btf_vmlinux; if (!meta->btf) { const struct bpf_reg_types *compatible; @@ -9018,7 +9062,39 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, } } - err = process_arg_ptr_to_btf_id(env, reg, argno, arg_type, btf_vmlinux, + if (meta->btf && (!is_trusted_reg(env, reg) || + bpf_type_has_unsafe_modifiers(reg->type))) { + if (!(arg_type & MEM_RCU)) { + const char *actual_type, *arg_name, *expected_type; + + expected_type = bpf_diag_fmt_btf_type(env, arg_btf, *arg_btf_id); + verbose(env, "%s must be referenced or trusted\n", + reg_arg_name(env, argno)); + arg_name = reg_arg_name(env, argno); + actual_type = bpf_diag_reg_type_plain(env, reg->type); + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Pass a pointer acquired from a verifier-tracked source, or call this function only inside the required protection if it accepts RCU pointers.", + "the function requires a trusted or resource-owning pointer to %s, but %s is %s", + expected_type, arg_name, actual_type); + return -EINVAL; + } + if (!is_rcu_reg(reg)) { + const char *actual_type, *arg_name, *expected_type; + + expected_type = bpf_diag_fmt_btf_type(env, arg_btf, *arg_btf_id); + verbose(env, "%s must be a rcu pointer\n", + reg_arg_name(env, argno)); + arg_name = reg_arg_name(env, argno); + actual_type = bpf_diag_reg_type_plain(env, reg->type); + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Use this function with a pointer that is valid in an RCU read lock region.", + "the function requires an RCU-protected pointer to %s, but %s is %s", + expected_type, arg_name, actual_type); + return -EINVAL; + } + } + + err = process_arg_ptr_to_btf_id(env, reg, argno, arg_type, arg_btf, *arg_btf_id, meta, insn_idx); if (err < 0) return err; @@ -9055,44 +9131,274 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, if (err) return err; break; + case ARG_PTR_TO_CTX: + if (is_bpf_cast_to_kern_ctx_kfunc(meta)) { + err = get_kern_ctx_btf_id(&env->log, resolve_prog_type(env->prog)); + if (err < 0) + return -EINVAL; + meta->ret_btf_id = err; + } + break; + case ARG_PTR_TO_ARENA: + break; + case ARG_PTR_TO_ALLOC_BTF_ID: + if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC)) { + if (!is_bpf_obj_drop_kfunc(meta->func_id)) { + verbose(env, "%s expected for bpf_obj_drop()\n", + reg_arg_name(env, argno)); + return -EINVAL; + } + } else if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC | MEM_PERCPU)) { + if (!is_bpf_percpu_obj_drop_kfunc(meta->func_id)) { + verbose(env, "%s expected for bpf_percpu_obj_drop()\n", + reg_arg_name(env, argno)); + return -EINVAL; + } + } + if (!reg_is_referenced(env, reg)) { + verbose(env, "allocated object must be referenced\n"); + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Pass the owned object pointer before it is released or transferred.", + "the allocated object pointer in %s must still carry verifier-tracked ownership, but this pointer no longer owns a live resource", + reg_arg_name(env, argno)); + return -EINVAL; + } + if (meta->btf == btf_vmlinux) { + meta->arg_btf = reg->btf; + meta->arg_btf_id = reg->btf_id; + } + break; case ARG_PTR_TO_FUNC: meta->subprogno = reg->subprogno; break; case ARG_PTR_TO_MEM: + { + enum bpf_access_type access_type; + bool known_memory; + /* The access to this pointer is only checked when we hit the * next is_mem_size argument below. */ - if (arg_type & MEM_FIXED_SIZE) { - err = check_mem_reg(env, reg, argno, arg_size, - arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ, meta, NULL); - if (err) - return err; - if (arg_type & MEM_ALIGNED) - err = check_ptr_alignment(env, reg, 0, arg_size, true); + if (!(arg_type & MEM_FIXED_SIZE)) + break; + + access_type = arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ; + if (meta->btf) + access_type = BPF_READ | BPF_WRITE; + + err = check_mem_reg(env, reg, argno, arg_size, access_type, meta, &known_memory); + if (err < 0) { + if (known_memory) + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Pass memory with at least the required number of accessible bytes and suitable read or write access.", + "the function expects %u bytes of memory, but the verifier cannot prove that %s provides a range of that size with the required read or write access", + arg_size, + bpf_diag_reg_type_plain(env, reg->type)); + else + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Pass stack, map, context, or other verifier-known memory of the expected type and size, not an integer cast to a pointer.", + "the function expects %u bytes of memory, but it is %s and not verifier-known memory", + arg_size, + bpf_diag_reg_type_plain(env, reg->type)); + return err; } + if (arg_type & MEM_ALIGNED) + err = check_ptr_alignment(env, reg, 0, arg_size, true); break; + } + case ARG_CONST_MEM_SIZE: + err = process_const_arg(env, reg, argno, meta); + if (err < 0) { + if (err == -EINVAL) + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Pass a compile-time constant or a value the verifier can prove is constant at this call.", + "the function requires this memory size to be a verifier-known constant, but %s is variable on this path", + reg_arg_name(env, argno)); + return err; + } + fallthrough; case ARG_MEM_SIZE: - err = check_mem_size_reg(env, get_func_arg_reg(caller, regs, arg - 1), reg, - argno_from_arg(arg), argno, - fn->arg_type[arg - 1] & MEM_WRITE ? BPF_WRITE : BPF_READ, - false, meta, NULL); - break; case ARG_MEM_SIZE_OR_ZERO: - err = check_mem_size_reg(env, get_func_arg_reg(caller, regs, arg - 1), reg, - argno_from_arg(arg), argno, - fn->arg_type[arg - 1] & MEM_WRITE ? BPF_WRITE : BPF_READ, - true, meta, NULL); + { + struct bpf_reg_state *buff_reg = get_func_arg_reg(caller, regs, arg - 1); + argno_t buff_argno = argno_from_arg(arg); + enum bpf_mem_size_failure failure; + const char *buff_arg, *size_arg; + bool zero_size_allowed; + u32 access_type; + + if (meta->btf && bpf_register_is_null(buff_reg)) + break; + + access_type = fn->arg_type[arg - 1] & MEM_WRITE ? BPF_WRITE : BPF_READ; + if (meta->btf) + access_type = BPF_READ | BPF_WRITE; + + zero_size_allowed = meta->btf || base_type(arg_type) == ARG_MEM_SIZE_OR_ZERO; + + err = check_mem_size_reg(env, buff_reg, reg, buff_argno, argno, + access_type, zero_size_allowed, meta, &failure); + if (!err) + break; + + buff_arg = bpf_diag_arg_name(env, buff_argno); + size_arg = bpf_diag_arg_name(env, argno); + verbose(env, "%s and ", reg_arg_name(env, buff_argno)); + verbose(env, "%s memory, len pair leads to invalid memory access\n", + reg_arg_name(env, argno)); + if (failure == BPF_MEM_SIZE_FAIL_MEMORY) { + bpf_diag_call_arg_fmt(env, insn_idx, buff_argno, meta->func_name, + "Pass a stack, map, context, or other verifier-known memory pointer, and keep the paired length within that object.", + "it is the memory pointer in a memory/length pair with %s, but %s does not provide a verifier-accessible range of the requested length", + size_arg, buff_arg); + } else if (failure == BPF_MEM_SIZE_FAIL_SIZE) { + if (reg_smin(reg) < 0) + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Constrain the memory size to a non-negative value smaller than BPF_MAX_VAR_SIZ before this call.", + "the memory size in %s may be negative because its signed minimum is %lld", + size_arg, reg_smin(reg)); + else if (!zero_size_allowed && reg_umin(reg) == 0) + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Ensure the memory size is non-zero before this call.", + "the memory size in %s may be zero, but the function requires a non-zero size", + size_arg); + else + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Constrain the memory size to a non-negative value smaller than BPF_MAX_VAR_SIZ before this call.", + "the memory size in %s may reach %llu bytes, but variable memory accesses must stay below %u bytes", + size_arg, reg_umax(reg), BPF_MAX_VAR_SIZ); + } break; - case ARG_PTR_TO_DYNPTR: + } + case ARG_PTR_TO_DYNPTR: { + if (is_bpf_dynptr_clone_kfunc(meta) && + (arg_type & MEM_UNINIT)) { + enum bpf_dynptr_type parent_type = meta->dynptr.type; + + if (parent_type == BPF_DYNPTR_TYPE_INVALID) { + verifier_bug(env, "no dynptr type for parent of clone"); + return -EFAULT; + } + + arg_type |= (unsigned int)get_dynptr_type_flag(parent_type); + } + err = process_dynptr_func(env, reg, argno, insn_idx, arg_type, meta); if (err) return err; break; + } + case ARG_PTR_TO_ITER: + if (is_bpf_iter_css_task_new_kfunc(meta) && + !check_css_task_iter_allowlist(env)) { + verbose(env, "css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs\n"); + return -EINVAL; + } + err = process_iter_arg(env, reg, argno, insn_idx, meta); + if (err < 0) + return err; + break; + case ARG_PTR_TO_LIST_HEAD: + if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC) && + !reg_is_referenced(env, reg)) { + verbose(env, "allocated object must be referenced\n"); + return -EINVAL; + } + err = process_kf_arg_ptr_to_list_head(env, reg, argno, meta); + if (err < 0) + return err; + break; + case ARG_PTR_TO_RB_ROOT: + if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC) && + !reg_is_referenced(env, reg)) { + verbose(env, "allocated object must be referenced\n"); + return -EINVAL; + } + err = process_kf_arg_ptr_to_rbtree_root(env, reg, argno, meta); + if (err < 0) + return err; + break; + case ARG_PTR_TO_LIST_NODE: + if (!(is_kfunc_arg_nonown_allowed(meta->btf, btf_arg) && + type_is_non_owning_ref(reg->type) && !reg_is_referenced(env, reg))) { + if (reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { + verbose(env, "%s expected pointer to allocated object\n", + reg_arg_name(env, argno)); + return -EINVAL; + } + if (!reg_is_referenced(env, reg)) { + verbose(env, "allocated object must be referenced\n"); + return -EINVAL; + } + } + err = process_kf_arg_ptr_to_list_node(env, reg, argno, meta); + if (err < 0) + return err; + break; + case ARG_PTR_TO_RB_NODE: + if (is_bpf_rbtree_add_kfunc(meta->func_id)) { + if (reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { + verbose(env, "%s expected pointer to allocated object\n", + reg_arg_name(env, argno)); + return -EINVAL; + } + if (!reg_is_referenced(env, reg)) { + verbose(env, "allocated object must be referenced\n"); + return -EINVAL; + } + } else { + if (!type_is_non_owning_ref(reg->type) && + !reg_is_referenced(env, reg)) { + verbose(env, "%s can only take non-owning or refcounted bpf_rb_node pointer\n", + meta->func_name); + return -EINVAL; + } + if (in_rbtree_lock_required_cb(env)) { + verbose(env, "%s not allowed in rbtree cb\n", meta->func_name); + return -EINVAL; + } + } + err = process_kf_arg_ptr_to_rbtree_node(env, reg, argno, meta); + if (err < 0) + return err; + break; case ARG_CONST_ALLOC_SIZE_OR_ZERO: + if (meta->btf && is_kfunc_arg_scalar_with_name(meta->btf, btf_arg, + "rdonly_buf_size")) + meta->r0_rdonly = true; err = process_const_alloc_mem_size(env, reg, argno, &meta->ret_mem); - if (err) + if (err < 0) { + if (meta->btf && err == -EINVAL) + bpf_diag_call_arg_fmt(env, insn_idx, argno, meta->func_name, + "Pass a verifier-known constant size for this function's buffer argument.", + "the function uses this argument as a return-buffer size, but %s is invalid or variable on this path", + reg_arg_name(env, argno)); return err; + } + break; + case ARG_PTR_TO_REFCOUNTED_KPTR: + { + struct btf_record *rec; + + if (!type_is_non_owning_ref(reg->type) && reg_is_referenced(env, reg)) + meta->arg_owning_ref = true; + + rec = reg_btf_record(reg); + if (!rec) { + verifier_bug(env, "Couldn't find btf_record"); + return -EFAULT; + } + + if (rec->refcount_off < 0) { + verbose(env, "%s doesn't point to a type with bpf_refcount field\n", + reg_arg_name(env, argno)); + return -EINVAL; + } + + meta->arg_btf = reg->btf; + meta->arg_btf_id = reg->btf_id; break; + } case ARG_PTR_TO_CONST_STR: { err = check_arg_const_str(env, reg, argno); @@ -9100,6 +9406,38 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return err; break; } + case ARG_PTR_TO_WORKQUEUE: + err = check_map_field_pointer(env, reg, argno, BPF_WORKQUEUE, &meta->map); + if (err < 0) + return err; + break; + case ARG_PTR_TO_TASK_WORK: + err = check_map_field_pointer(env, reg, argno, BPF_TASK_WORK, &meta->map); + if (err < 0) + return err; + break; + case ARG_PTR_TO_IRQ_FLAG: + err = process_irq_flag(env, reg, argno, meta); + if (err < 0) + return err; + break; + case ARG_PTR_TO_RES_SPIN_LOCK: + { + int flags; + + if (in_rbtree_lock_required_cb(env)) { + verbose(env, "can't res_spin_{lock,unlock} in rbtree cb\n"); + return -EACCES; + } + + flags = get_bpf_res_spin_lock_kfunc_flags(meta); + if (!flags) + return -EFAULT; + err = process_spin_lock(env, reg, argno, flags); + if (err < 0) + return err; + break; + } case ARG_KPTR_XCHG_DEST: err = process_kptr_func(env, regno, meta); if (err) @@ -9110,6 +9448,37 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return err; } +static int check_func_args(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, + int insn_idx) +{ + struct bpf_func_state *caller = cur_func(env); + const struct btf_param *args = NULL; + u32 arg, nargs = MAX_BPF_FUNC_REG_ARGS; + int err; + + if (meta->btf) { + args = btf_params(meta->func_proto); + nargs = btf_type_vlen(meta->func_proto); + } + + if (nargs > MAX_BPF_FUNC_REG_ARGS) { + err = check_outgoing_stack_args(env, caller, nargs, meta->func_name, + meta->btf, args); + if (err) + return err; + } + + for (arg = 0; arg < nargs; arg++) { + if (meta->fn->arg_type[arg] == ARG_UNUSED) + break; + err = check_func_arg(env, arg, meta, insn_idx); + if (err) + return err; + } + + return 0; +} + static bool may_update_sockmap(struct bpf_verifier_env *env, int func_id) { enum bpf_attach_type eatype = env->prog->expected_attach_type; @@ -11116,14 +11485,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn meta.func_id = func_id; meta.func_name = func_id_name(func_id); meta.fn = fn; - /* check args */ - for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { - if (fn->arg_type[i] == ARG_UNUSED) - break; - err = check_func_arg(env, i, &meta, insn_idx); - if (err) - return err; - } + err = check_func_args(env, &meta, insn_idx); + if (err) + return err; err = record_func_map(env, &meta, func_id, insn_idx); if (err) @@ -12092,7 +12456,10 @@ enum special_kfunc_type { KF_bpf_task_work_schedule_resume, KF_bpf_arena_alloc_pages, KF_bpf_arena_free_pages, + KF_bpf_arena_reserve_pages, KF_bpf_session_is_return, + KF_bpf_stream_vprintk, + KF_bpf_stream_print_stack, }; BTF_ID_LIST(special_kfunc_list) @@ -12182,11 +12549,29 @@ BTF_ID(func, bpf_task_work_schedule_signal) BTF_ID(func, bpf_task_work_schedule_resume) BTF_ID(func, bpf_arena_alloc_pages) BTF_ID(func, bpf_arena_free_pages) +BTF_ID(func, bpf_arena_reserve_pages) #ifdef CONFIG_BPF_EVENTS BTF_ID(func, bpf_session_is_return) #else BTF_ID_UNUSED #endif +BTF_ID(func, bpf_stream_vprintk) +BTF_ID(func, bpf_stream_print_stack) + +static bool is_bpf_cast_to_kern_ctx_kfunc(const struct bpf_call_arg_meta *meta) +{ + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_cast_to_kern_ctx]); +} + +static bool is_bpf_dynptr_clone_kfunc(const struct bpf_call_arg_meta *meta) +{ + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_clone]); +} + +static bool is_bpf_iter_css_task_new_kfunc(const struct bpf_call_arg_meta *meta) +{ + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_iter_css_task_new]); +} static bool is_bpf_obj_new_kfunc(u32 func_id) { @@ -12777,12 +13162,20 @@ static bool is_bpf_rbtree_api_kfunc(u32 btf_id) btf_id == special_kfunc_list[KF_bpf_rbtree_right]; } -static bool is_bpf_res_spin_lock_kfunc(u32 btf_id) +static int get_bpf_res_spin_lock_kfunc_flags(const struct bpf_call_arg_meta *meta) { - return btf_id == special_kfunc_list[KF_bpf_res_spin_lock] || - btf_id == special_kfunc_list[KF_bpf_res_spin_unlock] || - btf_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave] || - btf_id == special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore]; + int flags = PROCESS_RES_LOCK; + + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock_irqsave])) + flags |= PROCESS_SPIN_LOCK; + else if (!is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_unlock]) && + !is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore])) + return 0; + if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock_irqsave]) || + is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore])) + flags |= PROCESS_LOCK_IRQ; + return flags; } static bool kfunc_spin_allowed(struct bpf_verifier_env *env, s32 func_id, s16 offset) @@ -13059,449 +13452,6 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env) } } -static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta, - int insn_idx) -{ - const char *func_name = meta->func_name; - struct bpf_func_state *caller = cur_func(env); - struct bpf_reg_state *regs = cur_regs(env); - const struct btf *btf = meta->btf; - const struct btf_param *args; - struct btf_record *rec; - u32 i, nargs; - int ret; - - args = (const struct btf_param *)(meta->func_proto + 1); - nargs = btf_type_vlen(meta->func_proto); - - ret = check_outgoing_stack_args(env, caller, nargs, func_name, btf, args); - if (ret) - return ret; - - /* Check that BTF function arguments match actual types that the - * verifier sees. - */ - for (i = 0; i < nargs; i++) { - struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i); - enum bpf_arg_type arg_type = meta->fn->arg_type[i]; - const struct btf_type *t; - argno_t argno = argno_from_arg(i + 1); - int regno = reg_from_argno(argno); - u32 ref_id = args[i].type; - u32 arg_size = arg_type & MEM_FIXED_SIZE ? meta->fn->arg_size[i] : 0; - - if (arg_type == ARG_PTR_TO_PROG_AUX) { - cur_aux(env)->arg_prog = regno; - continue; - } - - if (arg_type == ARG_IGNORE) - continue; - - t = btf_type_skip_modifiers(btf, args[i].type, NULL); - - if (btf_type_is_ptr(t)) - btf_type_skip_modifiers(btf, t->type, &ref_id); - - ret = check_func_arg_nullability(env, reg, argno, arg_type, meta, insn_idx); - if (ret < 0) - return ret; - - ret = check_func_arg_release(env, reg, argno, arg_type, meta, insn_idx); - if (ret < 0) - return ret; - - if (reg_is_referenced(env, reg)) - update_ref_obj(&meta->ref_obj, reg); - - if (bpf_register_is_null(reg) && type_may_be_null(arg_type)) { - ret = mark_arg_precision(env, argno); - if (ret) - return ret; - continue; - } - - if (base_type(arg_type) == ARG_PTR_TO_BTF_ID) - ref_id = *meta->fn->arg_btf_id[i]; - - ret = resolve_func_arg_type(env, reg, i, meta, &arg_type, &arg_size); - if (ret < 0) - return ret; - - ret = check_reg_type(env, reg, argno, arg_type, meta); - if (ret < 0) - return ret; - - ret = check_func_arg_reg_off(env, reg, argno, arg_type); - if (ret < 0) - return ret; - - switch (base_type(arg_type)) { - case ARG_CONST_SCALAR: - ret = process_const_arg(env, reg, argno, meta); - if (ret < 0) { - if (ret == -EINVAL) - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass a compile-time constant or a value the verifier can prove is constant at this call.", - "the kfunc requires this scalar argument to be a verifier-known constant, but %s is variable on this path", - reg_arg_name(env, argno)); - return ret; - } - break; - case ARG_SCALAR: - break; - case ARG_CONST_ALLOC_SIZE_OR_ZERO: - if (is_kfunc_arg_scalar_with_name(btf, &args[i], "rdonly_buf_size")) - meta->r0_rdonly = true; - ret = process_const_alloc_mem_size(env, reg, argno, &meta->ret_mem); - if (ret < 0) { - if (ret == -EINVAL) - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass a verifier-known constant size for this kfunc buffer argument.", - "the kfunc uses this argument as a return-buffer size, but %s is invalid or variable on this path", - reg_arg_name(env, argno)); - return ret; - } - break; - case ARG_PTR_TO_CTX: - if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_cast_to_kern_ctx])) { - ret = get_kern_ctx_btf_id(&env->log, resolve_prog_type(env->prog)); - if (ret < 0) - return -EINVAL; - meta->ret_btf_id = ret; - } - break; - case ARG_PTR_TO_ARENA: - break; - case ARG_PTR_TO_ALLOC_BTF_ID: - if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC)) { - if (!is_bpf_obj_drop_kfunc(meta->func_id)) { - verbose(env, "%s expected for bpf_obj_drop()\n", - reg_arg_name(env, argno)); - return -EINVAL; - } - } else if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC | MEM_PERCPU)) { - if (!is_bpf_percpu_obj_drop_kfunc(meta->func_id)) { - verbose(env, "%s expected for bpf_percpu_obj_drop()\n", - reg_arg_name(env, argno)); - return -EINVAL; - } - } - if (!reg_is_referenced(env, reg)) { - verbose(env, "allocated object must be referenced\n"); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass the owned object pointer before it is released or transferred.", - "the allocated object pointer in %s must still carry verifier-tracked ownership, but this pointer no longer owns a live resource", - reg_arg_name(env, argno)); - return -EINVAL; - } - if (meta->btf == btf_vmlinux) { - meta->arg_btf = reg->btf; - meta->arg_btf_id = reg->btf_id; - } - break; - case ARG_PTR_TO_DYNPTR: - { - enum bpf_arg_type dynptr_arg_type = arg_type; - - if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_dynptr_clone]) && - (dynptr_arg_type & MEM_UNINIT)) { - enum bpf_dynptr_type parent_type = meta->dynptr.type; - - if (parent_type == BPF_DYNPTR_TYPE_INVALID) { - verifier_bug(env, "no dynptr type for parent of clone"); - return -EFAULT; - } - - dynptr_arg_type |= (unsigned int)get_dynptr_type_flag(parent_type); - } - - ret = process_dynptr_func(env, reg, argno, insn_idx, - dynptr_arg_type, meta); - if (ret < 0) - return ret; - break; - } - case ARG_PTR_TO_ITER: - if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_iter_css_task_new])) { - if (!check_css_task_iter_allowlist(env)) { - verbose(env, "css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs\n"); - return -EINVAL; - } - } - ret = process_iter_arg(env, reg, argno, insn_idx, meta); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_LIST_HEAD: - if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC) && - !reg_is_referenced(env, reg)) { - verbose(env, "allocated object must be referenced\n"); - return -EINVAL; - } - ret = process_kf_arg_ptr_to_list_head(env, reg, argno, meta); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_RB_ROOT: - if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC) && - !reg_is_referenced(env, reg)) { - verbose(env, "allocated object must be referenced\n"); - return -EINVAL; - } - ret = process_kf_arg_ptr_to_rbtree_root(env, reg, argno, meta); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_LIST_NODE: - if (is_kfunc_arg_nonown_allowed(btf, &args[i]) && - type_is_non_owning_ref(reg->type) && !reg_is_referenced(env, reg)) { - /* Allow bpf_list_front/back return value for - * __nonown_allowed list-node arguments. - */ - goto check_ok; - } - if (reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { - verbose(env, "%s expected pointer to allocated object\n", - reg_arg_name(env, argno)); - return -EINVAL; - } - if (!reg_is_referenced(env, reg)) { - verbose(env, "allocated object must be referenced\n"); - return -EINVAL; - } -check_ok: - ret = process_kf_arg_ptr_to_list_node(env, reg, argno, meta); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_RB_NODE: - if (is_bpf_rbtree_add_kfunc(meta->func_id)) { - if (reg->type != (PTR_TO_BTF_ID | MEM_ALLOC)) { - verbose(env, "%s expected pointer to allocated object\n", - reg_arg_name(env, argno)); - return -EINVAL; - } - if (!reg_is_referenced(env, reg)) { - verbose(env, "allocated object must be referenced\n"); - return -EINVAL; - } - } else { - if (!type_is_non_owning_ref(reg->type) && - !reg_is_referenced(env, reg)) { - verbose(env, "%s can only take non-owning or refcounted bpf_rb_node pointer\n", func_name); - return -EINVAL; - } - if (in_rbtree_lock_required_cb(env)) { - verbose(env, "%s not allowed in rbtree cb\n", func_name); - return -EINVAL; - } - } - - ret = process_kf_arg_ptr_to_rbtree_node(env, reg, argno, meta); - if (ret < 0) - return ret; - break; - case ARG_CONST_MAP_PTR: - ret = process_map_ptr_arg(env, reg, argno, meta); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_BTF_ID: - if (!is_trusted_reg(env, reg) || - bpf_type_has_unsafe_modifiers(reg->type)) { - if (!(arg_type & MEM_RCU)) { - const char *actual_type, *arg_name, *expected_type; - - expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); - verbose(env, "%s must be referenced or trusted\n", - reg_arg_name(env, argno)); - arg_name = reg_arg_name(env, argno); - actual_type = bpf_diag_reg_type_plain(env, reg->type); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass a pointer acquired from a verifier-tracked source, or call this kfunc only inside the required protection if it accepts RCU pointers.", - "the kfunc requires a trusted or resource-owning pointer to %s, but %s is %s", - expected_type, arg_name, actual_type); - return -EINVAL; - } - if (!is_rcu_reg(reg)) { - const char *actual_type, *arg_name, *expected_type; - - expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); - verbose(env, "%s must be a rcu pointer\n", - reg_arg_name(env, argno)); - arg_name = reg_arg_name(env, argno); - actual_type = bpf_diag_reg_type_plain(env, reg->type); - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Use this kfunc with a pointer that is valid in an RCU read lock region.", - "the kfunc requires an RCU-protected pointer to %s, but %s is %s", - expected_type, arg_name, actual_type); - return -EINVAL; - } - } - - ret = process_arg_ptr_to_btf_id(env, reg, argno, arg_type, btf, - ref_id, meta, insn_idx); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_MEM: - if (arg_type & MEM_FIXED_SIZE) { - bool known_memory; - - ret = check_mem_reg(env, reg, argno, arg_size, BPF_READ | BPF_WRITE, - meta, &known_memory); - if (ret < 0) { - const char *expected_type; - - expected_type = bpf_diag_fmt_btf_type(env, btf, ref_id); - if (known_memory) - bpf_diag_call_arg_fmt( - env, insn_idx, argno, func_name, - "Pass memory with at least the required number of accessible bytes and suitable read and write access.", - "the kfunc expects %u bytes of memory for %s, but the verifier cannot prove that %s provides a readable and writable range of that size", - arg_size, expected_type, - bpf_diag_reg_type_plain(env, reg->type)); - else - bpf_diag_call_arg_fmt( - env, insn_idx, argno, func_name, - "Pass stack, map, context, or other verifier-known memory of the expected type and size, not an integer cast to a pointer.", - "the kfunc expects %u bytes of memory for %s, but it is %s and not verifier-known memory", - arg_size, expected_type, - bpf_diag_reg_type_plain(env, reg->type)); - return ret; - } - } - break; - case ARG_CONST_MEM_SIZE: - ret = process_const_arg(env, reg, argno, meta); - if (ret < 0) { - if (ret == -EINVAL) - bpf_diag_call_arg_fmt(env, insn_idx, argno, func_name, - "Pass a compile-time constant or a value the verifier can prove is constant at this call.", - "the kfunc requires this memory size to be a verifier-known constant, but %s is variable on this path", - reg_arg_name(env, argno)); - return ret; - } - fallthrough; - case ARG_MEM_SIZE: - { - struct bpf_reg_state *buff_reg = get_func_arg_reg(caller, regs, i - 1); - struct bpf_reg_state *size_reg = reg; - argno_t buff_argno = argno_from_arg(i); - enum bpf_mem_size_failure failure; - - if (bpf_register_is_null(buff_reg)) - break; - - ret = check_mem_size_reg(env, buff_reg, size_reg, buff_argno, argno, - BPF_READ | BPF_WRITE, true, meta, &failure); - if (ret < 0) { - const char *buff_arg, *size_arg; - - buff_arg = bpf_diag_arg_name(env, buff_argno); - size_arg = bpf_diag_arg_name(env, argno); - verbose(env, "%s and ", reg_arg_name(env, buff_argno)); - verbose(env, "%s memory, len pair leads to invalid memory access\n", - reg_arg_name(env, argno)); - if (failure == BPF_MEM_SIZE_FAIL_MEMORY) { - bpf_diag_call_arg_fmt(env, insn_idx, buff_argno, func_name, - "Pass a stack, map, context, or other verifier-known memory pointer, and keep the paired length within that object.", - "it is the memory pointer in a memory/length pair with %s, but %s does not describe verifier-readable memory for the requested length", - size_arg, buff_arg); - } else if (failure == BPF_MEM_SIZE_FAIL_SIZE) { - if (reg_smin(size_reg) < 0) - bpf_diag_call_arg_fmt( - env, insn_idx, argno, func_name, - "Constrain the memory size to a non-negative value smaller than BPF_MAX_VAR_SIZ before this call.", - "the memory size in %s may be negative because its signed minimum is %lld", - size_arg, reg_smin(size_reg)); - else - bpf_diag_call_arg_fmt( - env, insn_idx, argno, func_name, - "Constrain the memory size to a non-negative value smaller than BPF_MAX_VAR_SIZ before this call.", - "the memory size in %s may reach %llu bytes, but variable memory accesses must stay below %u bytes", - size_arg, reg_umax(size_reg), BPF_MAX_VAR_SIZ); - } - return ret; - } - break; - } - case ARG_PTR_TO_FUNC: - meta->subprogno = reg->subprogno; - break; - case ARG_PTR_TO_REFCOUNTED_KPTR: - if (!type_is_non_owning_ref(reg->type) && reg_is_referenced(env, reg)) - meta->arg_owning_ref = true; - - rec = reg_btf_record(reg); - if (!rec) { - verifier_bug(env, "Couldn't find btf_record"); - return -EFAULT; - } - - if (rec->refcount_off < 0) { - verbose(env, "%s doesn't point to a type with bpf_refcount field\n", - reg_arg_name(env, argno)); - return -EINVAL; - } - - meta->arg_btf = reg->btf; - meta->arg_btf_id = reg->btf_id; - break; - case ARG_PTR_TO_CONST_STR: - ret = check_arg_const_str(env, reg, argno); - if (ret) - return ret; - break; - case ARG_PTR_TO_WORKQUEUE: - ret = check_map_field_pointer(env, reg, argno, BPF_WORKQUEUE, &meta->map); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_TIMER: - ret = process_timer_func(env, reg, argno, &meta->map); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_TASK_WORK: - ret = check_map_field_pointer(env, reg, argno, BPF_TASK_WORK, &meta->map); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_IRQ_FLAG: - ret = process_irq_flag(env, reg, argno, meta); - if (ret < 0) - return ret; - break; - case ARG_PTR_TO_RES_SPIN_LOCK: - { - int flags = PROCESS_RES_LOCK; - - if (in_rbtree_lock_required_cb(env)) { - verbose(env, "can't res_spin_{lock,unlock} in rbtree cb\n"); - return -EACCES; - } - if (!is_bpf_res_spin_lock_kfunc(meta->func_id)) - return -EFAULT; - if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock]) || - is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock_irqsave])) - flags |= PROCESS_SPIN_LOCK; - if (is_kfunc_call(meta, special_kfunc_list[KF_bpf_res_spin_lock_irqsave]) || - is_kfunc_call(meta, - special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore])) - flags |= PROCESS_LOCK_IRQ; - ret = process_spin_lock(env, reg, argno, flags); - if (ret < 0) - return ret; - break; - } - } - } - - return 0; -} - int bpf_fetch_kfunc_arg_meta(struct bpf_verifier_env *env, s32 func_id, s16 offset, @@ -13993,7 +13943,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, insn_aux->non_sleepable = true; /* Check the arguments */ - err = check_kfunc_args(env, &meta, insn_idx); + err = check_func_args(env, &meta, insn_idx); if (err < 0) return err; diff --git a/tools/testing/selftests/bpf/prog_tests/cb_refs.c b/tools/testing/selftests/bpf/prog_tests/cb_refs.c index e415acc08b16..c32c6dab49bc 100644 --- a/tools/testing/selftests/bpf/prog_tests/cb_refs.c +++ b/tools/testing/selftests/bpf/prog_tests/cb_refs.c @@ -11,7 +11,7 @@ struct { const char *prog_name; const char *err_msg; } cb_refs_tests[] = { - { "underflow_prog", "release function bpf_kfunc_call_test_release expects referenced PTR_TO_BTF_ID passed to R1" }, + { "underflow_prog", "R1 type=scalar expected=ptr_, trusted_ptr_, rcu_ptr_" }, { "leak_prog", "Unreleased reference id=4 alloc_insn=3" }, /* alloc_insn=3{2,3} */ { "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} */ diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c index 731eaa09db96..ede6a17d7da3 100644 --- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c @@ -191,7 +191,7 @@ int BPF_PROG(cgrp_kfunc_release_untrusted, struct cgroup *cgrp, const char *path } SEC("tp_btf/cgroup_mkdir") -__failure __msg("release function bpf_cgroup_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("R1 type=fp expected=ptr_, trusted_ptr_, rcu_ptr_") int BPF_PROG(cgrp_kfunc_release_fp, struct cgroup *cgrp, const char *path) { struct cgroup *acquired = (struct cgroup *)&path; diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c index 0ca2e1a38a94..12c8ac6099ca 100644 --- a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c @@ -179,7 +179,7 @@ int BPF_PROG(task_kfunc_release_untrusted, struct task_struct *task, u64 clone_f } SEC("tp_btf/task_newtask") -__failure __msg("release function bpf_task_release expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("R1 type=fp expected=ptr_, trusted_ptr_, rcu_ptr_") int BPF_PROG(task_kfunc_release_fp, struct task_struct *task, u64 clone_flags) { struct task_struct *acquired = (struct task_struct *)&clone_flags; diff --git a/tools/testing/selftests/bpf/progs/wakeup_source_fail.c b/tools/testing/selftests/bpf/progs/wakeup_source_fail.c index ff9ea5aa80ad..ec4e0f3ff792 100644 --- a/tools/testing/selftests/bpf/progs/wakeup_source_fail.c +++ b/tools/testing/selftests/bpf/progs/wakeup_source_fail.c @@ -42,7 +42,7 @@ int wakeup_source_access_lock_fields(void *ctx) } SEC("syscall") -__failure __msg("release function bpf_wakeup_sources_read_unlock expects referenced PTR_TO_BTF_ID passed to R1") +__failure __msg("R1 type=scalar expected=ptr_, trusted_ptr_, rcu_ptr_") int wakeup_source_unlock_no_lock(void *ctx) { struct bpf_ws_lock *lock = (void *)0x1; -- 2.52.0