public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc
@ 2026-04-08  9:48 Feng Yang
  2026-04-08 11:53 ` Jiri Olsa
  2026-04-08 12:16 ` Jiayuan Chen
  0 siblings, 2 replies; 5+ messages in thread
From: Feng Yang @ 2026-04-08  9:48 UTC (permalink / raw)
  To: ast, daniel, andrii, martin.lau, eddyz87, memxor, song,
	yonghong.song, jolsa, john.fastabend, kpsingh, mattbobrowski
  Cc: bpf, linux-kernel

From: Feng Yang <yangfeng@kylinos.cn>

Using the following BPF program will cause a kernel panic:
SEC("fmod_ret/security_task_alloc")
int fmod_task_alloc(void *ctx)
{
        return 1;
}

[  383.899321] BUG: kernel NULL pointer dereference, address: 0000000000000b99
[  383.899327] #PF: supervisor read access in kernel mode
[  383.899330] #PF: error_code(0x0000) - not-present page
[  383.899332] PGD 8000000108a60067 P4D 8000000108a60067 PUD 104550067 PMD 0
[  383.899341] Oops: Oops: 0000 [#1] SMP PTI
[  383.899346] CPU: 1 UID: 0 PID: 12925 Comm: test Kdump: loaded Not tainted 7.0.0-rc6+ #1 PREEMPT(lazy)
[  383.899349] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[  383.899351] RIP: 0010:get_task_pid+0x20/0x80
[  383.899358] Code: 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 55 89 f5 53 48 89 fb e8 0b 98 0a 00 85 ed 75 32 48 81 c3 98 0b 00 00 <48> 8b 1b 48 85 db 74 14 b8 01 00 00 00 f0 0f c1 03 85 c0 74 27 8d
[  383.899362] RSP: 0018:ffffd3ca0ab13b38 EFLAGS: 00010206
[  383.899367] RAX: 0000000000000001 RBX: 0000000000000b99 RCX: 0000000000000008
[  383.899371] RDX: ffff8b2c85860000 RSI: 0000000000000000 RDI: 0000000000000001
[  383.899374] RBP: 0000000000000000 R08: 0000010754cf50c8 R09: 0000010754cf50c8
[  383.899377] R10: ffffffff95a6c000 R11: 0000000000000024 R12: 0000000000000000
[  383.899380] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000001200000
[  383.899384] FS:  00007f3eae907740(0000) GS:ffff8b2d05e5c000(0000) knlGS:0000000000000000
[  383.899388] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  383.899390] CR2: 0000000000000b99 CR3: 0000000106a94003 CR4: 00000000000706f0
[  383.899393] Call Trace:
[  383.899397]  <TASK>
[  383.899401]  kernel_clone+0xe8/0x480
[  383.899406]  __do_sys_clone+0x65/0x90
[  383.899410]  do_syscall_64+0xca/0x860
[  383.899421]  ? next_uptodate_folio+0x85/0x2a0
[  383.899427]  ? percpu_counter_add_batch+0x4c/0x90
[  383.899434]  ? filemap_map_pages+0x3b7/0x4d0
[  383.899437]  ? do_read_fault+0x107/0x210
[  383.899442]  ? do_fault+0x1b2/0x330
[  383.899445]  ? __handle_mm_fault+0x49b/0x7a0
[  383.899448]  ? count_memcg_events+0xc4/0x160
[  383.899453]  ? handle_mm_fault+0xbb/0x370
[  383.899456]  ? do_user_addr_fault+0x209/0x680
[  383.899459]  ? irqentry_exit+0x7a/0x660
[  383.899462]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  383.899466] RIP: 0033:0x7f3eaeb426e7
[  383.899470] Code: 5d c3 90 f3 0f 1e fa 64 48 8b 04 25 10 00 00 00 45 31 c0 31 d2 31 f6 bf 11 00 20 01 4c 8d 90 d0 02 00 00 b8 38 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 39 89 c2 85 c0 75 2c 64 48 8b 04 25 10 00 00
[  383.899472] RSP: 002b:00007fff23a2c838 EFLAGS: 00000246 ORIG_RAX: 0000000000000038
[  383.899477] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f3eaeb426e7
[  383.899479] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000001200011
[  383.899481] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[  383.899483] R10: 00007f3eae907a10 R11: 0000000000000246 R12: 0000000000000001
[  383.899485] R13: 00007fff23a2dbe8 R14: 0000000000403de0 R15: 00007f3eaecf5000
[  383.899488]  </TASK>
[  383.899489] Modules linked in: nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables sunrpc intel_rapl_msr intel_rapl_common intel_uncore_frequency_common intel_pmc_core pmt_telemetry snd_intel8x0 pmt_discovery pmt_class snd_ac97_codec intel_pmc_ssram_telemetry ac97_bus intel_vsec snd_pcm snd_timer snd rapl soundcore joydev sg i2c_piix4 pcspkr vboxguest i2c_smbus nfnetlink sr_mod cdrom sd_mod ata_generic vmwgfx ahci libahci ata_piix drm_ttm_helper ghash_clmulni_intel video ttm e1000 wmi libata serio_raw dm_mirror dm_region_hash dm_log dm_multipath dm_mod fuse i2c_dev autofs4
[  383.899624] CR2: 0000000000000b99

