[PATCH bpf v4 0/3] Add validation for bpf_set_retval helper

[PATCH bpf v4 0/3] Add validation for bpf_set_retval helper

[Xu Kuohai]

From: Xu Kuohai <xukuohai@huawei.com>

The bpf_set_retval() helper is used by cgroup BPF programs to set the
return value of the kernel 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 the kernel panic reported
in [1].

This series adds validation for the argument of the bpf_set_retval()
helper.

For BPF_LSM_CGROUP, the same validation as BPF_LSM_MAC is enforced,
i.e. validate the argument against the LSM hook specific range, which
is 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 from this restriction, since valid
getsockopt implementations may return positive values (e.g. optlen), as
allowed by commit c4dcfdd406aa ("bpf: Move getsockopt retval to struct
bpf_cg_run_ctx").

[1] https://lore.kernel.org/all/567d3206-74a5-44e5-99c6-779c425f399e@std.uestc.edu.cn

v4:
- Remove the return value limit for BPF_CGROUP_GETSOCKOPT type
- Refine the range of return value of bpf_get_retval helper

v3: https://lore.kernel.org/bpf/20260530101239.590395-1-xukuohai@huaweicloud.com/
- Mark R1 as precise to prevent validation bypass via branch pruning (sashiko)

v2: https://lore.kernel.org/bpf/20260530055557.549474-1-xukuohai@huaweicloud.com/
- Extend validation from LSM cgroup BPF type to all cgroup BPF types (sashiko)

v1: https://lore.kernel.org/bpf/20260523085806.417723-1-xukuohai@huaweicloud.com/

Xu Kuohai (3):
  selftests/bpf: Restrict bpf_set_retval argument in sk_bypass_prot_mem
  bpf: Add validation for bpf_set_retval argument
  selftests/bpf: Add tests for bpf_set_retval validation

 kernel/bpf/verifier.c                         |  54 +++++++++
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../selftests/bpf/progs/sk_bypass_prot_mem.c  |   2 +
 .../selftests/bpf/progs/verifier_set_retval.c | 107 ++++++++++++++++++
 4 files changed, 165 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_set_retval.c

-- 
2.47.3

[PATCH bpf v4 1/3] selftests/bpf: Restrict bpf_set_retval argument in sk_bypass_prot_mem

[Xu Kuohai]

From: Xu Kuohai <xukuohai@huawei.com>

Test sk_bypass_prot_mem passes an unchecked value as argument to helper
bpf_set_retval(). The argument can be outside the valid range enforced
by the strict retval validation added in the next patch.

Restrict the argument to -EFAULT when it is outside the valid range, so
the test will not be rejected by the verifier when retval validation
is enforced.

Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
---
 tools/testing/selftests/bpf/progs/sk_bypass_prot_mem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/sk_bypass_prot_mem.c b/tools/testing/selftests/bpf/progs/sk_bypass_prot_mem.c
index 09a00d11ffcc..bae5283fca6b 100644
--- a/tools/testing/selftests/bpf/progs/sk_bypass_prot_mem.c
+++ b/tools/testing/selftests/bpf/progs/sk_bypass_prot_mem.c
@@ -5,6 +5,7 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
 #include <errno.h>
+#include "err.h"
 
 extern int tcp_memory_per_cpu_fw_alloc __ksym;
 extern int udp_memory_per_cpu_fw_alloc __ksym;
@@ -97,6 +98,7 @@ int sock_create(struct bpf_sock *ctx)
 	return 1;
 
 err:
+	set_if_not_errno_or_zero(err, -EFAULT);
 	bpf_set_retval(err);
 	return 0;
 }
-- 
2.47.3

[PATCH bpf v4 2/3] bpf: Add validation for bpf_set_retval argument

[Xu Kuohai]

From: Xu Kuohai <xukuohai@huawei.com>

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>
---
 kernel/bpf/verifier.c | 54 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c8d980fdd709..32b4d88c5b32 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9790,6 +9790,7 @@ 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];
 
 	if (ret_type != RET_INTEGER)
