Re: [PATCH v3 bpf-next 1/2] bpf: Fix Null-Pointer Dereference in kernel_clone() via BPF fmod_ret on security_task_alloc

[Feng Yang <yangfeng59949@163.com>]

On Sun, 12 Apr 2026 00:35:55 +0800 Alexei Starovoitov wrote:

> > +/* The system call return value is allowed to be an arbitrary value. */
> > +static int modify_return_get_retval_range(const struct bpf_prog *prog,
> > +                                         struct bpf_retval_range *range)
> > +{
> > +       return -EINVAL;
> > +}
> > +
> > +#endif /* CONFIG_FUNCTION_ERROR_INJECTION */

> I recall people already explained that this is no go.
> We cannot break all fmod_ret because error injection is disabled.

When error injection is disabled, the returned -EINVAL will result in a false return.
This shouldn't change the previous logic.

 		case BPF_TRACE_RAW_TP:
-		case BPF_MODIFY_RETURN:
 			return false;
+		case BPF_MODIFY_RETURN:
+			if (!bpf_security_get_retval_range(env->prog, range))
+				break;
+			if (modify_return_get_retval_range(env->prog, range))
+				return false;
+			break;

> Also see sashiko reports.
>

Okay.
Thansks.

> let's copy paste bpf_lsm_get_retval_range() for no good reason?!

So should we modify this part according to Jiayuan Chen's logic?

Jiayuan's previous reply:
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

> please don't send such poor quality patches.