This is because
1. The BPF program is designed to override the normal behavior of
`security_task_alloc` and forcibly return a non-zero error value (e.g.,
`-ENOMEM` or `1`).
2. User space triggers a process creation syscall (like `fork` or
`clone`). The kernel invokes `copy_process` to allocate and initialize a
new `task_struct` for the child process.
3. `copy_process` invokes `security_task_alloc`. Due to the attached BPF
program, it returns the positive value `1`.
4. `copy_process` treats any non-zero return from `security_task_alloc`
as a failure. It aborts initialization, cleans up, and returns the error
code cast to a pointer via `ERR_PTR(1)` (which evaluates to the memory
address `0x1`).
5. In `kernel_clone()`, the kernel uses the `IS_ERR()` macro to check if
the returned `task_struct *p` is an error pointer. However, `IS_ERR()`
only checks for negative error codes in the range `[-MAX_ERRNO, -1]`.
Since `0x1` is not in this range, the check `IS_ERR(p)` evaluates to
`false`.
6. The kernel incorrectly assumes `p` is a valid `task_struct` pointer
and proceeds to call `get_task_pid(p, PIDTYPE_PID)`. This dereferences
the invalid address `0x1` (plus the offset of the `thread_pid` field),
triggering a general protection fault or null-pointer dereference.

Currently, fmod_ret does not validate return values.
Such validation should be similar to that of BPF_LSM,
so we implement the same changes by referencing `bpf_lsm_get_retval_range` to fix this issue.

Reported-by: Quan Sun <2022090917019@std.uestc.edu.cn>
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
Closes: https://lore.kernel.org/bpf/973a1b7b-8ee7-407a-890a-11455d9cc5bf@std.uestc.edu.cn/
Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 include/linux/bpf_lsm.h |  8 ++++++++
 kernel/bpf/bpf_lsm.c    | 26 ++++++++++++++++++++++++++
 kernel/bpf/verifier.c   |  4 +++-
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index 643809cc78c3..434ae335b188 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -48,6 +48,8 @@ void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func)
 
 int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
 			     struct bpf_retval_range *range);
+void bpf_security_get_retval_range(const struct bpf_prog *prog,
+				   struct bpf_retval_range *range);
 int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
 				const struct bpf_dynptr *value_p, int flags);
 int bpf_remove_dentry_xattr_locked(struct dentry *dentry, const char *name__str);
@@ -91,6 +93,12 @@ static inline int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
 {
 	return -EOPNOTSUPP;
 }
+
+static inline void bpf_security_get_retval_range(const struct bpf_prog *prog,
+						 struct bpf_retval_range *range)
+{
+}
+
 static inline int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
 					      const struct bpf_dynptr *value_p, int flags)
 {
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index c5c925f00202..537af87b3d5d 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -448,3 +448,29 @@ int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
 	}
 	return 0;
 }
