diff options
| -rw-r--r-- | include/linux/bpf_verifier.h | 3 | ||||
| -rw-r--r-- | kernel/bpf/verifier.c | 58 | ||||
| -rw-r--r-- | tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c | 3 | ||||
| -rw-r--r-- | tools/testing/selftests/bpf/progs/verifier_global_subprogs.c | 2 |
4 files changed, 33 insertions, 33 deletions
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 183759f1e014..811342e3c041 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1656,6 +1656,9 @@ struct bpf_call_arg_meta { struct ret_mem_desc ret_mem; struct arg_raw_mem_desc arg_raw_mem; + /* Only set by subprog */ + bool subprog_may_change_pkt; + /* Only set by kfunc */ bool r0_rdonly; u32 kfunc_flags; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 8aa1336453a4..ad18b6ab7e88 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -7703,6 +7703,11 @@ size_error: return err; } +static bool is_subprog(const struct bpf_call_arg_meta *meta) +{ + return meta->btf && !meta->func_id; +} + static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, u32 mem_size, enum bpf_access_type access_type, struct bpf_call_arg_meta *meta, bool *known_memory) @@ -7721,10 +7726,11 @@ static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg } /* - * Only a global subprog (meta == NULL) may read poisoned stack slots: + * Only a global subprog may read poisoned stack slots: * its static stack liveness proved the callee body skips them. */ - size = (!meta && base_type(reg->type) == PTR_TO_STACK) ? -(int)mem_size : mem_size; + size = (is_subprog(meta) && + base_type(reg->type) == PTR_TO_STACK) ? -(int)mem_size : mem_size; if (access_type & BPF_READ) err = check_helper_mem_access(env, reg, argno, size, BPF_READ, true, meta, @@ -9036,8 +9042,8 @@ 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 (is_kfunc(meta) && base_type(arg_type) == ARG_PTR_TO_MEM && + /* Allow allocated memory for BTF-defined ARG_PTR_TO_MEM but not helpers. */ + if (!is_helper(meta) && base_type(arg_type) == ARG_PTR_TO_MEM && type_is_ptr_alloc_obj(type)) type = PTR_TO_MEM; @@ -9667,6 +9673,17 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, u32 slot, u32 p bpf_diag_reg_type_plain(env, reg->type)); return err; } + /* + * PTR_TO_PACKET gets passed as PTR_TO_MEM, preventing us from adjusting + * bounds tracking information. + */ + if (is_subprog(meta) && meta->subprog_may_change_pkt && + (reg_is_pkt_pointer_any(reg) || reg_is_dynptr_slice_pkt(reg))) { + verbose(env, + "cannot pass packet pointer %s to %s(): function may change packet data\n", + reg_arg_name(env, argno), meta->func_name); + return -EINVAL; + } if (arg_type & MEM_ALIGNED) err = check_ptr_alignment(env, reg, 0, arg_size, true); break; @@ -10806,6 +10823,9 @@ static void gen_subprog_arg_proto(const struct bpf_subprog_info *sub, const stru arg_type = ARG_IGNORE; } else if (base_type(arg_type) == ARG_PTR_TO_ARENA) { arg_type |= PTR_MAYBE_NULL; + } else if (base_type(arg_type) == ARG_PTR_TO_MEM) { + proto->arg_size[arg] = sub->args[slot].mem_size; + arg_type |= MEM_FIXED_SIZE | MEM_WRITE; } else if (base_type(arg_type) == ARG_PTR_TO_BTF_ID) { proto->arg_btf_id[arg] = &sub->args[slot].btf_id; } @@ -10821,7 +10841,6 @@ 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; const struct btf_param *args, *stack_args; const struct btf_type *func, *func_proto; struct bpf_call_arg_meta meta; @@ -10831,6 +10850,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, memset(&meta, 0, sizeof(meta)); meta.btf = btf; + meta.subprog_may_change_pkt = sub->changes_pkt_data; meta.func_name = bpf_subprog_name(env, subprog); ret = btf_prepare_func_args(env, subprog); @@ -10863,7 +10883,6 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, * verifier sees. */ for (arg = 0, slot = 0; arg < btf_type_vlen(func_proto); arg++, slot += nslots) { - struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, slot); enum bpf_arg_type arg_type = fn->arg_type[arg]; argno_t argno = argno_from_arg(slot + 1); const struct btf_type *t; @@ -10875,34 +10894,11 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, if (arg_type == ARG_SCALAR || arg_type == ARG_IGNORE || arg_type == ARG_PTR_TO_CTX || arg_type == ARG_PTR_TO_DYNPTR || base_type(arg_type) == ARG_PTR_TO_ARENA || - base_type(arg_type) == ARG_PTR_TO_BTF_ID) { + base_type(arg_type) == ARG_PTR_TO_BTF_ID || + base_type(arg_type) == ARG_PTR_TO_MEM) { ret = check_func_arg(env, arg, slot, 0, &meta, env->insn_idx); if (ret) return ret; - } else if (base_type(arg_type) == ARG_PTR_TO_MEM) { - 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, sub->args[slot].mem_size, - BPF_READ | BPF_WRITE, NULL, - NULL)) - return -EINVAL; - /* - * PTR_TO_PACKET get passed as PTR_TO_MEM, preventing - * us from adjusting bounds tracking info. - */ - if ((reg_is_pkt_pointer_any(reg) || reg_is_dynptr_slice_pkt(reg)) && - sub->changes_pkt_data) { - bpf_log(log, "%s is a packet pointer, but func#%d may change packet data\n", - reg_arg_name(env, argno), subprog); - return -EINVAL; - } - if (!(arg_type & PTR_MAYBE_NULL) && - (type_may_be_null(reg->type) || bpf_register_is_null(reg))) { - bpf_log(log, "%s is expected to be non-NULL\n", - reg_arg_name(env, argno)); - return -EINVAL; - } } else { verifier_bug(env, "unrecognized %s type %d", reg_arg_name(env, argno), arg_type); 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 f639e2767e35..03507eeae3cb 100644 --- a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c +++ b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c @@ -389,7 +389,8 @@ __weak int subprog_pkt_ptr_changes_data(struct __sk_buff *skb __arg_ctx, SEC("?tc") __failure __log_level(2) -__msg("R2 is a packet pointer, but func#{{[0-9]+}} may change packet data") +__msg("cannot pass packet pointer R2") +__msg("function may change packet data") __msg("Caller passes invalid args into func#{{[0-9]+}} ('subprog_pkt_ptr_changes_data')") int pkt_ptr_to_global_mem_arg_changes_data(struct __sk_buff *skb) { diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c index 27fbe54e8795..574b26b5a7df 100644 --- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c +++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c @@ -195,7 +195,7 @@ int arg_tag_nonnull_ptr_good(void *ctx) SEC("?raw_tp") __failure __log_level(2) -__msg("R1 is expected to be non-NULL") +__msg("Possibly NULL pointer passed to trusted R1") int arg_tag_nonnull_ptr_null_bad(void *ctx) { int y = 74; |