From: Xu Kuohai <xukuohai@huawei.com> mainline inclusion from mainline-v7.2-rc1 commit b1f7f67b74c2e29e9c83f7952290a3c7f156bf9a category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/9693 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- The bpf_set_retval() helper is used by cgroup BPF programs to set the return value of the target hook. The argument type for this helper is ARG_ANYTHING. This allows setting a positive value, which no cgroup hook expects and can cause issues, such as: - BPF_LSM_CGROUP: a positive value from bpf_lsm_socket_create bypasses the err < 0 check in __sock_create(), leaving the socket object unallocated. The positive return value is then propagated to the syscall entry __sys_socket(), which also bypasses the IS_ERR() guard and ultimately causes a NULL pointer dereference. - BPF_CGROUP_DEVICE: a positive value can be returned through cgroup device bpf prog -> devcgroup_check_permission() -> bdev_permission() -> bdev_file_open_by_dev(), where ERR_PTR(positive) produces a pointer that IS_ERR() does not catch, leading to a wild pointer dereference. - BPF_CGROUP_SOCK: a positive value can be returned through cgroup sock bpf prog -> __cgroup_bpf_run_filter_sk() -> inet_create() -> __sock_create(), where inet_create() frees the newly allocated sk via sk_common_release() and sets sock->sk = NULL on the non-zero return, but __sock_create() only checks err < 0 for cleanup, so a positive retval bypasses cleanup and returns a socket with NULL sk to userspace, triggering a NULL pointer dereference on subsequent socket operations. - BPF_CGROUP_SYSCTL: a positive value can be returned through the cgroup bpf prog -> __cgroup_bpf_run_filter_sysctl() -> proc_sys_call_handler(), where a non-zero return bypasses the normal sysctl proc_handler and is returned directly to userspace as return value of read() or write() syscall. So add validation for the argument of the bpf_set_retval() helper. For BPF_LSM_CGROUP, enforce the LSM hook specific range returned by bpf_lsm_get_retval_range(). For all other cgroup program types, restrict the argument to [-MAX_ERRNO, 0], which matches the kernel convention of 0 for success and negative errno for error. BPF_CGROUP_GETSOCKOPT is an exception, since valid getsockopt implementations may return positive values, as allowed by commit c4dcfdd406aa ("bpf: Move getsockopt retval to struct bpf_cg_run_ctx"). Also refine the return value range of bpf_get_retval() so that values returned by bpf_get_retval() can be passed directly to bpf_set_retval() without extra manual bounds checking. Fixes: b44123b4a3dc ("bpf: Add cgroup helpers bpf_{get,set}_retval to get/set syscall return value") Fixes: 69fd337a975c ("bpf: per-cgroup lsm flavor") Reported-by: Quan Sun <2022090917019@std.uestc.edu.cn> Closes: https://lore.kernel.org/all/567d3206-74a5-44e5-99c6-779c425f399e@std.uestc.e... Signed-off-by: Xu Kuohai <xukuohai@huawei.com> Link: https://lore.kernel.org/r/20260605140243.664590-3-xukuohai@huaweicloud.com Signed-off-by: Alexei Starovoitov <ast@kernel.org> Conflicts: kernel/bpf/verifier.c [ctx conflicts] Signed-off-by: Pu Lehui <pulehui@huawei.com> --- kernel/bpf/verifier.c | 55 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 3cbc46a591f9..9cf2b6427b1a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9940,7 +9940,9 @@ static void do_refine_retval_range(struct bpf_reg_state *regs, int ret_type, int func_id, struct bpf_call_arg_meta *meta) { + struct bpf_retval_range range; struct bpf_reg_state *ret_reg = ®s[BPF_REG_0]; + enum bpf_prog_type prog_type = resolve_prog_type(env->prog); if (ret_type != RET_INTEGER) return; @@ -9968,6 +9970,29 @@ static void do_refine_retval_range(struct bpf_reg_state *regs, int ret_type, ret_reg->s32_min_value = 0; reg_bounds_sync(ret_reg); break; + case BPF_FUNC_get_retval: + /* + * bpf_get_retval may see arbitrary value passed by bpf_prog_run_array_cg for + * CGROUP_GETSOCKOPT type. + */ + if (prog_type == BPF_PROG_TYPE_CGROUP_SOCKOPT && + env->prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT) + break; + + if (prog_type == BPF_PROG_TYPE_LSM && + env->prog->expected_attach_type == BPF_LSM_CGROUP) { + if (!env->prog->aux->attach_func_proto->type) + break; + bpf_lsm_get_retval_range(env->prog, &range); + } else { + range.minval = -MAX_ERRNO; + range.maxval = 0; + } + + reg_set_srange64(ret_reg, range.minval, range.maxval); + reg_set_srange32(ret_reg, range.minval, range.maxval); + reg_bounds_sync(ret_reg); + break; } } @@ -10354,6 +10379,24 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn } break; case BPF_FUNC_set_retval: + { + struct bpf_retval_range range = { + .minval = -MAX_ERRNO, + .maxval = 0, + .return_32bit = true + }; + struct bpf_reg_state *r1 = ®s[BPF_REG_1]; + + if (r1->type != SCALAR_VALUE) { + verbose(env, "R1 is not a scalar\n"); + return -EINVAL; + } + + /* CGROUP_GETSOCKOPT is allowed to return arbitrary value */ + if (prog_type == BPF_PROG_TYPE_CGROUP_SOCKOPT && + env->prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT) + break; + if (prog_type == BPF_PROG_TYPE_LSM && env->prog->expected_attach_type == BPF_LSM_CGROUP) { if (!env->prog->aux->attach_func_proto->type) { @@ -10363,8 +10406,20 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn verbose(env, "BPF_LSM_CGROUP that attach to void LSM hooks can't modify return value!\n"); return -EINVAL; } + bpf_lsm_get_retval_range(env->prog, &range); + } + + err = mark_chain_precision(env, BPF_REG_1); + if (err) + return err; + + if (!retval_range_within(range, r1)) { + verbose_invalid_scalar(env, r1, range, "At bpf_set_retval", "R1"); + return -EINVAL; } + break; + } case BPF_FUNC_dynptr_data: { struct bpf_reg_state *reg; -- 2.34.1