Re: [v6 10/10] ipe: Add BPF program load policy enforcement via Hornet integration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Silent fail-open of `BPF_PROG_LOAD` policies when `CONFIG_IPE_PROP_BPF_SIGNATURE` is disabled
- [Low] Build failure due to unconditional access to conditionally compiled field `ctx->bpf_kernel`
--

commit 168706f9eca9b1981fccd36319334eda60264af6
Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>

ipe: Add BPF program load policy enforcement via Hornet integration

This commit adds support for the bpf_prog_load_post_integrity LSM hook,
enabling IPE to make policy decisions about BPF program loading based on
integrity verdicts from the Hornet LSM. It introduces the BPF_PROG_LOAD
operation and multiple properties like bpf_signature, bpf_keyring, and
bpf_kernel.

> diff --git a/security/ipe/eval.c b/security/ipe/eval.c
> index 21439c5be336..9a6d583fea12 100644
> --- a/security/ipe/eval.c
> +++ b/security/ipe/eval.c
[ ... ]
> @@ -297,6 +342,32 @@ static bool evaluate_property(const struct ipe_eval_ctx *const ctx,
>  		return evaluate_fsv_sig_false(ctx);
>  	case IPE_PROP_FSV_SIG_TRUE:
>  		return evaluate_fsv_sig_true(ctx);
> +	case IPE_PROP_BPF_SIG_NONE:
> +		return evaluate_bpf_sig(ctx, LSM_INT_VERDICT_NONE);
[ ... ]
> +	case IPE_PROP_BPF_KERNEL_FALSE:
> +		return !ctx->bpf_kernel;
> +	case IPE_PROP_BPF_KERNEL_TRUE:
> +		return ctx->bpf_kernel;

If the kernel is compiled with CONFIG_IPE_PROP_BPF_SIGNATURE disabled,
will this cause a build failure?

In security/ipe/eval.h, the bpf_kernel field is conditionally compiled:

#ifdef CONFIG_IPE_PROP_BPF_SIGNATURE
	enum lsm_integrity_verdict bpf_verdict;
	s32 bpf_keyring_id;
	bool bpf_kernel;
#endif /* CONFIG_IPE_PROP_BPF_SIGNATURE */

Since evaluate_property() accesses ctx->bpf_kernel unconditionally, it seems
this would result in an undefined struct member error when the config is
turned off.

>  	default:
>  		return false;
>  	}

> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index 495bb765de1b..5af13903287f 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
[ ... ]
> @@ -60,6 +70,10 @@ static struct security_hook_list ipe_hooks[] __ro_after_init = {
>  #ifdef CONFIG_IPE_PROP_FS_VERITY_BUILTIN_SIG
>  	LSM_HOOK_INIT(inode_setintegrity, ipe_inode_setintegrity),
>  #endif /* CONFIG_IPE_PROP_FS_VERITY_BUILTIN_SIG */
> +#ifdef CONFIG_IPE_PROP_BPF_SIGNATURE
> +	LSM_HOOK_INIT(bpf_prog_load_post_integrity, ipe_bpf_prog_load_post_integrity),
> +	LSM_HOOK_INIT(bpf_prog_load, ipe_bpf_prog_load),
> +#endif /* CONFIG_IPE_PROP_BPF_SIGNATURE */
>  };

Does this create a scenario where policies fail-open silently if
CONFIG_IPE_PROP_BPF_SIGNATURE is disabled?

> diff --git a/security/ipe/policy_parser.c b/security/ipe/policy_parser.c
> index 6fa5bebf8471..71f63de56616 100644
> --- a/security/ipe/policy_parser.c
> +++ b/security/ipe/policy_parser.c
[ ... ]
> @@ -237,6 +237,7 @@ static const match_table_t operation_tokens = {
>  	{IPE_OP_KEXEC_INITRAMFS,	"op=KEXEC_INITRAMFS"},
>  	{IPE_OP_POLICY,			"op=POLICY"},
>  	{IPE_OP_X509,			"op=X509_CERT"},
> +	{IPE_OP_BPF_PROG_LOAD,		"op=BPF_PROG_LOAD"},
>  	{IPE_OP_INVALID,		NULL}
>  };

Because BPF_PROG_LOAD is added unconditionally to the parser, an administrator
could load a policy containing "DEFAULT op=BPF_PROG_LOAD action=DENY"
successfully.

However, because the LSM hooks are not registered, IPE will never evaluate
BPF program loads. This results in all BPF loads being silently allowed,
which contradicts the loaded policy.

Could the parser be updated to reject BPF_PROG_LOAD rules when the
corresponding enforcement hooks are disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260429191431.2345448-1-bboscaccy@linux.microsoft.com?part=10