@@ -9810,6 +9811,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_reval may see arbitrary value passed by bpf_prog_run_array_cg for
+		 * CGROUP_GETSOCKOPT type.
+		 */
+		if (env->prog->type == BPF_PROG_TYPE_CGROUP_SOCKOPT &&
+		    env->prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT)
+			break;
+
+		if (env->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");
@@ -10290,6 +10314,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 (env->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) {
@@ -10299,8 +10341,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.47.3

[PATCH bpf v4 3/3] selftests/bpf: Add tests for bpf_set_retval validation

[Xu Kuohai]

From: Xu Kuohai <xukuohai@huawei.com>

Add verifier tests to validate bpf_set_retval argument for cgroup
program types.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com> #v1
Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
---
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../selftests/bpf/progs/verifier_set_retval.c | 107 ++++++++++++++++++
 2 files changed, 109 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_set_retval.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 219ff2969868..89779d897aba 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -117,6 +117,7 @@
 #include "verifier_xdp.skel.h"
 #include "verifier_xdp_direct_packet_access.skel.h"
 #include "verifier_bits_iter.skel.h"
+#include "verifier_set_retval.skel.h"
 #include "verifier_lsm.skel.h"
 #include "verifier_jit_inline.skel.h"
 #include "irq.skel.h"
@@ -266,6 +267,7 @@ void test_verifier_xadd(void)                 { RUN(verifier_xadd); }
 void test_verifier_xdp(void)                  { RUN(verifier_xdp); }
 void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); }
 void test_verifier_bits_iter(void) { RUN(verifier_bits_iter); }
