From: Anton Protopopov SEC("fmod_ret/vfs_fallocate") int BPF_PROG(no_big_files, struct file *file, int mode, loff_t offset, loff_t len, int ret) { return len > MAX_LEN ? -EFBIG : 0; } This program keeps user space away from a bug below vfs_fallocate() until the fixed kernel runs. It does not load: "vfs_fallocate() is not modifiable". The build has tagged the function with the errors that it returns, the list: "bpf_fmodret_retval:-95,-29,-27,-26,-22,-21,-19,-9,-1". Let an fmod_ret program that is not sleepable attach to a tagged function. The declared targets stay as they are, without a restriction. A caller of a tagged function can get each error of the list from the function itself. The build knows nothing about other values: R0 at an exit the trampoline 0 calls the function an error of the list skips it, the caller gets the error another value -, the program does not load With -EDQUOT in place of -EFBIG: At program exit the register R0 should be 0 or one of -95,-29,-27,.. The check is static: every value in the range of R0 is 0 or in the list. [-27, 0] has -25, which is not in the list. The program above gets to the exit in two states, R0 = -27 and R0 = 0. The verifier tests R0 at the exit, so flow_join_makes_up() does not join ranges that leave a gap. R0 has a 64-bit and a 32-bit range. clang ends "return a && b ? -EPERM : 0" with "w0 &= 1; w0 = -w0; exit". That is [0, U32_MAX] and [-1, 0]. target returns range R0 at the exit int 32-bit w0 = -22 -22, allowed r0 = 1 << 32 rejected long 64-bit w0 = -22 4294967274, rejected The caller of an int function reads the low 32 bits. The trampoline tests all 64 bits: 1 << 32 skips the function and the caller sees 0. So the 32-bit range decides only if the high 32 bits are known to be 0. That fits the ABI of x86-64 and arm64, see the comment. What returns to the trampoline exit of the main program, of an exception callback checked cookie of bpf_throw(), default callback checked tail call refused extension of the main program refused by existing checks Signed-off-by: Anton Protopopov Co-developed-by: Alexei Starovoitov Signed-off-by: Alexei Starovoitov --- include/linux/bpf.h | 2 + include/linux/bpf_verifier.h | 2 + kernel/bpf/verifier.c | 83 ++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 520fd72e21fd..ee26185557ef 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1542,6 +1542,8 @@ struct bpf_attach_target_info { struct module *tgt_mod; const char *tgt_name; const struct btf_type *tgt_type; + /* The errors in the "bpf_fmodret_retval:" tag, if that is what allows fmod_ret */ + const char *fmodret_retvals; }; #define BPF_DISPATCHER_MAX 48 /* Fits in 2048B */ diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index d4fdb724cc1b..8a3cded313f4 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1359,6 +1359,8 @@ struct bpf_verifier_env { char tmp_arg_name[32]; struct bpf_insn insn_buf[INSN_BUF_SIZE]; struct bpf_insn epilogue_buf[INSN_BUF_SIZE]; + /* See check_fmodret_retval() */ + const char *fmodret_retvals; struct bpf_liveness *liveness; struct bpf_iarray *succ; struct bpf_iarray *gotox_tmp_buf; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 56924ce392e0..3ee90c2bb1ec 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -13531,6 +13531,14 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn } if (func_id == BPF_FUNC_tail_call) { + /* + * The program which a tail call jumps to returns for this one, + * and a program array does not check the values it may return. + */ + if (env->fmodret_retvals) { + verbose(env, "tail calls are not allowed in fmod_ret programs with restricted return values\n"); + return -EINVAL; + } if (env->cur_state->curframe) { struct bpf_verifier_state *branch; @@ -20209,6 +20217,66 @@ static bool program_returns_void(struct bpf_verifier_env *env) return false; } +static bool fmodret_retval_allowed(const char *list, s64 value) +{ + char *end; + + if (!value) + return true; + for (; *list; list = *end ? end + 1 : end) { + if (simple_strtoll(list, &end, 10) == value) + return true; + } + return false; +} + +/* + * The build tags the functions which return errors: "bpf_fmodret_retval:-22,-12", + * see tools/bpf/resolve_btfids/fmodret.c. An fmod_ret program can attach to such + * a function. A caller can get each of these errors today: the function returns + * them itself. So the program must return one of them, or 0 to call the function. + */ +static int check_fmodret_retval(struct bpf_verifier_env *env, int regno, + const char *exit_ctx, const char *reg_name) +{ + const struct bpf_prog_aux *aux = env->prog->aux; + struct bpf_reg_state *reg = reg_state(env, regno); + s64 smin = reg_smin(reg), smax = reg_smax(reg), v; + const struct btf_type *t; + + if (reg->type != SCALAR_VALUE) { + verbose(env, "%s the register %s is not a known value (%s)\n", + exit_ctx, reg_name, reg_type_str(env, reg->type)); + return -EINVAL; + } + + /* + * "w0 = -22" zero-extends, the caller of an int function sees -22. The + * trampoline tests all 64 bits: 1 << 32 would skip the function, and the + * caller would see 0. So the low 32 bits decide only if the high ones are 0. + * + * That is right where the ABI leaves the high 32 bits of an int in the + * return value register open: x86-64, arm64. s390x, riscv64, ppc64 and + * loongarch want it sign-extended: look here before one of them is added + * to CONFIG_BPF_FMODRET_SCAN. + */ + t = btf_type_skip_modifiers(aux->attach_btf, aux->attach_func_proto->type, NULL); + if (t->size == 4 && reg_umax(reg) <= U32_MAX) { + smin = reg_s32_min(reg); + smax = reg_s32_max(reg); + } + + /* Stops at the first value which is not in the list, however wide the range is */ + for (v = smin; v <= smax; v++) { + if (!fmodret_retval_allowed(env->fmodret_retvals, v)) { + verbose(env, "%s the register %s should be 0 or one of %s\n", + exit_ctx, reg_name, env->fmodret_retvals); + return -EINVAL; + } + } + return 0; +} + static int check_return_code(struct bpf_verifier_env *env, int regno, const char *reg_name) { const char *exit_ctx = "At program exit"; @@ -20265,6 +20333,9 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char if (prog_type == BPF_PROG_TYPE_CGROUP_SKB && (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)) enforce_attach_type_range = tnum_range(2, 3); + if (env->fmodret_retvals) + return check_fmodret_retval(env, regno, exit_ctx, reg_name); + if (!return_retval_range(env, &range)) return 0; @@ -24260,6 +24331,16 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, if (btf_kfunc_is_modify_return(btf, btf_id, prog) || !check_attach_modify_return(addr, tname)) ret = 0; + if (ret) { + /* Nobody declared it as a target. Has the build tagged it? */ + tgt_info->fmodret_retvals = + btf_find_decl_tag_value(btf, btf_type_by_id(btf, btf_id), -1, + "bpf_fmodret_retval:"); + if (IS_ERR(tgt_info->fmodret_retvals)) + tgt_info->fmodret_retvals = NULL; + else + ret = 0; + } if (ret) { module_put(mod); bpf_log(log, "%s() is not modifiable\n", tname); @@ -24438,6 +24519,8 @@ static int check_attach_btf_id(struct bpf_verifier_env *env) return -EINVAL; } + env->fmodret_retvals = tgt_info.fmodret_retvals; + /* * We don't get trampoline for tracing_multi programs at this point, * it's done when tracing_multi link is created. -- 2.55.0