Re: [PATCH bpf-next v3 13/15] landlock: Implement the LSM policy object hooks

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 4479 bytes --]

> diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> index 88fa97fc3bd2..9a2971197892 100644
> --- a/security/landlock/cred.h
> +++ b/security/landlock/cred.h
> @@ -59,6 +59,16 @@ struct landlock_cred_security {
>  	 */
>  	struct landlock_domain *domain;
>
> +#ifdef CONFIG_BPF_LSM
> +	/**
> +	 * @staged: Restriction staged by the bprm_apply_policy_object() hook,
> +	 * owning its domain reference, applied at bprm_committing_creds().
> +	 * Only ever set on credentials prepared for an execution; committed
> +	 * task credentials never carry a staged restriction.
> +	 */
> +	struct landlock_restriction staged;
> +#endif /* CONFIG_BPF_LSM */
> +
>  #ifdef CONFIG_SECURITY_LANDLOCK_LOG

Does adding the staged field here grow struct landlock_file_security for
every open file?

struct landlock_cred_security is embedded by value in the file blob:

security/landlock/fs.h:88-90:
	struct landlock_cred_security fown_subject;

and security/landlock/setup.c sets .lbs_file = sizeof(struct
landlock_file_security), so that blob is allocated for every struct
file.

struct landlock_restriction is 16 bytes (domain pointer + u32 flags +
tail padding), so adding it grows landlock_cred_security from 11 bytes
(8+2+1) to 27 bytes (8+16+2+1), making struct landlock_file_security
grow from 32 to 48 bytes (+50%) when CONFIG_BPF_LSM=y.

The struct's kernel-doc says "This structure is packed to minimize the
size of struct landlock_file_security," but the new field is documented
as "Only ever set on credentials prepared for an execution; committed
task credentials never carry a staged restriction," so it is always
zero in the file blob.

hook_file_set_fowner() at security/landlock/fs.c copies this blob with
raw struct assignments, so the 16 exec-only bytes are copied on every
fcntl(F_SETOWN) and socket-owner set. hook_file_free_security() only
touches fown_subject.domain, never staged.

CONFIG_BPF_LSM=y is enabled by mainstream distributions alongside
CONFIG_SECURITY_LANDLOCK, making this the common configuration. Could
the exec-only staging state live in a separate structure that is not
embedded in the per-file blob?

> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 1465f8a5c464..22edb140bb7f 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h

[ ... ]

> @@ -146,6 +147,16 @@ struct landlock_rules {
>  	u32 num_rules;
>  };
>
> +#ifdef CONFIG_BPF_LSM
> +/*
> + * Landlock's lsm_policy_object types.  The namespace is private to
> + * Landlock; 0 stays reserved as "unset".
> + */
> +enum landlock_policy_type {
> +	LANDLOCK_POLICY_TYPE_RULESET = 1,
> +};
> +#endif /* CONFIG_BPF_LSM */
> +
>  /**
>   * struct landlock_ruleset - Landlock ruleset
>   *
> @@ -157,6 +168,17 @@ struct landlock_ruleset {
>  	 * @rules: Red-black tree storage for rules.
>  	 */
>  	struct landlock_rules rules;
> +
> +#ifdef CONFIG_BPF_LSM
> +	/**
> +	 * @policy_object: Identity under which the ruleset is handed out
> +	 * to BPF programs as a referenced kptr: the LSM policy kfuncs
> +	 * dispatch back to Landlock through its lsmid.  Kept outside the
> +	 * union with @work_free: RCU readers may read its lsmid while a
> +	 * queued free waits out the grace period.
> +	 */
> +	struct lsm_policy_object policy_object;
> +#endif /* CONFIG_BPF_LSM */
>  	/**
>  	 * @usage: Number of file descriptors referencing this ruleset.  Kept
>  	 * outside the union with @work_free: RCU readers may still call

Should the @usage documentation be updated?

The comment says "Number of file descriptors referencing this ruleset,"
but after this patch a reference can be held with no file descriptor
involved at all.

hook_policy_object_from_fd() calls landlock_get_ruleset_from_fd() to
increment @usage, then the BPF program parks that owned reference in a
map kptr field with bpf_kptr_xchg(). Userspace can close the ruleset fd
afterwards, leaving @usage at 1 with zero file descriptors referencing
the ruleset. hook_policy_object_get() also increments the refcount for
an RCU reader holding no fd.

The rest of this struct's documentation is maintained precisely (three
members carry "Kept outside the union with @work_free" rationales), so
the stale enumeration stands out.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34398515657