On Fri, 10 Apr 2026 07:00:25, [email protected] wrote:
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 9c1135d373e2..37975e9a291c 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
>
> [ ... ]
>
> > +/* hooks return 0 or 1 */
> > +BTF_SET_START(bool_security_hooks)
> > +BTF_ID(func, security_xfrm_state_pol_flow_match)
> > +BTF_ID(func, security_audit_rule_known)
> > +BTF_ID(func, security_inode_xattr_skipcap)
> > +BTF_SET_END(bool_security_hooks)
>
> There are two issues with this BTF set.
>
> First, security_inode_xattr_skipcap does not exist as a kernel function.
> The LSM hook inode_xattr_skipcap is only invoked via call_int_hook()
> inside security_inode_setxattr() and security_inode_removexattr().
> There is no standalone security_inode_xattr_skipcap() wrapper. The
> equivalent entry in bool_lsm_hooks in kernel/bpf/bpf_lsm.c correctly
> references bpf_lsm_inode_xattr_skipcap instead.
>
Yes, and in fact, the return values of `security_inode_removexattr` and
`security_inode_setxattr`
can be those of `inode_removexattr` and `inode_setxattr` respectively, so the
0-1 restriction is not necessary here.
> Second, the v1 of this patch had #ifdef guards matching those in
> bool_lsm_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
>
> These guards were dropped in v2. security_xfrm_state_pol_flow_match
> is defined under CONFIG_SECURITY_NETWORK_XFRM in security/security.c,
> and security_audit_rule_known is defined under CONFIG_AUDIT. Without
> the guards, BTF_IDs will be unresolvable when those configs are
> disabled.
I saw there was a default return prototype even without the corresponding
config,
and assumed that hooking would still work without it, so I removed the config
guards.
However, I didn't notice that these are inline functions and cannot be hooked.
I will revert this change in the next version.
> > +
> > +/* Similar to bpf_lsm_get_retval_range,
> > + * ensure that the return values of fmod_ret are valid.
> > + */
>
> This isn't a bug, but the BPF subsystem uses the kernel multi-line
> comment style where the opening /* goes on its own line:
>
> /*
> * Similar to bpf_lsm_get_retval_range,
> * ensure that the return values of fmod_ret are valid.
> */
>
> [ ... ]
>
> This commit appears to be a major bug fix addressing a kernel NULL
> pointer dereference but is missing a Fixes: tag. The bug was
> introduced when fmod_ret was first allowed on security_* functions
> without return value validation:
>
> Fixes: 6ba43b761c41 ("bpf: Attachment verification for BPF_MODIFY_RETURN")
>
Thanks.