+void test_verifier_set_retval(void)            { RUN(verifier_set_retval); }
 void test_verifier_lsm(void)                  { RUN(verifier_lsm); }
 void test_irq(void)			      { RUN(irq); }
 void test_verifier_mtu(void)		      { RUN(verifier_mtu); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_set_retval.c b/tools/testing/selftests/bpf/progs/verifier_set_retval.c
new file mode 100644
index 000000000000..1415cd15cede
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_set_retval.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+SEC("lsm_cgroup/socket_create")
+__description("lsm_cgroup bpf_set_retval success")
+__success
+int BPF_PROG(lsm_cgroup_set_retval_zero_valid, int family, int type, int protocol, int kern)
+{
+	bpf_set_retval(0);
+	return 0;
+}
+
+SEC("lsm_cgroup/socket_create")
+__description("lsm_cgroup bpf_set_retval valid errno")
+__success
+int BPF_PROG(lsm_cgroup_set_retval_negative_valid, int family, int type, int protocol, int kern)
+{
+	bpf_set_retval(-12);
+	return 0;
+}
+
+SEC("lsm_cgroup/socket_create")
+__description("lsm_cgroup bpf_set_retval invalid negative value")
+__failure __msg("should have been in [-4095, 0]")
+int BPF_PROG(lsm_cgroup_set_retval_negative_invalid, int family, int type, int protocol, int kern)
+{
+	bpf_set_retval(-4096);
+	return 0;
+}
+
+SEC("lsm_cgroup/socket_create")
+__description("lsm_cgroup bpf_set_retval invalid positive value")
+__failure __msg("should have been in [-4095, 0]")
+int BPF_PROG(lsm_cgroup_set_retval_positive_invalid, int family, int type, int protocol, int kern)
+{
+	bpf_set_retval(1);
+	return 0;
+}
+
+SEC("cgroup/dev")
+__description("cgroup_device bpf_set_retval success")
+__success
+int cgroup_dev_set_retval_0(struct bpf_cgroup_dev_ctx *ctx)
+{
+	bpf_set_retval(0);
+	return 1;
+}
+
+SEC("cgroup/dev")
+__description("cgroup_device bpf_set_retval valid errno")
+__success
+int cgroup_dev_set_retval_neg_maxerrno(struct bpf_cgroup_dev_ctx *ctx)
+{
+	bpf_set_retval(-4095);
+	return 1;
+}
+
+SEC("cgroup/dev")
+__description("cgroup_device bpf_set_retval invalid positive value")
+__failure __msg("should have been in [-4095, 0]")
+int cgroup_dev_set_retval_1(struct bpf_cgroup_dev_ctx *ctx)
+{
+	bpf_set_retval(1);
+	return 1;
+}
+
+SEC("cgroup/dev")
+__description("cgroup_device bpf_set_retval invalid negative value")
+__failure __msg("should have been in [-4095, 0]")
+int cgroup_dev_set_retval_neg_4096(struct bpf_cgroup_dev_ctx *ctx)
+{
+	bpf_set_retval(-4096);
+	return 1;
+}
+
+SEC("cgroup/dev")
+__description("bpf_set_retval bounds check survives state pruning")
+__failure __msg("should have been in [-4095, 0]")
+__naked int cgroup_dev_set_retval_pruning_bypass(struct bpf_cgroup_dev_ctx *ctx)
+{
+	asm volatile (
+		"call %[bpf_get_prandom_u32];"
+		"if r0 != 0 goto 1f;"
+		"r0 = r0;"
+		"r0 = r0;"
+		"r0 = r0;"
+		"r0 = r0;"
+		"goto 2f;"
+	"1:"
+		"call %[bpf_get_prandom_u32];"
+	"2:"
+		"r1 = r0;"
+		"call %[bpf_set_retval];"
+		"r0 = 1;"
+		"exit;"
+		:
+		: __imm(bpf_get_prandom_u32),
+		  __imm(bpf_set_retval)
+		: __clobber_common
+	);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.47.3

Re: [PATCH bpf v4 2/3] bpf: Add validation for bpf_set_retval argument

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2725 bytes --]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7fb88e1cd7c4..1bf58685b655 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9952,6 +9952,7 @@ 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];
>
>  	if (ret_type != RET_INTEGER)
> @@ -9980,6 +9981,29 @@ static int do_refine_retval_range(struct bpf_verifier_env *env,
>  		ret_reg->s32_min_value = 0;
>  		reg_bounds_sync(ret_reg);
>  		break;
> +	case BPF_FUNC_get_retval:
> +		/*
> +		 * bpf_get_reval may see arbitrary value passed by bpf_prog_run_array_cg for
> +		 * CGROUP_GETSOCKOPT type.
> +		 */
> +		if (env->prog->type == BPF_PROG_TYPE_CGROUP_SOCKOPT &&
> +		    env->prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT)
> +			break;
> +
> +		if (env->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);
                    ^^^^^^^^^^^^^^^

Are reg_set_srange64() and reg_set_srange32() defined in this kernel? A
search across the entire tree finds only these two call sites and no
definition. The struct bpf_retval_range has minval/maxval fields but no
associated reg_set_srange* helper exists in this codebase.

Looking at the existing pattern in this same function (lines 9969-9970),
the code directly assigns fields:

    ret_reg->smin_value = -MAX_ERRNO;
    ret_reg->s32_min_value = -MAX_ERRNO;

The reg_set_srange* helpers appear to belong to a different verifier
variant using reg->r64 = cnum64_from_srange(...) that isn't present here.

Should this use the same pattern as the BPF_FUNC_get_stack case above it:

    ret_reg->smin_value = range.minval;
    ret_reg->smax_value = range.maxval;
    ret_reg->s32_min_value = range.minval;
    ret_reg->s32_max_value = range.maxval;
    reg_bounds_sync(ret_reg);

This would match the existing codebase and avoid undefined function
references that will fail to compile.

> +		reg_bounds_sync(ret_reg);
> +		break;
>  	}
>
>  	return reg_bounds_sanity_check(env, ret_reg, "retval");

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26955107708

Re: [PATCH bpf v4 2/3] bpf: Add validation for bpf_set_retval argument

[Alexei Starovoitov]

