Re: [PATCH bpf-next 10/13] bpf: Add the bpf_landlock_restrict_binprm kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Multiple calls to bpf_landlock_restrict_binprm() do not compose; a later call replaces a previously staged restriction instead of intersecting it, violating BPF LSM composition principles.
- [Medium] bpf_landlock_kfunc_filter incorrectly restricts the release kfunc bpf_landlock_put_ruleset to only the two bprm sleepable hooks, violating the stated design and preventing valid reference cleanup in other LSM hooks.
--

--- Patch [10]: [PATCH bpf-next 10/13] bpf: Add the bpf_landlock_restrict_binprm kfunc ---
commit fe3f43b90a0bd65c9f7f90c67126bdf361901840
Author: Justin Suess <utilityemal77@gmail.com>

bpf: Add the bpf_landlock_restrict_binprm kfunc

This commit adds the bpf_landlock_restrict_binprm kfunc to enforce Landlock
rulesets on execution credentials. The restriction is staged and committed at
the execution's point of no return. It also limits the enforcement to specific
LSM hooks.

> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 9ff1c35fcd6e5..67aa902f12570 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c

[ ... ]

> @@ -535,6 +535,42 @@ __bpf_kfunc void bpf_landlock_put_ruleset(struct bpf_landlock_ruleset *ruleset)
>  	security_policy_kptr_put(LSM_ID_LANDLOCK, &policy);
>  }
>  
> +/**
> + * bpf_landlock_restrict_binprm - Enforce a Landlock ruleset on exec
> + *                                credentials
> + * @bprm: execution context providing the prepared credentials to
> + *        restrict
> + * @ruleset: Landlock ruleset to enforce
> + * @flags: landlock_restrict_self(2) flags, except
> + *         %LANDLOCK_RESTRICT_SELF_TSYNC
> + *
> + * Restrict the credentials prepared in @bprm with @ruleset, so that
> + * the executed task starts confined by it, following the same domain
> + * composition rules as landlock_restrict_self(2).  The restriction is
> + * staged and only committed at the execution's point of no return: an
> + * execution that fails before that point is unaffected, and a later
> + * call on the same execution replaces a previously staged
> + * restriction.  @ruleset is only borrowed: the caller keeps its
> + * reference.
> + *
> + * The landlock_restrict_self(2) flags apply with their usual
> + * semantics.  Only %LANDLOCK_RESTRICT_SELF_TSYNC is rejected, as it
> + * targets the calling threads rather than the execution.
> + *
> + * Return: 0 on success, -EOPNOTSUPP if the Landlock LSM is not
> + * enabled, -EINVAL if @flags contains an unsupported flag, other
> + * negative values on failure as for landlock_restrict_self(2).
> + */
> +__bpf_kfunc int bpf_landlock_restrict_binprm(struct linux_binprm *bprm,
> +					     struct bpf_landlock_ruleset *ruleset,
> +					     u32 flags)
> +{
> +	union lsm_policy_kptr policy = { .landlock.ruleset = ruleset };
> +
> +	return security_bprm_enforce_policy_kptr(LSM_ID_LANDLOCK, bprm,
> +						 &policy, flags);

[Severity: High]
Does replacing instead of intersecting the ruleset break the BPF LSM
composition principles?

If multiple BPF LSM programs are attached to the same hook, a later call to
bpf_landlock_restrict_binprm() replaces a previously staged restriction
rather than layering or intersecting it.

This allows a subsequent program to completely bypass and discard the security
policy enforced by an earlier program.

> +}
> +

[ ... ]

> @@ -566,7 +605,10 @@ BTF_ID_LIST_SINGLE(bpf_landlock_get_ruleset_ids, func,
>   * in the fd table of the task that set the ruleset up, so
>   * bpf_landlock_get_ruleset_from_fd() is exclusive to syscall
>   * programs, which run in that task's context; an LSM program runs in
> - * the context of the task it mediates.
> + * the context of the task it mediates.  Enforcement is exclusive to
> + * the sleepable bprm LSM hooks the policy operation is specified
> + * for, and the release kfunc is allowed wherever a reference can be
> + * held.
>   */
>  static int bpf_landlock_kfunc_filter(const struct bpf_prog *prog, u32 kfunc_id)
>  {
> @@ -575,6 +617,8 @@ static int bpf_landlock_kfunc_filter(const struct bpf_prog *prog, u32 kfunc_id)
>  
>  	switch (prog->type) {
>  	case BPF_PROG_TYPE_SYSCALL:
> +		if (kfunc_id == bpf_landlock_restrict_binprm_ids[0])
> +			return -EACCES;
>  		return 0;
>  	case BPF_PROG_TYPE_LSM:
>  		if (kfunc_id == bpf_landlock_get_ruleset_ids[0])

[Severity: Medium]
Does the bpf_landlock_kfunc_filter() logic inadvertently restrict the release
kfunc as well?

The updated comment states that the release kfunc (bpf_landlock_put_ruleset)
is allowed wherever a reference can be held.

However, in the BPF_PROG_TYPE_LSM case, the filter still applies this check
to all kfuncs (including the release kfunc) instead of isolating it to the
restriction kfunc:

	if (!btf_id_set_contains(&bpf_landlock_kfunc_hooks,
				 prog->aux->attach_btf_id))
		return -EACCES;

This prevents BPF LSM programs attached to non-bprm hooks from calling
bpf_landlock_put_ruleset() to release acquired ruleset references.

Should there be an exemption for the release kfunc here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731022047.189137-1-utilityemal77@gmail.com?part=10