[PATCH bpf 0/2] Add return value check for BPF_LSM_CGROUP

[PATCH bpf 0/2] Add return value check for BPF_LSM_CGROUP

[Xu Kuohai]

Quan Sun reported a NULL pointer dereference caused by invalid return value of
BPF_LSM_CGROUP program [1].

The cause is that the BPF_LSM_CGROUP programs use bpf_set_retval() helper to
set return value for the target LSM hook, and the value is not validated,
making any arbitrary value legally accepted.

To fix it, add return value check for BPF_LSM_CGROUP programs. 

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

Xu Kuohai (2):
  bpf: Add return value check for BPF_LSM_CGROUP
  selftests/bpf: Add return value tests for lsm cgroup

 kernel/bpf/verifier.c                         | 10 +++++
 .../selftests/bpf/progs/verifier_lsm.c        | 45 +++++++++++++++++++
 2 files changed, 55 insertions(+)

-- 
2.43.0

[PATCH bpf 1/2] bpf: Add return value check for BPF_LSM_CGROUP

[Xu Kuohai]

From: Xu Kuohai <xukuohai@huawei.com>

BPF_LSM_CGROUP programs use bpf_set_retval() helper to set the return
value, but the value is not validated. This could cause kernel panic
similar to the bug fixed by commit 5d99e198be27 ("bpf, lsm: Add check
for BPF LSM return value").

Fix it by verifying the argument for bpf_set_retval() falls within the
valid return value range for the target hook.

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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7fb88e1cd7c4..fe60a695de55 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10462,6 +10462,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 	case BPF_FUNC_set_retval:
 		if (prog_type == BPF_PROG_TYPE_LSM &&
 		    env->prog->expected_attach_type == BPF_LSM_CGROUP) {
+			struct bpf_retval_range range;
+			struct bpf_reg_state *r1 = &regs[BPF_REG_1];
+
 			if (!env->prog->aux->attach_func_proto->type) {
 				/* Make sure programs that attach to void
 				 * hooks don't try to modify return value.
@@ -10469,6 +10472,13 @@ 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);
+			range.return_32bit = true;
+			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:
-- 
2.43.0

Re: [PATCH bpf 1/2] bpf: Add return value check for BPF_LSM_CGROUP

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The bounds validation for bpf_set_retval() can be bypassed by passing a pointer instead of a scalar, leading to the exact kernel panic the patch attempts to fix.

Pre-existing issues:
- [High] Bounds enforcement is strictly restricted to BPF_PROG_TYPE_LSM, completely neglecting other vulnerable cgroup program types like BPF_PROG_TYPE_CGROUP_DEVICE.
--

commit db7ea535b43831ef01a0cbacc405e0287e5dd010
Author: Xu Kuohai <xukuohai@huawei.com>

bpf: Add return value check for BPF_LSM_CGROUP

This commit adds validation for the return value set by the bpf_set_retval()
helper in BPF_LSM_CGROUP programs. It verifies that the provided argument
falls within the valid return value range for the target hook to prevent
kernel panics.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7fb88e1cd7c4d..fe60a695de558 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10462,6 +10462,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>  	case BPF_FUNC_set_retval:
>  		if (prog_type == BPF_PROG_TYPE_LSM &&
>  		    env->prog->expected_attach_type == BPF_LSM_CGROUP) {

[Severity: High]
This is a pre-existing issue, but does restricting this check to
BPF_PROG_TYPE_LSM leave other cgroup program types vulnerable?

Programs like BPF_PROG_TYPE_CGROUP_DEVICE can also call bpf_set_retval()
with arbitrary positive values. If such a program returns 1 (allow)
after setting a positive return value like 1234, the value propagates
through __cgroup_bpf_check_dev_permission() and bdev_permission(), causing
bdev_file_open_by_dev() to return ERR_PTR(1234).

Since 1234 is not a valid negative error code, IS_ERR() would return false,
potentially leading to a kernel panic when the caller attempts to
dereference 1234 as a file pointer.

Should we extend this validation to all cgroup program types?

> +			struct bpf_retval_range range;
> +			struct bpf_reg_state *r1 = &regs[BPF_REG_1];
> +
>  			if (!env->prog->aux->attach_func_proto->type) {
>  				/* Make sure programs that attach to void
>  				 * hooks don't try to modify return value.
> @@ -10469,6 +10472,13 @@ 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);
> +			range.return_32bit = true;
> +			if (!retval_range_within(range, r1)) {

[Severity: High]
Can this bounds check be bypassed if a pointer is passed instead of a scalar?

Because bpf_set_retval_proto uses ARG_ANYTHING, check_func_arg() allows
pointer registers when env->allow_ptr_leaks is true (which is the case
with CAP_BPF).

If a pointer like PTR_TO_STACK is passed without a variable offset,
its s32_min_value and s32_max_value are both 0. Since 0 falls within the
valid [-MAX_ERRNO, 0] range, retval_range_within() will allow it.

At runtime, the 64-bit pointer address would be truncated to a 32-bit integer,
potentially yielding an arbitrary positive return value and causing the exact
kernel panic this patch intends to fix.

Should the verifier explicitly ensure that R1 is a SCALAR_VALUE before
validating its range?

> +				verbose_invalid_scalar(env, r1, range, "At bpf_set_retval", "R1");
> +				return -EINVAL;
> +			}
>  		}
>  		break;
>  	case BPF_FUNC_dynptr_data:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523085806.417723-1-xukuohai@huaweicloud.com?part=1

Re: [PATCH bpf 1/2] bpf: Add return value check for BPF_LSM_CGROUP

[Xu Kuohai]

On 5/23/2026 5:48 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] The bounds validation for bpf_set_retval() can be bypassed by passing a pointer instead of a scalar, leading to the exact kernel panic the patch attempts to fix.
> 
> Pre-existing issues:
> - [High] Bounds enforcement is strictly restricted to BPF_PROG_TYPE_LSM, completely neglecting other vulnerable cgroup program types like BPF_PROG_TYPE_CGROUP_DEVICE.
> --
> 
> commit db7ea535b43831ef01a0cbacc405e0287e5dd010
> Author: Xu Kuohai <xukuohai@huawei.com>
> 
> bpf: Add return value check for BPF_LSM_CGROUP
> 
> This commit adds validation for the return value set by the bpf_set_retval()
> helper in BPF_LSM_CGROUP programs. It verifies that the provided argument
> falls within the valid return value range for the target hook to prevent
> kernel panics.
> 
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 7fb88e1cd7c4d..fe60a695de558 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -10462,6 +10462,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>>   	case BPF_FUNC_set_retval:
>>   		if (prog_type == BPF_PROG_TYPE_LSM &&
>>   		    env->prog->expected_attach_type == BPF_LSM_CGROUP) {
> 
> [Severity: High]
> This is a pre-existing issue, but does restricting this check to
> BPF_PROG_TYPE_LSM leave other cgroup program types vulnerable?
> 
> Programs like BPF_PROG_TYPE_CGROUP_DEVICE can also call bpf_set_retval()
> with arbitrary positive values. If such a program returns 1 (allow)
> after setting a positive return value like 1234, the value propagates
> through __cgroup_bpf_check_dev_permission() and bdev_permission(), causing
> bdev_file_open_by_dev() to return ERR_PTR(1234).
>
> Since 1234 is not a valid negative error code, IS_ERR() would return false,
> potentially leading to a kernel panic when the caller attempts to
> dereference 1234 as a file pointer.
>

Good catch! Confirmed with a PoC.

> Should we extend this validation to all cgroup program types?
> 

I think so, will do.

>> +			struct bpf_retval_range range;
>> +			struct bpf_reg_state *r1 = &regs[BPF_REG_1];
>> +
>>   			if (!env->prog->aux->attach_func_proto->type) {
>>   				/* Make sure programs that attach to void
>>   				 * hooks don't try to modify return value.
>> @@ -10469,6 +10472,13 @@ 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);
>> +			range.return_32bit = true;
>> +			if (!retval_range_within(range, r1)) {
> 
> [Severity: High]
> Can this bounds check be bypassed if a pointer is passed instead of a scalar?
> 
> Because bpf_set_retval_proto uses ARG_ANYTHING, check_func_arg() allows
> pointer registers when env->allow_ptr_leaks is true (which is the case
> with CAP_BPF).
> 
> If a pointer like PTR_TO_STACK is passed without a variable offset,
> its s32_min_value and s32_max_value are both 0. Since 0 falls within the
> valid [-MAX_ERRNO, 0] range, retval_range_within() will allow it.
> 
> At runtime, the 64-bit pointer address would be truncated to a 32-bit integer,
> potentially yielding an arbitrary positive return value and causing the exact
> kernel panic this patch intends to fix.
> 
> Should the verifier explicitly ensure that R1 is a SCALAR_VALUE before
> validating its range?
>

Yes, will check if R1 is SCALAR in next version

>> +				verbose_invalid_scalar(env, r1, range, "At bpf_set_retval", "R1");
>> +				return -EINVAL;
>> +			}
>>   		}
>>   		break;
>>   	case BPF_FUNC_dynptr_data:
> 

[PATCH bpf 2/2] selftests/bpf: Add return value tests for lsm cgroup

[Xu Kuohai]

From: Xu Kuohai <xukuohai@huawei.com>

Add tests to check return values set by bpf_set_retval() helper for lsm
cgroup programs.

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

diff --git a/tools/testing/selftests/bpf/progs/verifier_lsm.c b/tools/testing/selftests/bpf/progs/verifier_lsm.c
index 38e8e9176862..2072671ed643 100644
--- a/tools/testing/selftests/bpf/progs/verifier_lsm.c
+++ b/tools/testing/selftests/bpf/progs/verifier_lsm.c
@@ -188,4 +188,49 @@ int BPF_PROG(null_check, struct file *file)
 	return 0;
 }
 
+SEC("lsm_cgroup/socket_create")
+__description("lsm_cgroup with -4095~0 retval test 1")
+__success
+int BPF_PROG(lsm_cgroup_set_retval_zero_valid, struct task_struct *task)
+{
+	bpf_set_retval(0);
+	return 0;
+}
+
+SEC("lsm_cgroup/socket_create")
+__description("lsm_cgroup with -4095~0 retval test 2")
+__success
+int BPF_PROG(lsm_cgroup_set_retval_negative_valid, struct task_struct *task)
+{
+	bpf_set_retval(-12);
+	return 0;
+}
+
+SEC("lsm_cgroup/socket_create")
+__description("lsm_cgroup with -4095~0 retval test 3")
+__failure __msg("should have been in [-4095, 0]")
+int BPF_PROG(lsm_cgroup_set_retval_negative_invalid, struct task_struct *task)
+{
+	bpf_set_retval(-4096);
+	return 0;
+}
+
+SEC("lsm_cgroup/socket_create")
+__description("lsm_cgroup with -4095~0 retval test 4")
+__failure __msg("should have been in [-4095, 0]")
+int BPF_PROG(lsm_cgroup_set_retval_positive_invalid, struct task_struct *task)
+{
+	bpf_set_retval(1);
+	return 0;
+}
+
+SEC("lsm_cgroup/file_release")
+__description("lsm_cgroup bpf_set_retval on void hook test")
+__failure __msg("BPF_LSM_CGROUP that attach to void LSM hooks can't modify return value")
+int BPF_PROG(lsm_cgroup_set_retval_for_void_hook, struct file *file)
+{
+	bpf_set_retval(0);
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH bpf 2/2] selftests/bpf: Add return value tests for lsm cgroup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] BPF test programs for the `socket_create` LSM hook incorrectly declare `struct task_struct *task` as their argument instead of `int family`.
--

commit b7003df7335bb1051987324aa639e3088d3673b5
Author: Xu Kuohai <xukuohai@huawei.com>

selftests/bpf: Add return value tests for lsm cgroup

This commit adds tests to check return values set by the bpf_set_retval()
helper for lsm cgroup programs.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_lsm.c b/tools/testing/selftests/bpf/progs/verifier_lsm.c
> index 38e8e91768620..2072671ed6439 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_lsm.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_lsm.c

[ ... ]

> @@ -188,4 +188,49 @@ int BPF_PROG(null_check, struct file *file)
>  	return 0;
>  }
>  
> +SEC("lsm_cgroup/socket_create")
> +__description("lsm_cgroup with -4095~0 retval test 1")
> +__success
> +int BPF_PROG(lsm_cgroup_set_retval_zero_valid, struct task_struct *task)

[Severity: Low]
Is this argument type correct for the socket_create hook? The LSM hook for
socket_create expects (int family, int type, int protocol, int kern), so
the first argument should be int family.

Using struct task_struct *task incorrectly casts the first integer argument
to a pointer. While it works here because the argument isn't dereferenced,
could this cause issues if this code is used as a template and the argument
is accessed in other tests?

> +{
> +	bpf_set_retval(0);
> +	return 0;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523085806.417723-1-xukuohai@huaweicloud.com?part=2

Re: [PATCH bpf 2/2] selftests/bpf: Add return value tests for lsm cgroup

[Xu Kuohai]

On 5/23/2026 6:08 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] BPF test programs for the `socket_create` LSM hook incorrectly declare `struct task_struct *task` as their argument instead of `int family`.
> --
> 
> commit b7003df7335bb1051987324aa639e3088d3673b5
> Author: Xu Kuohai <xukuohai@huawei.com>
> 
> selftests/bpf: Add return value tests for lsm cgroup
> 
> This commit adds tests to check return values set by the bpf_set_retval()
> helper for lsm cgroup programs.
> 
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_lsm.c b/tools/testing/selftests/bpf/progs/verifier_lsm.c
>> index 38e8e91768620..2072671ed6439 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_lsm.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_lsm.c
> 
> [ ... ]
> 
>> @@ -188,4 +188,49 @@ int BPF_PROG(null_check, struct file *file)
>>   	return 0;
>>   }
>>   
>> +SEC("lsm_cgroup/socket_create")
>> +__description("lsm_cgroup with -4095~0 retval test 1")
>> +__success
>> +int BPF_PROG(lsm_cgroup_set_retval_zero_valid, struct task_struct *task)
> 
> [Severity: Low]
> Is this argument type correct for the socket_create hook? The LSM hook for
> socket_create expects (int family, int type, int protocol, int kern), so
> the first argument should be int family.
> 
> Using struct task_struct *task incorrectly casts the first integer argument
> to a pointer. While it works here because the argument isn't dereferenced,
> could this cause issues if this code is used as a template and the argument
> is accessed in other tests?
>

Ah, the argument type is indeed wrong. I will fix it in the next version.

>> +{
>> +	bpf_set_retval(0);
>> +	return 0;
>> +}
> 
> [ ... ]
> 

Re: [PATCH bpf 2/2] selftests/bpf: Add return value tests for lsm cgroup

[Emil Tsalapatis]

On Sat May 23, 2026 at 4:58 AM EDT, Xu Kuohai wrote:
> From: Xu Kuohai <xukuohai@huawei.com>
>
> Add tests to check return values set by bpf_set_retval() helper for lsm
> cgroup programs.

After fixing the task_struct arg feel free to add:

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

Nit: The test messages are kinda obscure, could you replace -4095~0 with "valid errno or
success" or something similar? E.g., test1/2/3/4 could be described as
"success"/"valid errno"/"invalid errno"/invalid value" instead of numbers.

>
> Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
> ---
>  .../selftests/bpf/progs/verifier_lsm.c        | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_lsm.c b/tools/testing/selftests/bpf/progs/verifier_lsm.c
> index 38e8e9176862..2072671ed643 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_lsm.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_lsm.c
> @@ -188,4 +188,49 @@ int BPF_PROG(null_check, struct file *file)
>  	return 0;
>  }
>  
> +SEC("lsm_cgroup/socket_create")
> +__description("lsm_cgroup with -4095~0 retval test 1")
> +__success
> +int BPF_PROG(lsm_cgroup_set_retval_zero_valid, struct task_struct *task)
> +{
> +	bpf_set_retval(0);
> +	return 0;
> +}
> +
> +SEC("lsm_cgroup/socket_create")
> +__description("lsm_cgroup with -4095~0 retval test 2")
> +__success
> +int BPF_PROG(lsm_cgroup_set_retval_negative_valid, struct task_struct *task)
> +{
> +	bpf_set_retval(-12);
> +	return 0;
> +}
> +
> +SEC("lsm_cgroup/socket_create")
> +__description("lsm_cgroup with -4095~0 retval test 3")
> +__failure __msg("should have been in [-4095, 0]")
> +int BPF_PROG(lsm_cgroup_set_retval_negative_invalid, struct task_struct *task)
> +{
> +	bpf_set_retval(-4096);
> +	return 0;
> +}
> +
> +SEC("lsm_cgroup/socket_create")
> +__description("lsm_cgroup with -4095~0 retval test 4")
> +__failure __msg("should have been in [-4095, 0]")
> +int BPF_PROG(lsm_cgroup_set_retval_positive_invalid, struct task_struct *task)
> +{
> +	bpf_set_retval(1);
> +	return 0;
> +}
> +
> +SEC("lsm_cgroup/file_release")
> +__description("lsm_cgroup bpf_set_retval on void hook test")
> +__failure __msg("BPF_LSM_CGROUP that attach to void LSM hooks can't modify return value")
> +int BPF_PROG(lsm_cgroup_set_retval_for_void_hook, struct file *file)
> +{
> +	bpf_set_retval(0);
> +	return 0;
> +}
> +
>  char _license[] SEC("license") = "GPL";

Re: [PATCH bpf 2/2] selftests/bpf: Add return value tests for lsm cgroup

[Xu Kuohai]

On 5/26/2026 2:43 AM, Emil Tsalapatis wrote:
> On Sat May 23, 2026 at 4:58 AM EDT, Xu Kuohai wrote:
>> From: Xu Kuohai <xukuohai@huawei.com>
>>
>> Add tests to check return values set by bpf_set_retval() helper for lsm
>> cgroup programs.
> 
> After fixing the task_struct arg feel free to add:
> 
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> 
> Nit: The test messages are kinda obscure, could you replace -4095~0 with "valid errno or
> success" or something similar? E.g., test1/2/3/4 could be described as
> "success"/"valid errno"/"invalid errno"/invalid value" instead of numbers.
>

Makes sense, thanks.