On Thu, Jun 4, 2026 at 6:52 AM <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 7fb88e1cd7c4..1bf58685b655 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -9952,6 +9952,7 @@ 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];
> >
> >       if (ret_type != RET_INTEGER)
> > @@ -9980,6 +9981,29 @@ static int do_refine_retval_range(struct bpf_verifier_env *env,
> >               ret_reg->s32_min_value = 0;
> >               reg_bounds_sync(ret_reg);
> >               break;
> > +     case BPF_FUNC_get_retval:
> > +             /*
> > +              * bpf_get_reval may see arbitrary value passed by bpf_prog_run_array_cg for
> > +              * CGROUP_GETSOCKOPT type.
> > +              */
> > +             if (env->prog->type == BPF_PROG_TYPE_CGROUP_SOCKOPT &&
> > +                 env->prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT)
> > +                     break;
> > +
> > +             if (env->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);
>                     ^^^^^^^^^^^^^^^
>
> Are reg_set_srange64() and reg_set_srange32() defined in this kernel? A
> search across the entire tree finds only these two call sites and no
> definition. The struct bpf_retval_range has minval/maxval fields but no
> associated reg_set_srange* helper exists in this codebase.

AI is correct, because patch subject is wrong.
It should have been [PATCH bpf-next v4].
Pls resubmit targeting correct tree.

pw-bot: cr

Re: [PATCH bpf v4 2/3] bpf: Add validation for bpf_set_retval argument

[Yonghong Song]

On 6/4/26 6:04 AM, Xu Kuohai wrote:
> From: Xu Kuohai <xukuohai@huawei.com>
>
> 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.

FYI, the following patch:
     https://lore.kernel.org/bpf/20260603105317.944304-4-dawei.feng@seu.edu.cn/
will change return value for BPF_CGROUP_SYSCTL from 1 to 0.

>
> 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>
> ---
>   kernel/bpf/verifier.c | 54 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
>
[...]

Re: [PATCH bpf v4 2/3] bpf: Add validation for bpf_set_retval argument

[Xu Kuohai]

On 6/5/2026 12:29 AM, Alexei Starovoitov wrote:
> On Thu, Jun 4, 2026 at 6:52 AM <bot+bpf-ci@kernel.org> wrote:
>>
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index 7fb88e1cd7c4..1bf58685b655 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -9952,6 +9952,7 @@ 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];
>>>
>>>        if (ret_type != RET_INTEGER)
>>> @@ -9980,6 +9981,29 @@ static int do_refine_retval_range(struct bpf_verifier_env *env,
>>>                ret_reg->s32_min_value = 0;
>>>                reg_bounds_sync(ret_reg);
>>>                break;
>>> +     case BPF_FUNC_get_retval:
>>> +             /*
>>> +              * bpf_get_reval may see arbitrary value passed by bpf_prog_run_array_cg for
>>> +              * CGROUP_GETSOCKOPT type.
>>> +              */
>>> +             if (env->prog->type == BPF_PROG_TYPE_CGROUP_SOCKOPT &&
>>> +                 env->prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT)
>>> +                     break;
>>> +
>>> +             if (env->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);
>>                      ^^^^^^^^^^^^^^^
>>
>> Are reg_set_srange64() and reg_set_srange32() defined in this kernel? A
>> search across the entire tree finds only these two call sites and no
>> definition. The struct bpf_retval_range has minval/maxval fields but no
>> associated reg_set_srange* helper exists in this codebase.
> 
> AI is correct, because patch subject is wrong.
> It should have been [PATCH bpf-next v4].
> Pls resubmit targeting correct tree.
>

OK, will target bpf-next in v5.

> pw-bot: cr

Re: [PATCH bpf v4 2/3] bpf: Add validation for bpf_set_retval argument

[Xu Kuohai]

On 6/5/2026 3:19 AM, Yonghong Song wrote:
> 
> 
> On 6/4/26 6:04 AM, Xu Kuohai wrote:
>> From: Xu Kuohai <xukuohai@huawei.com>
>>
>> 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.
> 
> FYI, the following patch:
>      https://lore.kernel.org/bpf/20260603105317.944304-4-dawei.feng@seu.edu.cn/
> will change return value for BPF_CGROUP_SYSCTL from 1 to 0.
> 

Hmm, it is a complementary fix. It updates the BPF_CGROUP_SYSCTL to use 0
as the success return value, while my patch restricts the bpf prog to only
return 0 for success or -errno for failure.

> [...]