+
+/* hooks return 0 or 1 */
+BTF_SET_START(bool_security_hooks)
+#ifdef CONFIG_SECURITY_NETWORK_XFRM
+BTF_ID(func, security_xfrm_state_pol_flow_match)
+#endif
+#ifdef CONFIG_AUDIT
+BTF_ID(func, security_audit_rule_known)
+#endif
+BTF_ID(func, security_inode_xattr_skipcap)
+BTF_SET_END(bool_security_hooks)
+
+/* Similar to bpf_lsm_get_retval_range,
+ * ensure that the return values of fmod_ret are valid.
+ */
+void bpf_security_get_retval_range(const struct bpf_prog *prog,
+				   struct bpf_retval_range *retval_range)
+{
+	if (btf_id_set_contains(&bool_security_hooks, prog->aux->attach_btf_id)) {
+		retval_range->minval = 0;
+		retval_range->maxval = 1;
+	} else {
+		retval_range->minval = -MAX_ERRNO;
+		retval_range->maxval = 0;
+	}
+}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 594260c1f382..3bfc67983e12 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18413,8 +18413,10 @@ static bool return_retval_range(struct bpf_verifier_env *env, struct bpf_retval_
 			*range = retval_range(0, 0);
 			break;
 		case BPF_TRACE_RAW_TP:
-		case BPF_MODIFY_RETURN:
 			return false;
+		case BPF_MODIFY_RETURN:
+			bpf_security_get_retval_range(env->prog, range);
+			break;
 		case BPF_TRACE_ITER:
 		default:
 			break;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc
  2026-04-08  9:48 [PATCH bpf] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc Feng Yang
@ 2026-04-08 11:53 ` Jiri Olsa
  2026-04-09 10:24   ` Feng Yang
  2026-04-08 12:16 ` Jiayuan Chen
  1 sibling, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2026-04-08 11:53 UTC (permalink / raw)
  To: Feng Yang
  Cc: ast, daniel, andrii, martin.lau, eddyz87, memxor, song,
	yonghong.song, john.fastabend, kpsingh, mattbobrowski, bpf,
	linux-kernel

On Wed, Apr 08, 2026 at 05:48:16PM +0800, Feng Yang wrote:
> From: Feng Yang <yangfeng@kylinos.cn>
> 
> Using the following BPF program will cause a kernel panic:
> SEC("fmod_ret/security_task_alloc")
> int fmod_task_alloc(void *ctx)
> {
>         return 1;
> }
> 
> [  383.899321] BUG: kernel NULL pointer dereference, address: 0000000000000b99
> [  383.899327] #PF: supervisor read access in kernel mode
> [  383.899330] #PF: error_code(0x0000) - not-present page
> [  383.899332] PGD 8000000108a60067 P4D 8000000108a60067 PUD 104550067 PMD 0
> [  383.899341] Oops: Oops: 0000 [#1] SMP PTI
> [  383.899346] CPU: 1 UID: 0 PID: 12925 Comm: test Kdump: loaded Not tainted 7.0.0-rc6+ #1 PREEMPT(lazy)
> [  383.899349] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
> [  383.899351] RIP: 0010:get_task_pid+0x20/0x80
> [  383.899358] Code: 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 55 89 f5 53 48 89 fb e8 0b 98 0a 00 85 ed 75 32 48 81 c3 98 0b 00 00 <48> 8b 1b 48 85 db 74 14 b8 01 00 00 00 f0 0f c1 03 85 c0 74 27 8d
> [  383.899362] RSP: 0018:ffffd3ca0ab13b38 EFLAGS: 00010206
> [  383.899367] RAX: 0000000000000001 RBX: 0000000000000b99 RCX: 0000000000000008
> [  383.899371] RDX: ffff8b2c85860000 RSI: 0000000000000000 RDI: 0000000000000001
> [  383.899374] RBP: 0000000000000000 R08: 0000010754cf50c8 R09: 0000010754cf50c8
> [  383.899377] R10: ffffffff95a6c000 R11: 0000000000000024 R12: 0000000000000000
> [  383.899380] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000001200000
> [  383.899384] FS:  00007f3eae907740(0000) GS:ffff8b2d05e5c000(0000) knlGS:0000000000000000
> [  383.899388] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  383.899390] CR2: 0000000000000b99 CR3: 0000000106a94003 CR4: 00000000000706f0
> [  383.899393] Call Trace:
> [  383.899397]  <TASK>
> [  383.899401]  kernel_clone+0xe8/0x480
> [  383.899406]  __do_sys_clone+0x65/0x90
> [  383.899410]  do_syscall_64+0xca/0x860
> [  383.899421]  ? next_uptodate_folio+0x85/0x2a0
> [  383.899427]  ? percpu_counter_add_batch+0x4c/0x90
> [  383.899434]  ? filemap_map_pages+0x3b7/0x4d0
> [  383.899437]  ? do_read_fault+0x107/0x210
> [  383.899442]  ? do_fault+0x1b2/0x330
> [  383.899445]  ? __handle_mm_fault+0x49b/0x7a0
> [  383.899448]  ? count_memcg_events+0xc4/0x160
> [  383.899453]  ? handle_mm_fault+0xbb/0x370
> [  383.899456]  ? do_user_addr_fault+0x209/0x680
> [  383.899459]  ? irqentry_exit+0x7a/0x660
> [  383.899462]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [  383.899466] RIP: 0033:0x7f3eaeb426e7
> [  383.899470] Code: 5d c3 90 f3 0f 1e fa 64 48 8b 04 25 10 00 00 00 45 31 c0 31 d2 31 f6 bf 11 00 20 01 4c 8d 90 d0 02 00 00 b8 38 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 39 89 c2 85 c0 75 2c 64 48 8b 04 25 10 00 00
> [  383.899472] RSP: 002b:00007fff23a2c838 EFLAGS: 00000246 ORIG_RAX: 0000000000000038
> [  383.899477] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f3eaeb426e7
> [  383.899479] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000001200011
> [  383.899481] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> [  383.899483] R10: 00007f3eae907a10 R11: 0000000000000246 R12: 0000000000000001
> [  383.899485] R13: 00007fff23a2dbe8 R14: 0000000000403de0 R15: 00007f3eaecf5000
> [  383.899488]  </TASK>
> [  383.899489] Modules linked in: nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables sunrpc intel_rapl_msr intel_rapl_common intel_uncore_frequency_common intel_pmc_core pmt_telemetry snd_intel8x0 pmt_discovery pmt_class snd_ac97_codec intel_pmc_ssram_telemetry ac97_bus intel_vsec snd_pcm snd_timer snd rapl soundcore joydev sg i2c_piix4 pcspkr vboxguest i2c_smbus nfnetlink sr_mod cdrom sd_mod ata_generic vmwgfx ahci libahci ata_piix drm_ttm_helper ghash_clmulni_intel video ttm e1000 wmi libata serio_raw dm_mirror dm_region_hash dm_log dm_multipath dm_mod fuse i2c_dev autofs4
> [  383.899624] CR2: 0000000000000b99
> 
> This is because
> 1. The BPF program is designed to override the normal behavior of
> `security_task_alloc` and forcibly return a non-zero error value (e.g.,
> `-ENOMEM` or `1`).
> 2. User space triggers a process creation syscall (like `fork` or
> `clone`). The kernel invokes `copy_process` to allocate and initialize a
> new `task_struct` for the child process.
> 3. `copy_process` invokes `security_task_alloc`. Due to the attached BPF
> program, it returns the positive value `1`.
> 4. `copy_process` treats any non-zero return from `security_task_alloc`
> as a failure. It aborts initialization, cleans up, and returns the error
> code cast to a pointer via `ERR_PTR(1)` (which evaluates to the memory
> address `0x1`).
> 5. In `kernel_clone()`, the kernel uses the `IS_ERR()` macro to check if
> the returned `task_struct *p` is an error pointer. However, `IS_ERR()`
> only checks for negative error codes in the range `[-MAX_ERRNO, -1]`.
> Since `0x1` is not in this range, the check `IS_ERR(p)` evaluates to
> `false`.
> 6. The kernel incorrectly assumes `p` is a valid `task_struct` pointer
> and proceeds to call `get_task_pid(p, PIDTYPE_PID)`. This dereferences
> the invalid address `0x1` (plus the offset of the `thread_pid` field),
> triggering a general protection fault or null-pointer dereference.
> 
> Currently, fmod_ret does not validate return values.
> Such validation should be similar to that of BPF_LSM,
> so we implement the same changes by referencing `bpf_lsm_get_retval_range` to fix this issue.
> 
> Reported-by: Quan Sun <2022090917019@std.uestc.edu.cn>
> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
> Closes: https://lore.kernel.org/bpf/973a1b7b-8ee7-407a-890a-11455d9cc5bf@std.uestc.edu.cn/
> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> ---
>  include/linux/bpf_lsm.h |  8 ++++++++
>  kernel/bpf/bpf_lsm.c    | 26 ++++++++++++++++++++++++++
>  kernel/bpf/verifier.c   |  4 +++-
>  3 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
> index 643809cc78c3..434ae335b188 100644
> --- a/include/linux/bpf_lsm.h
> +++ b/include/linux/bpf_lsm.h
> @@ -48,6 +48,8 @@ void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func)
>  
>  int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
>  			     struct bpf_retval_range *range);
> +void bpf_security_get_retval_range(const struct bpf_prog *prog,
> +				   struct bpf_retval_range *range);
>  int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
>  				const struct bpf_dynptr *value_p, int flags);
>  int bpf_remove_dentry_xattr_locked(struct dentry *dentry, const char *name__str);
> @@ -91,6 +93,12 @@ static inline int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
>  {
>  	return -EOPNOTSUPP;
>  }
> +
> +static inline void bpf_security_get_retval_range(const struct bpf_prog *prog,
> +						 struct bpf_retval_range *range)
> +{
> +}
> +
>  static inline int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
>  					      const struct bpf_dynptr *value_p, int flags)
>  {
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index c5c925f00202..537af87b3d5d 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -448,3 +448,29 @@ int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
>  	}
>  	return 0;
>  }
> +
> +/* hooks return 0 or 1 */
> +BTF_SET_START(bool_security_hooks)
> +#ifdef CONFIG_SECURITY_NETWORK_XFRM
> +BTF_ID(func, security_xfrm_state_pol_flow_match)
> +#endif
> +#ifdef CONFIG_AUDIT
> +BTF_ID(func, security_audit_rule_known)
> +#endif
> +BTF_ID(func, security_inode_xattr_skipcap)
> +BTF_SET_END(bool_security_hooks)
> +
> +/* Similar to bpf_lsm_get_retval_range,
> + * ensure that the return values of fmod_ret are valid.
> + */
> +void bpf_security_get_retval_range(const struct bpf_prog *prog,
> +				   struct bpf_retval_range *retval_range)
> +{
> +	if (btf_id_set_contains(&bool_security_hooks, prog->aux->attach_btf_id)) {
> +		retval_range->minval = 0;
> +		retval_range->maxval = 1;
> +	} else {
> +		retval_range->minval = -MAX_ERRNO;
> +		retval_range->maxval = 0;
> +	}
> +}

ai has a point that fmod_ret can attach to other than security functions 

https://sashiko.dev/#/patchset/20260408094816.228322-1-yangfeng59949%40163.com

most of them seem to return errno (ERRNO), but there's also few with
'TRUE' and one with 'NULL' ..  we could check if the function is on
the injection list and check the return value accordingly?

jirka


> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 594260c1f382..3bfc67983e12 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18413,8 +18413,10 @@ static bool return_retval_range(struct bpf_verifier_env *env, struct bpf_retval_
>  			*range = retval_range(0, 0);
>  			break;
>  		case BPF_TRACE_RAW_TP:
> -		case BPF_MODIFY_RETURN:
>  			return false;
> +		case BPF_MODIFY_RETURN:
> +			bpf_security_get_retval_range(env->prog, range);
> +			break;
>  		case BPF_TRACE_ITER:
>  		default:
>  			break;
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc
  2026-04-08  9:48 [PATCH bpf] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc Feng Yang
  2026-04-08 11:53 ` Jiri Olsa
@ 2026-04-08 12:16 ` Jiayuan Chen
  2026-04-09 10:25   ` Feng Yang
  1 sibling, 1 reply; 5+ messages in thread
From: Jiayuan Chen @ 2026-04-08 12:16 UTC (permalink / raw)
  To: Feng Yang, ast, daniel, andrii, martin.lau, eddyz87, memxor, song,
	yonghong.song, jolsa, john.fastabend, kpsingh, mattbobrowski
  Cc: bpf, linux-kernel


On 4/8/26 5:48 PM, Feng Yang wrote:
> From: Feng Yang <yangfeng@kylinos.cn>


[...]

>
> diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
> index 643809cc78c3..434ae335b188 100644
> --- a/include/linux/bpf_lsm.h
> +++ b/include/linux/bpf_lsm.h
> @@ -48,6 +48,8 @@ void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func)
>   
>   int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
>   			     struct bpf_retval_range *range);
> +void bpf_security_get_retval_range(const struct bpf_prog *prog,
> +				   struct bpf_retval_range *range);
>   int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
>   				const struct bpf_dynptr *value_p, int flags);
>   int bpf_remove_dentry_xattr_locked(struct dentry *dentry, const char *name__str);
> @@ -91,6 +93,12 @@ static inline int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
>   {
>   	return -EOPNOTSUPP;
>   }
> +
> +static inline void bpf_security_get_retval_range(const struct bpf_prog *prog,
> +						 struct bpf_retval_range *range)
> +{
> +}
> +
>   static inline int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
>   					      const struct bpf_dynptr *value_p, int flags)
>   {
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index c5c925f00202..537af87b3d5d 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -448,3 +448,29 @@ int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
>   	}
>   	return 0;
>   }
> +
> +/* hooks return 0 or 1 */
> +BTF_SET_START(bool_security_hooks)
> +#ifdef CONFIG_SECURITY_NETWORK_XFRM
> +BTF_ID(func, security_xfrm_state_pol_flow_match)
> +#endif
> +#ifdef CONFIG_AUDIT
> +BTF_ID(func, security_audit_rule_known)
> +#endif
> +BTF_ID(func, security_inode_xattr_skipcap)
> +BTF_SET_END(bool_security_hooks)
> +
> +/* Similar to bpf_lsm_get_retval_range,
> + * ensure that the return values of fmod_ret are valid.
> + */
> +void bpf_security_get_retval_range(const struct bpf_prog *prog,
> +				   struct bpf_retval_range *retval_range)
> +{
> +	if (btf_id_set_contains(&bool_security_hooks, prog->aux->attach_btf_id)) {
> +		retval_range->minval = 0;
> +		retval_range->maxval = 1;
> +	} else {
> +		retval_range->minval = -MAX_ERRNO;
> +		retval_range->maxval = 0;
> +	}
> +}
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 594260c1f382..3bfc67983e12 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18413,8 +18413,10 @@ static bool return_retval_range(struct bpf_verifier_env *env, struct bpf_retval_
>   			*range = retval_range(0, 0);
>   			break;
>   		case BPF_TRACE_RAW_TP:
> -		case BPF_MODIFY_RETURN:
>   			return false;
> +		case BPF_MODIFY_RETURN:
> +			bpf_security_get_retval_range(env->prog, range);
> +			break;

This is a breaking change. The verifier rejection log should be more 
descriptive
so users can understand why their program is being rejected.

>   		case BPF_TRACE_ITER:
>   		default:
>   			break;

Also, I think a whitelist approach would be better here.
The known danger is specifically those security hooks whose return 
values get fed into ERR_PTR() by callers, such as:
- security_task_alloc
- security_inode_readlink
- security_task_movememory
- security_inode_follow_link
- security_fs_context_submount
- security_dentry_create_files_as
- security_perf_event_alloc
- security_inode_get_acl


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc
  2026-04-08 11:53 ` Jiri Olsa
@ 2026-04-09 10:24   ` Feng Yang
  0 siblings, 0 replies; 5+ messages in thread
From: Feng Yang @ 2026-04-09 10:24 UTC (permalink / raw)
  To: olsajiri
  Cc: andrii, ast, bpf, daniel, eddyz87, john.fastabend, kpsingh,
	linux-kernel, martin.lau, mattbobrowski, memxor, song,
	yangfeng59949, yonghong.song

On Wed, 8 Apr 2026 13:53:50 +0200 Jiri Olsa wrote:

[...]

> > +void bpf_security_get_retval_range(const struct bpf_prog *prog,
> > +				   struct bpf_retval_range *retval_range)
> > +{
> > +	if (btf_id_set_contains(&bool_security_hooks, prog->aux->attach_btf_id)) {
> > +		retval_range->minval = 0;
> > +		retval_range->maxval = 1;
> > +	} else {
> > +		retval_range->minval = -MAX_ERRNO;
> > +		retval_range->maxval = 0;
> > +	}
> > +}
> 
> ai has a point that fmod_ret can attach to other than security functions 
> 
> https://sashiko.dev/#/patchset/20260408094816.228322-1-yangfeng59949%40163.com
> 
> most of them seem to return errno (ERRNO), but there's also few with
> 'TRUE' and one with 'NULL' ..  we could check if the function is on
> the injection list and check the return value accordingly?
> 
> jirka

Oh right, I missed that. I'm also setting the correct return values for the error injection cases and will submit after testing.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc
  2026-04-08 12:16 ` Jiayuan Chen
@ 2026-04-09 10:25   ` Feng Yang
  0 siblings, 0 replies; 5+ messages in thread
From: Feng Yang @ 2026-04-09 10:25 UTC (permalink / raw)
  To: jiayuan.chen
  Cc: andrii, ast, bpf, daniel, eddyz87, john.fastabend, jolsa, kpsingh,
	linux-kernel, martin.lau, mattbobrowski, memxor, song,
	yangfeng59949, yonghong.song

On Wed, 8 Apr 2026 20:16:45 +0800 Jiayuan Chen wrote:

[...]

> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 594260c1f382..3bfc67983e12 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -18413,8 +18413,10 @@ static bool return_retval_range(struct bpf_verifier_env *env, struct bpf_retval_
> >   			*range = retval_range(0, 0);
> >   			break;
> >   		case BPF_TRACE_RAW_TP:
> > -		case BPF_MODIFY_RETURN:
> >   			return false;
> > +		case BPF_MODIFY_RETURN:
> > +			bpf_security_get_retval_range(env->prog, range);
> > +			break;
> 
> This is a breaking change. The verifier rejection log should be more 
> descriptive
> so users can understand why their program is being rejected.

It should be easy to see from the existing error log that the return value does not meet the requirements:

At program exit the register R0 has smin=1 smax=1 should have been in [-4095, 0]
processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

> 
> >   		case BPF_TRACE_ITER:
> >   		default:
> >   			break;
> 
> Also, I think a whitelist approach would be better here.
> The known danger is specifically those security hooks whose return 
> values get fed into ERR_PTR() by callers, such as:
> - security_task_alloc
> - security_inode_readlink
> - security_task_movememory
> - security_inode_follow_link
> - security_fs_context_submount
> - security_dentry_create_files_as
> - security_perf_event_alloc
> - security_inode_get_acl


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-09 10:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08  9:48 [PATCH bpf] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc Feng Yang
2026-04-08 11:53 ` Jiri Olsa
2026-04-09 10:24   ` Feng Yang
2026-04-08 12:16 ` Jiayuan Chen
2026-04-09 10:25   ` Feng Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox