]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
bpf: Add validation for bpf_set_retval argument
authorXu Kuohai <xukuohai@huawei.com>
Fri, 5 Jun 2026 14:02:42 +0000 (14:02 +0000)
committerAlexei Starovoitov <ast@kernel.org>
Fri, 5 Jun 2026 22:55:43 +0000 (15:55 -0700)
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.edu.cn
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>
kernel/bpf/verifier.c

index 3b874bbbaac0b897b59209ad2a30eb1542b37e12..935595138aa01c83185076ae65b3ee14f574352f 100644 (file)
@@ -9770,7 +9770,9 @@ static int do_refine_retval_range(struct bpf_verifier_env *env,
                                  int func_id,
                                  struct bpf_call_arg_meta *meta)
 {
+       struct bpf_retval_range range;
        struct bpf_reg_state *ret_reg = &regs[BPF_REG_0];
+       enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
 
        if (ret_type != RET_INTEGER)
                return 0;
@@ -9790,6 +9792,29 @@ static int do_refine_retval_range(struct bpf_verifier_env *env,
                reg_set_urange32(ret_reg, 0, nr_cpu_ids - 1);
                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;
        }
 
        return reg_bounds_sanity_check(env, ret_reg, "retval");
@@ -10259,6 +10284,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 = &regs[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) {
@@ -10268,8 +10311,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_write:
        {
                enum bpf_dynptr_type dynptr_type = meta.dynptr.type;