To prepare for unifying the helper and kfunc call_arg_meta, group the scattered MEM_UNINIT ("raw") memory argument fields (raw_mode, regno and access_size) into a new struct arg_raw_mem_desc. The intention is to make it clear about when these are set and used instead of fields with overly generic names. Identify the raw argument once, up front, in check_raw_mode_ok() (like check_proto_release_reg() does for release_regno), recording its regno. check_stack_range_initialized() now recognizes the raw buffer by matching that regno, so the separate raw_mode flag is no longer needed, and the per-argument "meta->raw_mode = arg_type & MEM_UNINIT" assignments in check_func_arg() go away with it. A raw memory argument can be tagged either ARG_PTR_TO_MEM | MEM_UNINIT or ARG_PTR_TO_MAP_VALUE | MEM_UNINIT (the output buffer of bpf_map_pop_elem() and bpf_map_peek_elem()). Either may be passed as a PTR_TO_STACK, which reaches check_stack_range_initialized() through check_helper_mem_access(), so both must be treated as raw. Extend arg_type_is_raw_mem() to match the map value case as well; otherwise check_raw_mode_ok() would not record the regno for it and an uninitialized stack buffer passed to those helpers would be wrongly rejected for programs without CAP_PERFMON. No functional change intended. This patch does not enable raw_mode memory access for kfunc (i.e., uninit stack will not be allowed to be passed to kfunc for unprivileged programs). Existing kfuncs with arguments tagged with __uninit are either priviledged or dynptr kfuncs, which take another path to make sure the access is checked by check_mem_access(). Acked-by: Eduard Zingerman Signed-off-by: Amery Hung --- include/linux/bpf_verifier.h | 9 ++++++ kernel/bpf/verifier.c | 58 +++++++++++++++++------------------- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 317e99b9acc0..d3592f5b8621 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1464,6 +1464,15 @@ struct ref_obj_desc { u8 cnt; }; +/* + * A memory argument a call fills in. The verifier allows the stack to be uninitialized if + * the range is a known constant. Stack slots are marked as STACK_MISC by check_mem_access(). + */ +struct arg_raw_mem_desc { + u8 regno; + int size; +}; + struct bpf_kfunc_call_arg_meta { /* In parameters */ struct btf *btf; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 7dd961ede88d..1a41f99a9133 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -256,11 +256,9 @@ struct bpf_call_arg_meta { struct bpf_map_desc map; struct bpf_dynptr_desc dynptr; struct ref_obj_desc ref_obj; - bool raw_mode; + struct arg_raw_mem_desc arg_raw_mem; bool pkt_access; u8 release_regno; - int regno; - int access_size; int mem_size; u64 msize_max_value; int func_id; @@ -6690,6 +6688,8 @@ static int check_stack_range_initialized( * but BTF based global subprog validation isn't accurate enough. */ bool allow_poison = access_size < 0 || clobber; + /* The call will initialize the memory; uninitialized stack allowed */ + bool raw_mode = meta && meta->arg_raw_mem.regno == reg_from_argno(argno); access_size = abs(access_size); @@ -6725,16 +6725,14 @@ static int check_stack_range_initialized( * helper return since specific bounds are unknown what may * cause uninitialized stack leaking. */ - if (meta && meta->raw_mode) - meta = NULL; + raw_mode = false; min_off = reg_smin(reg) + off; max_off = reg_smax(reg) + off; } - if (meta && meta->raw_mode) { - meta->access_size = access_size; - meta->regno = reg_from_argno(argno); + if (raw_mode) { + meta->arg_raw_mem.size = access_size; return 0; } @@ -7727,7 +7725,13 @@ static bool arg_type_is_mem_size(enum bpf_arg_type type) static bool arg_type_is_raw_mem(enum bpf_arg_type type) { - return base_type(type) == ARG_PTR_TO_MEM && + /* + * A map value output buffer (e.g. bpf_map_pop_elem) is also a raw + * (uninitialized) memory argument, and like ARG_PTR_TO_MEM it may be + * passed as a PTR_TO_STACK that reaches check_stack_range_initialized(). + */ + return (base_type(type) == ARG_PTR_TO_MEM || + base_type(type) == ARG_PTR_TO_MAP_VALUE) && type & MEM_UNINIT; } @@ -8403,7 +8407,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, verifier_bug(env, "invalid map_ptr to access map->value"); return -EFAULT; } - meta->raw_mode = arg_type & MEM_UNINIT; err = check_helper_mem_access(env, reg, argno_from_reg(regno), meta->map.ptr->value_size, arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ, false, meta); @@ -8446,7 +8449,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, /* The access to this pointer is only checked when we hit the * next is_mem_size argument below. */ - meta->raw_mode = arg_type & MEM_UNINIT; if (arg_type & MEM_FIXED_SIZE) { err = check_helper_mem_access(env, reg, argno_from_reg(regno), fn->arg_size[arg], arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ, @@ -8797,26 +8799,19 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env, return -EINVAL; } -static bool check_raw_mode_ok(const struct bpf_func_proto *fn) +static bool check_raw_mode_ok(const struct bpf_func_proto *fn, struct bpf_call_arg_meta *meta) { - int count = 0; + int i; - if (arg_type_is_raw_mem(fn->arg1_type)) - count++; - if (arg_type_is_raw_mem(fn->arg2_type)) - count++; - if (arg_type_is_raw_mem(fn->arg3_type)) - count++; - if (arg_type_is_raw_mem(fn->arg4_type)) - count++; - if (arg_type_is_raw_mem(fn->arg5_type)) - count++; + for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) { + if (!arg_type_is_raw_mem(fn->arg_type[i])) + continue; + if (meta->arg_raw_mem.regno) + return false; + meta->arg_raw_mem.regno = i + 1; + } - /* We only support one arg being in raw mode at the moment, - * which is sufficient for the helper functions we have - * right now. - */ - return count <= 1; + return true; } static bool check_args_pair_invalid(const struct bpf_func_proto *fn, int arg) @@ -8906,7 +8901,7 @@ static bool check_proto_release_reg(const struct bpf_func_proto *fn, struct bpf_ static int check_func_proto(const struct bpf_func_proto *fn, struct bpf_call_arg_meta *meta) { - return check_raw_mode_ok(fn) && + return check_raw_mode_ok(fn, meta) && check_arg_pair_ok(fn) && check_mem_arg_rw_flag_ok(fn) && check_proto_release_reg(fn, meta) && @@ -10303,8 +10298,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn /* Mark slots with STACK_MISC in case of raw mode, stack offset * is inferred from register state. */ - for (i = 0; i < meta.access_size; i++) { - err = check_mem_access(env, insn_idx, regs + meta.regno, argno_from_reg(meta.regno), i, BPF_B, + for (i = 0; i < meta.arg_raw_mem.size; i++) { + err = check_mem_access(env, insn_idx, regs + meta.arg_raw_mem.regno, + argno_from_reg(meta.arg_raw_mem.regno), i, BPF_B, BPF_WRITE, -1, false, false); if (err) return err; -- 2.52.0