* Re: [PATCH v3 27/34] tomoyo: move initcalls to the LSM framework
From: John Johansen @ 2025-09-02 18:09 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-63-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Reviewed-by: Kees Cook <kees@kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/tomoyo/common.h | 2 ++
> security/tomoyo/securityfs_if.c | 4 +---
> security/tomoyo/tomoyo.c | 1 +
> 3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
> index 0e8e2e959aef..3b2a97d10a5d 100644
> --- a/security/tomoyo/common.h
> +++ b/security/tomoyo/common.h
> @@ -924,6 +924,8 @@ struct tomoyo_task {
>
> /********** Function prototypes. **********/
>
> +int tomoyo_interface_init(void);
> +
> bool tomoyo_address_matches_group(const bool is_ipv6, const __be32 *address,
> const struct tomoyo_group *group);
> bool tomoyo_compare_number_union(const unsigned long value,
> diff --git a/security/tomoyo/securityfs_if.c b/security/tomoyo/securityfs_if.c
> index 7e69747b2f77..33933645f5b9 100644
> --- a/security/tomoyo/securityfs_if.c
> +++ b/security/tomoyo/securityfs_if.c
> @@ -233,7 +233,7 @@ static void __init tomoyo_create_entry(const char *name, const umode_t mode,
> *
> * Returns 0.
> */
> -static int __init tomoyo_interface_init(void)
> +int __init tomoyo_interface_init(void)
> {
> struct tomoyo_domain_info *domain;
> struct dentry *tomoyo_dir;
> @@ -269,5 +269,3 @@ static int __init tomoyo_interface_init(void)
> tomoyo_load_builtin_policy();
> return 0;
> }
> -
> -fs_initcall(tomoyo_interface_init);
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index ed0f7b052a85..a015cf0c4a00 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -617,4 +617,5 @@ DEFINE_LSM(tomoyo) = {
> .flags = LSM_FLAG_LEGACY_MAJOR,
> .blobs = &tomoyo_blob_sizes,
> .init = tomoyo_init,
> + .initcall_fs = tomoyo_interface_init,
> };
^ permalink raw reply
* Re: [PATCH v3 28/34] safesetid: move initcalls to the LSM framework
From: John Johansen @ 2025-09-02 18:10 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-64-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Reviewed-by: Kees Cook <kees@kernel.org>
> Acked-by: Micah Morton <mortonm@chromium.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/safesetid/lsm.c | 1 +
> security/safesetid/lsm.h | 2 ++
> security/safesetid/securityfs.c | 3 +--
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index 9a7c68d4e642..d5fb949050dd 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -289,4 +289,5 @@ static int __init safesetid_security_init(void)
> DEFINE_LSM(safesetid_security_init) = {
> .id = &safesetid_lsmid,
> .init = safesetid_security_init,
> + .initcall_fs = safesetid_init_securityfs,
> };
> diff --git a/security/safesetid/lsm.h b/security/safesetid/lsm.h
> index d346f4849cea..bf5172e2c3f7 100644
> --- a/security/safesetid/lsm.h
> +++ b/security/safesetid/lsm.h
> @@ -70,4 +70,6 @@ enum sid_policy_type _setid_policy_lookup(struct setid_ruleset *policy,
> extern struct setid_ruleset __rcu *safesetid_setuid_rules;
> extern struct setid_ruleset __rcu *safesetid_setgid_rules;
>
> +int safesetid_init_securityfs(void);
> +
> #endif /* _SAFESETID_H */
> diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
> index 8e1ffd70b18a..ece259f75b0d 100644
> --- a/security/safesetid/securityfs.c
> +++ b/security/safesetid/securityfs.c
> @@ -308,7 +308,7 @@ static const struct file_operations safesetid_gid_file_fops = {
> .write = safesetid_gid_file_write,
> };
>
> -static int __init safesetid_init_securityfs(void)
> +int __init safesetid_init_securityfs(void)
> {
> int ret;
> struct dentry *policy_dir;
> @@ -345,4 +345,3 @@ static int __init safesetid_init_securityfs(void)
> securityfs_remove(policy_dir);
> return ret;
> }
> -fs_initcall(safesetid_init_securityfs);
^ permalink raw reply
* Re: [PATCH v3 30/34] lockdown: move initcalls to the LSM framework
From: John Johansen @ 2025-09-02 18:12 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-66-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Reviewed-by: Kees Cook <kees@kernel.org>
> Acked-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/lockdown/lockdown.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 4813f168ff93..8d46886d2cca 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -161,8 +161,6 @@ static int __init lockdown_secfs_init(void)
> return PTR_ERR_OR_ZERO(dentry);
> }
>
> -core_initcall(lockdown_secfs_init);
> -
> #ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY
> DEFINE_EARLY_LSM(lockdown) = {
> #else
> @@ -170,4 +168,5 @@ DEFINE_LSM(lockdown) = {
> #endif
> .id = &lockdown_lsmid,
> .init = lockdown_lsm_init,
> + .initcall_core = lockdown_secfs_init,
> };
^ permalink raw reply
* Re: [PATCH v3 33/34] lsm: consolidate all of the LSM framework initcalls
From: John Johansen @ 2025-09-02 18:20 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-69-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> The LSM framework itself registers a small number of initcalls, this
> patch converts these initcalls into the new initcall mechanism.
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/inode.c | 3 +--
> security/lsm.h | 4 ++++
> security/lsm_init.c | 14 ++++++++++++--
> security/min_addr.c | 5 +++--
> 4 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/security/inode.c b/security/inode.c
> index a5e7a073e672..21b1f9b4d396 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -384,7 +384,7 @@ static const struct file_operations lsm_ops = {
> };
> #endif
>
> -static int __init securityfs_init(void)
> +int __init securityfs_init(void)
> {
> int retval;
>
> @@ -403,4 +403,3 @@ static int __init securityfs_init(void)
> #endif
> return 0;
> }
> -core_initcall(securityfs_init);
> diff --git a/security/lsm.h b/security/lsm.h
> index 8dc267977ae0..436219260376 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -35,4 +35,8 @@ extern struct kmem_cache *lsm_inode_cache;
> int lsm_cred_alloc(struct cred *cred, gfp_t gfp);
> int lsm_task_alloc(struct task_struct *task);
>
> +/* LSM framework initializers */
> +int securityfs_init(void);
> +int min_addr_init(void);
> +
> #endif /* _LSM_H_ */
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 697482a22a02..2bd705836df8 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -488,7 +488,12 @@ int __init security_init(void)
> */
> static int __init security_initcall_pure(void)
> {
> - return lsm_initcall(pure);
> + int rc_adr, rc_lsm;
> +
> + rc_adr = min_addr_init();
> + rc_lsm = lsm_initcall(pure);
> +
> + return (rc_adr ? rc_adr : rc_lsm);
> }
> pure_initcall(security_initcall_pure);
>
> @@ -506,7 +511,12 @@ early_initcall(security_initcall_early);
> */
> static int __init security_initcall_core(void)
> {
> - return lsm_initcall(core);
> + int rc_sfs, rc_lsm;
> +
> + rc_sfs = securityfs_init();
> + rc_lsm = lsm_initcall(core);
> +
> + return (rc_sfs ? rc_sfs : rc_lsm);
> }
> core_initcall(security_initcall_core);
>
> diff --git a/security/min_addr.c b/security/min_addr.c
> index df1bc643d886..40714bdeefbe 100644
> --- a/security/min_addr.c
> +++ b/security/min_addr.c
> @@ -4,6 +4,8 @@
> #include <linux/security.h>
> #include <linux/sysctl.h>
>
> +#include "lsm.h"
> +
> /* amount of vm to protect from userspace access by both DAC and the LSM*/
> unsigned long mmap_min_addr;
> /* amount of vm to protect from userspace using CAP_SYS_RAWIO (DAC) */
> @@ -54,11 +56,10 @@ static const struct ctl_table min_addr_sysctl_table[] = {
> },
> };
>
> -static int __init init_mmap_min_addr(void)
> +int __init min_addr_init(void)
> {
> register_sysctl_init("vm", min_addr_sysctl_table);
> update_mmap_min_addr();
>
> return 0;
> }
> -pure_initcall(init_mmap_min_addr);
^ permalink raw reply
* Re: [PATCH v3 34/34] lsm: add a LSM_STARTED_ALL notification event
From: John Johansen @ 2025-09-02 18:21 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-70-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Add a new LSM notifier event, LSM_STARTED_ALL, which is fired once at
> boot when all of the LSMs have been started.
>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> include/linux/security.h | 1 +
> security/lsm_init.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 8560c50edd2e..c13f0a849024 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -85,6 +85,7 @@ struct timezone;
>
> enum lsm_event {
> LSM_POLICY_CHANGE,
> + LSM_STARTED_ALL,
> };
>
> struct dm_verity_digest {
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 2bd705836df8..af4046c5c581 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -556,6 +556,7 @@ static int __init security_initcall_late(void)
>
> rc = lsm_initcall(late);
> lsm_pr_dbg("all enabled LSMs fully activated\n");
> + call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
>
> return rc;
> }
^ permalink raw reply
* Re: [PATCH v3 14/34] lsm: rename/rework append_ordered_lsm() into lsm_order_append()
From: John Johansen @ 2025-09-03 0:17 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-50-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Rename append_ordered_lsm() to lsm_order_append() to better match
> convention and do some rework. The rework includes moving the
> LSM_FLAG_EXCLUSIVE logic from lsm_prepare() to lsm_order_append()
> in order to consolidate the individual LSM append/activation code,
> and adding logic to skip appending explicitly disabled LSMs to the
> active LSM list.
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/lsm_init.c | 76 +++++++++++++++++++++++++--------------------
> 1 file changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index ec46ddb18e1e..a314484d7c2f 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -124,24 +124,48 @@ static bool __init lsm_order_exists(struct lsm_info *lsm)
> return false;
> }
>
> -/* Append an LSM to the list of ordered LSMs to initialize. */
> -static int last_lsm __initdata;
> -static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from)
> +/**
> + * lsm_order_append - Append a LSM to the ordered list
> + * @lsm: LSM definition
> + * @src: source of the addition
> + *
> + * Append @lsm to the enabled LSM array after ensuring that it hasn't been
> + * explicitly disabled, is a duplicate entry, or would run afoul of the
> + * LSM_FLAG_EXCLUSIVE logic.
> + */
> +static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
> {
> /* Ignore duplicate selections. */
> if (lsm_order_exists(lsm))
> return;
>
> - if (WARN(last_lsm == MAX_LSM_COUNT, "%s: out of LSM static calls!?\n", from))
> - return;
> + /* Skip explicitly disabled LSMs. */
> + if (lsm->enabled && !lsm_is_enabled(lsm))
> + goto out;
>
> - /* Enable this LSM, if it is not already set. */
> - if (!lsm->enabled)
> - lsm->enabled = &lsm_enabled_true;
> - lsm_order[last_lsm] = lsm;
> - lsm_idlist[last_lsm++] = lsm->id;
> + if (WARN(lsm_active_cnt == MAX_LSM_COUNT,
> + "%s: out of LSM static calls!?\n", src)) {
> + lsm_enabled_set(lsm, false);
> + goto out;
> + }
>
> - init_debug("%s ordered: %s (%s)\n", from, lsm->id->name,
> + if (lsm->flags & LSM_FLAG_EXCLUSIVE) {
> + if (lsm_exclusive) {
> + init_debug("exclusive disabled: %s\n", lsm->id->name);
> + lsm_enabled_set(lsm, false);
> + goto out;
> + } else {
> + init_debug("exclusive chosen: %s\n", lsm->id->name);
> + lsm_exclusive = lsm;
> + }
> + }
> +
> + lsm_enabled_set(lsm, true);
> + lsm_order[lsm_active_cnt] = lsm;
> + lsm_idlist[lsm_active_cnt++] = lsm->id;
> +
> +out:
> + init_debug("%s ordered: %s (%s)\n", src, lsm->id->name,
> lsm_is_enabled(lsm) ? "enabled" : "disabled");
> }
>
> @@ -163,26 +187,12 @@ static void __init lsm_set_blob_size(int *need, int *lbs)
> */
> static void __init lsm_prepare(struct lsm_info *lsm)
> {
> - struct lsm_blob_sizes *blobs;
> + struct lsm_blob_sizes *blobs = lsm->blobs;
>
> - if (!lsm_is_enabled(lsm)) {
> - lsm_enabled_set(lsm, false);
> + if (!blobs)
> return;
> - } else if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && lsm_exclusive) {
> - init_debug("exclusive disabled: %s\n", lsm->id->name);
> - lsm_enabled_set(lsm, false);
> - return;
> - }
> -
> - /* Mark the LSM as enabled. */
> - lsm_enabled_set(lsm, true);
> - if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && !lsm_exclusive) {
> - init_debug("exclusive chosen: %s\n", lsm->id->name);
> - lsm_exclusive = lsm;
> - }
>
> /* Register the LSM blob sizes. */
> - blobs = lsm->blobs;
> lsm_set_blob_size(&blobs->lbs_cred, &blob_sizes.lbs_cred);
> lsm_set_blob_size(&blobs->lbs_file, &blob_sizes.lbs_file);
> lsm_set_blob_size(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> @@ -227,7 +237,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
> /* LSM_ORDER_FIRST is always first. */
> lsm_for_each_raw(lsm) {
> if (lsm->order == LSM_ORDER_FIRST)
> - append_ordered_lsm(lsm, " first");
> + lsm_order_append(lsm, " first");
> }
>
> /* Process "security=", if given. */
> @@ -259,7 +269,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
> lsm_for_each_raw(lsm) {
> if (strcmp(lsm->id->name, name) == 0) {
> if (lsm->order == LSM_ORDER_MUTABLE)
> - append_ordered_lsm(lsm, origin);
> + lsm_order_append(lsm, origin);
> found = true;
> }
> }
> @@ -275,14 +285,14 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
> if (lsm_order_exists(lsm))
> continue;
> if (strcmp(lsm->id->name, lsm_order_legacy) == 0)
> - append_ordered_lsm(lsm, "security=");
> + lsm_order_append(lsm, "security=");
> }
> }
>
> /* LSM_ORDER_LAST is always last. */
> lsm_for_each_raw(lsm) {
> if (lsm->order == LSM_ORDER_LAST)
> - append_ordered_lsm(lsm, " last");
> + lsm_order_append(lsm, " last");
> }
>
> /* Disable all LSMs not in the ordered list. */
> @@ -415,8 +425,8 @@ int __init early_security_init(void)
> struct lsm_info *lsm;
>
> lsm_early_for_each_raw(lsm) {
> - if (!lsm->enabled)
> - lsm->enabled = &lsm_enabled_true;
> + lsm_enabled_set(lsm, true);
> + lsm_order_append(lsm, "early");
> lsm_prepare(lsm);
> initialize_lsm(lsm);
> }
^ permalink raw reply
* Re: [PATCH v3 15/34] lsm: rename/rework ordered_lsm_parse() to lsm_order_parse()
From: John Johansen @ 2025-09-03 8:40 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-51-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Rename ordered_lsm_parse() to lsm_order_parse() for the sake of
> consistency with the other LSM initialization routines, and also
> do some minor rework of the function. Aside from some minor style
> decisions, the majority of the rework involved shuffling the order
> of the LSM_FLAG_LEGACY and LSM_ORDER_FIRST code so that the
> LSM_FLAG_LEGACY checks are handled first; it is important to note
> that this doesn't affect the order in which the LSMs are registered.
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/lsm_init.c | 82 ++++++++++++++++++++-------------------------
> 1 file changed, 37 insertions(+), 45 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index a314484d7c2f..7b2491120fc8 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -228,83 +228,75 @@ static void __init initialize_lsm(struct lsm_info *lsm)
> }
> }
>
> -/* Populate ordered LSMs list from comma-separated LSM name list. */
> -static void __init ordered_lsm_parse(const char *order, const char *origin)
> +/**
> + * lsm_order_parse - Parse the comma delimited LSM list
> + * @list: LSM list
> + * @src: source of the list
> + */
> +static void __init lsm_order_parse(const char *list, const char *src)
> {
> struct lsm_info *lsm;
> char *sep, *name, *next;
>
> - /* LSM_ORDER_FIRST is always first. */
> - lsm_for_each_raw(lsm) {
> - if (lsm->order == LSM_ORDER_FIRST)
> - lsm_order_append(lsm, " first");
> - }
> -
> - /* Process "security=", if given. */
> + /* Handle any Legacy LSM exclusions if one was specified. */
> if (lsm_order_legacy) {
> - struct lsm_info *major;
> -
> /*
> - * To match the original "security=" behavior, this
> - * explicitly does NOT fallback to another Legacy Major
> - * if the selected one was separately disabled: disable
> - * all non-matching Legacy Major LSMs.
> + * To match the original "security=" behavior, this explicitly
> + * does NOT fallback to another Legacy Major if the selected
> + * one was separately disabled: disable all non-matching
> + * Legacy Major LSMs.
> */
> - lsm_for_each_raw(major) {
> - if ((major->flags & LSM_FLAG_LEGACY_MAJOR) &&
> - strcmp(major->id->name, lsm_order_legacy) != 0) {
> - lsm_enabled_set(major, false);
> + lsm_for_each_raw(lsm) {
> + if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) &&
> + strcmp(lsm->id->name, lsm_order_legacy)) {
> + lsm_enabled_set(lsm, false);
> init_debug("security=%s disabled: %s (only one legacy major LSM)\n",
> - lsm_order_legacy, major->id->name);
> + lsm_order_legacy, lsm->id->name);
> }
> }
> }
>
> - sep = kstrdup(order, GFP_KERNEL);
> + /* LSM_ORDER_FIRST */
> + lsm_for_each_raw(lsm) {
> + if (lsm->order == LSM_ORDER_FIRST)
> + lsm_order_append(lsm, "first");
> + }
> +
> + /* Normal or "mutable" LSMs */
> + sep = kstrdup(list, GFP_KERNEL);
> next = sep;
> /* Walk the list, looking for matching LSMs. */
> while ((name = strsep(&next, ",")) != NULL) {
> - bool found = false;
> -
> lsm_for_each_raw(lsm) {
> - if (strcmp(lsm->id->name, name) == 0) {
> - if (lsm->order == LSM_ORDER_MUTABLE)
> - lsm_order_append(lsm, origin);
> - found = true;
> - }
> + if (!strcmp(lsm->id->name, name) &&
> + lsm->order == LSM_ORDER_MUTABLE)
> + lsm_order_append(lsm, src);
> }
> -
> - if (!found)
> - init_debug("%s ignored: %s (not built into kernel)\n",
> - origin, name);
> }
> + kfree(sep);
>
> - /* Process "security=", if given. */
> + /* Legacy LSM if specified. */
> if (lsm_order_legacy) {
> lsm_for_each_raw(lsm) {
> - if (lsm_order_exists(lsm))
> - continue;
> - if (strcmp(lsm->id->name, lsm_order_legacy) == 0)
> - lsm_order_append(lsm, "security=");
> + if (!strcmp(lsm->id->name, lsm_order_legacy))
> + lsm_order_append(lsm, src);
> }
> }
>
> - /* LSM_ORDER_LAST is always last. */
> + /* LSM_ORDER_LAST */
> lsm_for_each_raw(lsm) {
> if (lsm->order == LSM_ORDER_LAST)
> - lsm_order_append(lsm, " last");
> + lsm_order_append(lsm, "last");
> }
>
> - /* Disable all LSMs not in the ordered list. */
> + /* Disable all LSMs not previously enabled. */
> lsm_for_each_raw(lsm) {
> if (lsm_order_exists(lsm))
> continue;
> lsm_enabled_set(lsm, false);
> init_debug("%s skipped: %s (not in requested order)\n",
> - origin, lsm->id->name);
> + src, lsm->id->name);
> }
> -
> - kfree(sep);
> }
>
> /**
> @@ -322,9 +314,9 @@ static void __init lsm_init_ordered(void)
> lsm_order_legacy, lsm_order_cmdline);
> lsm_order_legacy = NULL;
> }
> - ordered_lsm_parse(lsm_order_cmdline, "cmdline");
> + lsm_order_parse(lsm_order_cmdline, "cmdline");
> } else
> - ordered_lsm_parse(lsm_order_builtin, "builtin");
> + lsm_order_parse(lsm_order_builtin, "builtin");
>
> lsm_order_for_each(lsm) {
> lsm_prepare(*lsm);
^ permalink raw reply
* Re: [PATCH v3 18/34] lsm: fold lsm_init_ordered() into security_init()
From: John Johansen @ 2025-09-03 8:47 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-54-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> With only security_init() calling lsm_init_ordered, it makes little
> sense to keep lsm_init_ordered() as a standalone function. Fold
> lsm_init_ordered() into security_init().
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/lsm_init.c | 159 ++++++++++++++++++++------------------------
> 1 file changed, 73 insertions(+), 86 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 1f64222925c1..f87f5441617b 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -18,6 +18,9 @@ static __initdata int lsm_enabled_false = 0;
> extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
> extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
>
> +/* Number of "early" LSMs */
> +static __initdata unsigned int lsm_count_early;
> +
> /* Build and boot-time LSM ordering. */
> static __initconst const char *const lsm_order_builtin = CONFIG_LSM;
> static __initdata const char *lsm_order_cmdline;
> @@ -169,7 +172,6 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
> lsm_is_enabled(lsm) ? "enabled" : "disabled");
> }
>
> -
> /**
> * lsm_blob_size_update - Update the LSM blob size and offset information
> * @sz_req: the requested additional blob size
> @@ -313,14 +315,74 @@ static void __init lsm_order_parse(const char *list, const char *src)
> }
> }
>
> -/**
> - * lsm_init_ordered - Initialize the ordered LSMs
> - */
> -static void __init lsm_init_ordered(void)
> +static void __init lsm_static_call_init(struct security_hook_list *hl)
> {
> - unsigned int first = 0;
> + struct lsm_static_call *scall = hl->scalls;
> + int i;
> +
> + for (i = 0; i < MAX_LSM_COUNT; i++) {
> + /* Update the first static call that is not used yet */
> + if (!scall->hl) {
> + __static_call_update(scall->key, scall->trampoline,
> + hl->hook.lsm_func_addr);
> + scall->hl = hl;
> + static_branch_enable(scall->active);
> + return;
> + }
> + scall++;
> + }
> + panic("%s - Ran out of static slots.\n", __func__);
> +}
> +
> +/**
> + * security_add_hooks - Add a modules hooks to the hook lists.
> + * @hooks: the hooks to add
> + * @count: the number of hooks to add
> + * @lsmid: the identification information for the security module
> + *
> + * Each LSM has to register its hooks with the infrastructure.
> + */
> +void __init security_add_hooks(struct security_hook_list *hooks, int count,
> + const struct lsm_id *lsmid)
> +{
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + hooks[i].lsmid = lsmid;
> + lsm_static_call_init(&hooks[i]);
> + }
> +}
> +
> +int __init early_security_init(void)
> +{
> + struct lsm_info *lsm;
> +
> + lsm_early_for_each_raw(lsm) {
> + lsm_enabled_set(lsm, true);
> + lsm_order_append(lsm, "early");
> + lsm_prepare(lsm);
> + lsm_init_single(lsm);
> + lsm_count_early++;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * security_init - Initializes the LSM framework
> + *
> + * This should be called early in the kernel initialization sequence.
> + */
> +int __init security_init(void)
> +{
> + unsigned int cnt;
> struct lsm_info **lsm;
> struct lsm_info *early;
> + unsigned int first = 0;
> +
> + init_debug("legacy security=%s\n", lsm_order_legacy ? : " *unspecified*");
> + init_debug(" CONFIG_LSM=%s\n", lsm_order_builtin);
> + init_debug("boot arg lsm=%s\n", lsm_order_cmdline ? : " *unspecified*");
>
> if (lsm_order_cmdline) {
> if (lsm_order_legacy) {
> @@ -332,9 +394,8 @@ static void __init lsm_init_ordered(void)
> } else
> lsm_order_parse(lsm_order_builtin, "builtin");
>
> - lsm_order_for_each(lsm) {
> + lsm_order_for_each(lsm)
> lsm_prepare(*lsm);
> - }
>
> pr_info("initializing lsm=");
> lsm_early_for_each_raw(early) {
> @@ -383,87 +444,13 @@ static void __init lsm_init_ordered(void)
> if (lsm_task_alloc(current))
> panic("%s: early task alloc failed.\n", __func__);
>
> + cnt = 0;
> lsm_order_for_each(lsm) {
> + /* skip the "early" LSMs as they have already been setup */
> + if (cnt++ < lsm_count_early)
> + continue;
> lsm_init_single(*lsm);
> }
> -}
> -
> -static void __init lsm_static_call_init(struct security_hook_list *hl)
> -{
> - struct lsm_static_call *scall = hl->scalls;
> - int i;
> -
> - for (i = 0; i < MAX_LSM_COUNT; i++) {
> - /* Update the first static call that is not used yet */
> - if (!scall->hl) {
> - __static_call_update(scall->key, scall->trampoline,
> - hl->hook.lsm_func_addr);
> - scall->hl = hl;
> - static_branch_enable(scall->active);
> - return;
> - }
> - scall++;
> - }
> - panic("%s - Ran out of static slots.\n", __func__);
> -}
> -
> -/**
> - * security_add_hooks - Add a modules hooks to the hook lists.
> - * @hooks: the hooks to add
> - * @count: the number of hooks to add
> - * @lsmid: the identification information for the security module
> - *
> - * Each LSM has to register its hooks with the infrastructure.
> - */
> -void __init security_add_hooks(struct security_hook_list *hooks, int count,
> - const struct lsm_id *lsmid)
> -{
> - int i;
> -
> - for (i = 0; i < count; i++) {
> - hooks[i].lsmid = lsmid;
> - lsm_static_call_init(&hooks[i]);
> - }
> -}
> -
> -int __init early_security_init(void)
> -{
> - struct lsm_info *lsm;
> -
> - lsm_early_for_each_raw(lsm) {
> - lsm_enabled_set(lsm, true);
> - lsm_order_append(lsm, "early");
> - lsm_prepare(lsm);
> - lsm_init_single(lsm);
> - }
> -
> - return 0;
> -}
> -
> -/**
> - * security_init - initializes the security framework
> - *
> - * This should be called early in the kernel initialization sequence.
> - */
> -int __init security_init(void)
> -{
> - struct lsm_info *lsm;
> -
> - init_debug("legacy security=%s\n", lsm_order_legacy ? : " *unspecified*");
> - init_debug(" CONFIG_LSM=%s\n", lsm_order_builtin);
> - init_debug("boot arg lsm=%s\n", lsm_order_cmdline ? : " *unspecified*");
> -
> - /*
> - * Append the names of the early LSM modules now that kmalloc() is
> - * available
> - */
> - lsm_early_for_each_raw(lsm) {
> - init_debug(" early started: %s (%s)\n", lsm->id->name,
> - lsm_is_enabled(lsm) ? "enabled" : "disabled");
> - }
> -
> - /* Load LSMs in specified order. */
> - lsm_init_ordered();
>
> return 0;
> }
^ permalink raw reply
* Re: [PATCH v3 19/34] lsm: add/tweak function header comment blocks in lsm_init.c
From: John Johansen @ 2025-09-03 8:48 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-55-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Add function header comments for lsm_static_call_init() and
> early_security_init(), tweak the existing comment block for
> security_add_hooks().
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/lsm_init.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index f87f5441617b..37593805ba9e 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -315,6 +315,10 @@ static void __init lsm_order_parse(const char *list, const char *src)
> }
> }
>
> +/**
> + * lsm_static_call_init - Initialize a LSM's static calls
> + * @hl: LSM hook list
> + */
> static void __init lsm_static_call_init(struct security_hook_list *hl)
> {
> struct lsm_static_call *scall = hl->scalls;
> @@ -335,12 +339,12 @@ static void __init lsm_static_call_init(struct security_hook_list *hl)
> }
>
> /**
> - * security_add_hooks - Add a modules hooks to the hook lists.
> - * @hooks: the hooks to add
> - * @count: the number of hooks to add
> - * @lsmid: the identification information for the security module
> + * security_add_hooks - Add a LSM's hooks to the LSM framework's hook lists
> + * @hooks: LSM hooks to add
> + * @count: number of hooks to add
> + * @lsmid: identification information for the LSM
> *
> - * Each LSM has to register its hooks with the infrastructure.
> + * Each LSM has to register its hooks with the LSM framework.
> */
> void __init security_add_hooks(struct security_hook_list *hooks, int count,
> const struct lsm_id *lsmid)
> @@ -353,6 +357,9 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> }
> }
>
> +/**
> + * early_security_init - Initialize the early LSMs
> + */
> int __init early_security_init(void)
> {
> struct lsm_info *lsm;
^ permalink raw reply
* Re: [PATCH v3 21/34] lsm: output available LSMs when debugging
From: John Johansen @ 2025-09-03 8:49 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-57-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> This will display all of the LSMs built into the kernel, regardless
> of if they are enabled or not.
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/lsm_init.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 2f7ae26fca0f..c6aea57a01e6 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -363,6 +363,8 @@ int __init early_security_init(void)
> {
> struct lsm_info *lsm;
>
> + /* NOTE: lsm_pr_dbg() doesn't work here as lsm_debug is not yet set */
> +
> lsm_early_for_each_raw(lsm) {
> lsm_enabled_set(lsm, true);
> lsm_order_append(lsm, "early");
> @@ -385,9 +387,24 @@ int __init security_init(void)
> struct lsm_info **lsm;
>
> if (lsm_debug) {
> - lsm_pr("built-in LSM list: %s\n", lsm_order_builtin);
> + struct lsm_info *i;
> +
> + cnt = 0;
> + lsm_pr("available LSMs: ");
> + lsm_early_for_each_raw(i)
> + lsm_pr_cont("%s%s(E)", (cnt++ ? "," : ""), i->id->name);
> + lsm_for_each_raw(i)
> + lsm_pr_cont("%s%s", (cnt++ ? "," : ""), i->id->name);
> + lsm_pr_cont("\n");
> +
> + lsm_pr("built-in LSM config: %s\n", lsm_order_builtin);
> +
> lsm_pr("legacy LSM parameter: %s\n", lsm_order_legacy);
> lsm_pr("boot LSM parameter: %s\n", lsm_order_cmdline);
> +
> + /* see the note about lsm_pr_dbg() in early_security_init() */
> + lsm_early_for_each_raw(i)
> + lsm_pr("enabled LSM early:%s\n", i->id->name);
> }
>
> if (lsm_order_cmdline) {
^ permalink raw reply
* Re: [PATCH v3 20/34] lsm: cleanup the debug and console output in lsm_init.c
From: John Johansen @ 2025-09-03 10:22 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-56-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Move away from an init specific init_debug() macro to a more general
> lsm_pr()/lsm_pr_cont()/lsm_pr_dbg() set of macros that are available
> both before and after init. In the process we do a number of minor
> changes to improve the LSM initialization output and cleanup the code
> somewhat.
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> security/lsm.h | 11 ++++
> security/lsm_init.c | 123 +++++++++++++++++++-------------------------
> security/security.c | 2 +
> 3 files changed, 66 insertions(+), 70 deletions(-)
>
> diff --git a/security/lsm.h b/security/lsm.h
> index dbe755c45e57..8dc267977ae0 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -6,9 +6,20 @@
> #ifndef _LSM_H_
> #define _LSM_H_
>
> +#include <linux/printk.h>
> #include <linux/lsm_hooks.h>
> #include <linux/lsm_count.h>
>
> +/* LSM debugging */
> +extern bool lsm_debug;
> +#define lsm_pr(...) pr_info(__VA_ARGS__)
> +#define lsm_pr_cont(...) pr_cont(__VA_ARGS__)
> +#define lsm_pr_dbg(...) \
> + do { \
> + if (lsm_debug) \
> + pr_info(__VA_ARGS__); \
> + } while (0)
> +
> /* List of configured LSMs */
> extern unsigned int lsm_active_cnt;
> extern const struct lsm_id *lsm_idlist[];
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 37593805ba9e..2f7ae26fca0f 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -30,13 +30,6 @@ static __initdata const char *lsm_order_legacy;
> static __initdata struct lsm_info *lsm_order[MAX_LSM_COUNT + 1];
> static __initdata struct lsm_info *lsm_exclusive;
>
> -static __initdata bool debug;
> -#define init_debug(...) \
> - do { \
> - if (debug) \
> - pr_info(__VA_ARGS__); \
> - } while (0)
> -
> #define lsm_order_for_each(iter) \
> for ((iter) = lsm_order; *(iter); (iter)++)
> #define lsm_for_each_raw(iter) \
> @@ -77,7 +70,7 @@ __setup("lsm=", lsm_choose_lsm);
> */
> static int __init lsm_debug_enable(char *str)
> {
> - debug = true;
> + lsm_debug = true;
> return 1;
> }
> __setup("lsm.debug", lsm_debug_enable);
> @@ -143,22 +136,28 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
> return;
>
> /* Skip explicitly disabled LSMs. */
> - if (lsm->enabled && !lsm_is_enabled(lsm))
> - goto out;
> + if (lsm->enabled && !lsm_is_enabled(lsm)) {
> + lsm_pr_dbg("skip previously disabled LSM %s:%s\n",
> + src, lsm->id->name);
> + return;
> + }
>
> - if (WARN(lsm_active_cnt == MAX_LSM_COUNT,
> - "%s: out of LSM static calls!?\n", src)) {
> + if (lsm_active_cnt == MAX_LSM_COUNT) {
> + pr_warn("exceeded maximum LSM count on %s:%s\n",
> + src, lsm->id->name);
> lsm_enabled_set(lsm, false);
> - goto out;
> + return;
> }
>
> if (lsm->flags & LSM_FLAG_EXCLUSIVE) {
> if (lsm_exclusive) {
> - init_debug("exclusive disabled: %s\n", lsm->id->name);
> + lsm_pr_dbg("skip exclusive LSM conflict %s:%s\n",
> + src, lsm->id->name);
> lsm_enabled_set(lsm, false);
> - goto out;
> + return;
> } else {
> - init_debug("exclusive chosen: %s\n", lsm->id->name);
> + lsm_pr_dbg("select exclusive LSM %s:%s\n",
> + src, lsm->id->name);
> lsm_exclusive = lsm;
> }
> }
> @@ -167,9 +166,7 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
> lsm_order[lsm_active_cnt] = lsm;
> lsm_idlist[lsm_active_cnt++] = lsm->id;
>
> -out:
> - init_debug("%s ordered: %s (%s)\n", src, lsm->id->name,
> - lsm_is_enabled(lsm) ? "enabled" : "disabled");
> + lsm_pr_dbg("enabling LSM %s:%s\n", src, lsm->id->name);
> }
>
> /**
> @@ -239,7 +236,7 @@ static void __init lsm_init_single(struct lsm_info *lsm)
> if (!lsm_is_enabled(lsm))
> return;
>
> - init_debug("initializing %s\n", lsm->id->name);
> + lsm_pr_dbg("initializing %s\n", lsm->id->name);
> ret = lsm->init();
> WARN(ret, "%s failed to initialize: %d\n", lsm->id->name, ret);
> }
> @@ -266,8 +263,8 @@ static void __init lsm_order_parse(const char *list, const char *src)
> if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) &&
> strcmp(lsm->id->name, lsm_order_legacy)) {
> lsm_enabled_set(lsm, false);
> - init_debug("security=%s disabled: %s (only one legacy major LSM)\n",
> - lsm_order_legacy, lsm->id->name);
> + lsm_pr_dbg("skip legacy LSM conflict %s:%s\n",
> + src, lsm->id->name);
> }
> }
> }
> @@ -310,8 +307,7 @@ static void __init lsm_order_parse(const char *list, const char *src)
> if (lsm_order_exists(lsm))
> continue;
> lsm_enabled_set(lsm, false);
> - init_debug("%s skipped: %s (not in requested order)\n",
> - src, lsm->id->name);
> + lsm_pr_dbg("skip disabled LSM %s:%s\n", src, lsm->id->name);
> }
> }
>
> @@ -319,7 +315,7 @@ static void __init lsm_order_parse(const char *list, const char *src)
> * lsm_static_call_init - Initialize a LSM's static calls
> * @hl: LSM hook list
> */
> -static void __init lsm_static_call_init(struct security_hook_list *hl)
> +static int __init lsm_static_call_init(struct security_hook_list *hl)
> {
> struct lsm_static_call *scall = hl->scalls;
> int i;
> @@ -331,11 +327,12 @@ static void __init lsm_static_call_init(struct security_hook_list *hl)
> hl->hook.lsm_func_addr);
> scall->hl = hl;
> static_branch_enable(scall->active);
> - return;
> + return 0;
> }
> scall++;
> }
> - panic("%s - Ran out of static slots.\n", __func__);
> +
> + return -ENOSPC;
> }
>
> /**
> @@ -353,7 +350,9 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>
> for (i = 0; i < count; i++) {
> hooks[i].lsmid = lsmid;
> - lsm_static_call_init(&hooks[i]);
> + if (lsm_static_call_init(&hooks[i]))
> + panic("exhausted LSM callback slots with LSM %s\n",
> + lsmid->name);
> }
> }
>
> @@ -384,19 +383,16 @@ int __init security_init(void)
> {
> unsigned int cnt;
> struct lsm_info **lsm;
> - struct lsm_info *early;
> - unsigned int first = 0;
>
> - init_debug("legacy security=%s\n", lsm_order_legacy ? : " *unspecified*");
> - init_debug(" CONFIG_LSM=%s\n", lsm_order_builtin);
> - init_debug("boot arg lsm=%s\n", lsm_order_cmdline ? : " *unspecified*");
> + if (lsm_debug) {
> + lsm_pr("built-in LSM list: %s\n", lsm_order_builtin);
> + lsm_pr("legacy LSM parameter: %s\n", lsm_order_legacy);
> + lsm_pr("boot LSM parameter: %s\n", lsm_order_cmdline);
> + }
>
> if (lsm_order_cmdline) {
> - if (lsm_order_legacy) {
> - pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
> - lsm_order_legacy, lsm_order_cmdline);
> + if (lsm_order_legacy)
> lsm_order_legacy = NULL;
> - }
> lsm_order_parse(lsm_order_cmdline, "cmdline");
> } else
> lsm_order_parse(lsm_order_builtin, "builtin");
> @@ -404,38 +400,25 @@ int __init security_init(void)
> lsm_order_for_each(lsm)
> lsm_prepare(*lsm);
>
> - pr_info("initializing lsm=");
> - lsm_early_for_each_raw(early) {
> - if (lsm_is_enabled(early))
> - pr_cont("%s%s",
> - first++ == 0 ? "" : ",", early->id->name);
> + if (lsm_debug) {
> + lsm_pr("blob(cred) size %d\n", blob_sizes.lbs_cred);
> + lsm_pr("blob(file) size %d\n", blob_sizes.lbs_file);
> + lsm_pr("blob(ib) size %d\n", blob_sizes.lbs_ib);
> + lsm_pr("blob(inode) size %d\n", blob_sizes.lbs_inode);
> + lsm_pr("blob(ipc) size %d\n", blob_sizes.lbs_ipc);
> + lsm_pr("blob(key) size %d\n", blob_sizes.lbs_key);
> + lsm_pr("blob(msg_msg)_size %d\n", blob_sizes.lbs_msg_msg);
> + lsm_pr("blob(sock) size %d\n", blob_sizes.lbs_sock);
> + lsm_pr("blob(superblock) size %d\n", blob_sizes.lbs_superblock);
> + lsm_pr("blob(perf_event) size %d\n", blob_sizes.lbs_perf_event);
> + lsm_pr("blob(task) size %d\n", blob_sizes.lbs_task);
> + lsm_pr("blob(tun_dev) size %d\n", blob_sizes.lbs_tun_dev);
> + lsm_pr("blob(xattr) count %d\n", blob_sizes.lbs_xattr_count);
> + lsm_pr("blob(bdev) size %d\n", blob_sizes.lbs_bdev);
> + lsm_pr("blob(bpf_map) size %d\n", blob_sizes.lbs_bpf_map);
> + lsm_pr("blob(bpf_prog) size %d\n", blob_sizes.lbs_bpf_prog);
> + lsm_pr("blob(bpf_token) size %d\n", blob_sizes.lbs_bpf_token);
> }
> - lsm_order_for_each(lsm) {
> - if (lsm_is_enabled(*lsm))
> - pr_cont("%s%s",
> - first++ == 0 ? "" : ",", (*lsm)->id->name);
> - }
> - pr_cont("\n");
> -
> - init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
> - init_debug("file blob size = %d\n", blob_sizes.lbs_file);
> - init_debug("ib blob size = %d\n", blob_sizes.lbs_ib);
> - init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
> - init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
> -#ifdef CONFIG_KEYS
> - init_debug("key blob size = %d\n", blob_sizes.lbs_key);
> -#endif /* CONFIG_KEYS */
> - init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
> - init_debug("sock blob size = %d\n", blob_sizes.lbs_sock);
> - init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> - init_debug("perf event blob size = %d\n", blob_sizes.lbs_perf_event);
> - init_debug("task blob size = %d\n", blob_sizes.lbs_task);
> - init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
> - init_debug("xattr slots = %d\n", blob_sizes.lbs_xattr_count);
> - init_debug("bdev blob size = %d\n", blob_sizes.lbs_bdev);
> - init_debug("bpf map blob size = %d\n", blob_sizes.lbs_bpf_map);
> - init_debug("bpf prog blob size = %d\n", blob_sizes.lbs_bpf_prog);
> - init_debug("bpf token blob size = %d\n", blob_sizes.lbs_bpf_token);
>
> if (blob_sizes.lbs_file)
> lsm_file_cache = kmem_cache_create("lsm_file_cache",
> @@ -447,9 +430,9 @@ int __init security_init(void)
> SLAB_PANIC, NULL);
>
> if (lsm_cred_alloc((struct cred *)current->cred, GFP_KERNEL))
> - panic("%s: early cred alloc failed.\n", __func__);
> + panic("early LSM cred alloc failed\n");
> if (lsm_task_alloc(current))
> - panic("%s: early task alloc failed.\n", __func__);
> + panic("early LSM task alloc failed\n");
>
> cnt = 0;
> lsm_order_for_each(lsm) {
> diff --git a/security/security.c b/security/security.c
> index add46073af0c..c9642020755e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -73,6 +73,8 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = {
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> +bool lsm_debug __ro_after_init;
> +
> unsigned int lsm_active_cnt __ro_after_init;
> const struct lsm_id *lsm_idlist[MAX_LSM_COUNT];
>
^ permalink raw reply
* Re: [PATCH v3 08/12] bpf: Implement signature verification for BPF programs
From: Paul Moore @ 2025-09-03 16:28 UTC (permalink / raw)
To: ast, KP Singh, andrii, daniel; +Cc: bpf, linux-security-module, bboscaccy, kys
In-Reply-To: <CAHC9VhTFF4gnc2Lu4XOkA+20sQtcvG3WgmJnXN4eiPSifc-+sg@mail.gmail.com>
On Tue, Aug 19, 2025 at 3:19 PM Paul Moore <paul@paul-moore.com> wrote:
> On Wed, Aug 13, 2025 at 6:17 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Wed, Aug 13, 2025 at 5:37 PM KP Singh <kpsingh@kernel.org> wrote:
> > > On Wed, Aug 13, 2025 at 11:02 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >
> > > > It's nice to see a v3 revision, but it would be good to see some
> > > > comments on Blaise's reply to your v2 revision. From what I can see
> > > > it should enable the different use cases and requirements that have
> > > > been posted.
> > >
> > > I will defer to Alexei and others here (mostly due to time crunch). It
> > > would however be useful to explain the use-cases in which signed maps
> > > are useful (beyond being a different approach than the current
> > > delegated verification).
>
> I wanted to bring this up again as it has been another week with no
> comment from the BPF side of the house regarding Blaise's additions.
> As a reminder, Blaise's patch can be found here:
>
> https://lore.kernel.org/linux-security-module/87sei58vy3.fsf@microsoft.com
Another gentle ping. I realize everyone is busy, and August is a
popular month for holidays, but it has been a month since Blaise
posted his patch/snippet; it would be nice to get some feedback on the
basic idea.
https://lore.kernel.org/linux-security-module/87sei58vy3.fsf@microsoft.com
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
From: Stephen Smalley @ 2025-09-03 16:56 UTC (permalink / raw)
To: Thiébaud Weksteen
Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
Nick Kralevich, Jeff Xu, linux-kernel, linux-security-module,
selinux, linux-mm
In-Reply-To: <CAEjxPJ6G2iK9Yp8eCwbwHQfF1J3WBEVU42kAMQHNuuC_H5QHNw@mail.gmail.com>
On Wed, Aug 27, 2025 at 9:23 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Aug 25, 2025 at 11:18 PM Thiébaud Weksteen <tweek@google.com> wrote:
> >
> > Prior to this change, no security hooks were called at the creation of a
> > memfd file. It means that, for SELinux as an example, it will receive
> > the default type of the filesystem that backs the in-memory inode. In
> > most cases, that would be tmpfs, but if MFD_HUGETLB is passed, it will
> > be hugetlbfs. Both can be considered implementation details of memfd.
> >
> > It also means that it is not possible to differentiate between a file
> > coming from memfd_create and a file coming from a standard tmpfs mount
> > point.
> >
> > Additionally, no permission is validated at creation, which differs from
> > the similar memfd_secret syscall.
> >
> > Call security_inode_init_security_anon during creation. This ensures
> > that the file is setup similarly to other anonymous inodes. On SELinux,
> > it means that the file will receive the security context of its task.
> >
> > The ability to limit fexecve on memfd has been of interest to avoid
> > potential pitfalls where /proc/self/exe or similar would be executed
> > [1][2]. Reuse the "execute_no_trans" and "entrypoint" access vectors,
> > similarly to the file class. These access vectors may not make sense for
> > the existing "anon_inode" class. Therefore, define and assign a new
> > class "memfd_file" to support such access vectors.
> >
> > Guard these changes behind a new policy capability named "memfd_class".
> >
> > [1] https://crbug.com/1305267
> > [2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/
> >
> > Signed-off-by: Thiébaud Weksteen <tweek@google.com>
>
> This looks good to me, but do you have a test for it, preferably via
> patch for the selinux-testsuite?
> See https://github.com/SELinuxProject/selinux-testsuite/commit/023b79b8319e5fe222fb5af892c579593e1cbc50
> for an example.
>
> Otherwise, you can add my:
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
And now having run the tests posted in:
https://lore.kernel.org/selinux/20250902055401.618729-1-tweek@google.com/
you can also add my:
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
^ permalink raw reply
* [RFC PATCH] lsm,selinux: introduce LSM_ATTR_UNSHARE and wire it up for SELinux
From: Stephen Smalley @ 2025-09-03 19:24 UTC (permalink / raw)
To: selinux, linux-security-module
Cc: paul, omosnace, john.johansen, serge, casey, Stephen Smalley
In the hopes of nudging the conversation in [1] in a more focused
direction with a goal of getting SELinux namespaces upstreamed, this
RFC provides a starting point for a concrete discussion. This is RFC
only and has only been build-tested thus far.
This defines a new LSM_ATTR_UNSHARE attribute for the
lsm_set_self_attr(2) system call and wires it up for SELinux to invoke
the underlying function for unsharing the SELinux namespace. As with
the selinuxfs interface, this immediately unshares the SELinux
namespace of the current process just like an unshare(2) system call
would do for other namespaces. I have not yet explored the
alternatives of deferring the unshare to the next unshare(2),
clone(2), or execve(2) call and would want to first confirm that doing
so does not introduce any issues in the kernel or make it harder to
integrate with existing container runtimes.
Differences between this syscall interface and the selinuxfs interface
that need discussion before moving forward:
1. The syscall interface does not currently check any Linux capability
or DAC permissions, whereas the selinuxfs interface can only be set by
uid-0 or CAP_DAC_OVERRIDE processes. We need to decide what if any
capability or DAC check should apply to this syscall interface and if
any, add the checks to either the LSM framework code or to the SELinux
hook function.
Pros: Checking a capability or DAC permissions prevents misuse of this
interface by unprivileged processes, particularly on systems with
policies that do not yet define any of the new SELinux permissions
introduced for controlling this operation. This is a potential concern
on Linux distributions that do not tightly coordinate kernel updates
with policy updates (or where users may choose to deploy upstream
kernels on their own), but not on Android.
Cons: Checking a capability or DAC permissions requires any process
that uses this facility to have the corresponding capability or
permissions, which might otherwise be unnecessary and create
additional risks. This is less likely if we use a capability already
required by container runtimes and similar components that might
leverage this facility for unsharing SELinux namespaces.
2. The syscall interface checks a new SELinux unshare_selinuxns
permission in the process2 class between the task SID and itself,
similar to other checks for setting process attributes. This means
that:
allow domain self:process2 *; -or-
allow domain self:process2 ~anything-other-than-unshare_selinuxns; -or-
allow domain self:process2 unshare_selinuxns;
would allow a process to unshare its SELinux namespace.
The selinuxfs interface checks a new unshare permission in the
security class between the task SID and the security initial SID,
likewise similar to other checks for setting selinuxfs attributes.
This means that:
allow domain security_t:security *; -or-
allow domain security_t:security ~anything-other-than-unshare; -or-
allow domain security_t:security unshare;
would allow a process to unshare its SELinux namespace.
Technically, the selinuxfs interface also currently requires open and
write access to the selinuxfs node; hence:
allow domain security_t:file { open write };
is also required for the selinuxfs interface.
We need to decide what we want the SELinux check(s) to be for the
syscall and whether it should be more like the former (process
attributes) or more like the latter (security policy settings). Note
that the permission name itself is unimportant here and only differs
because it seemed less evident in the process2 class that we are
talking about a SELinux namespace otherwise.
Regardless, either form of allow rule can be prohibited in policies
via neverallow rules on systems that enforce their usage
(e.g. Android, not necessarily on Linux distributions).
3. The selinuxfs interface currently offers more functionality than I
have implemented here for the sycall interface, including:
a) the ability to read the selinuxfs node to see if your namespace has
been unshared, which should be easily implementable via
lsm_get_self_attr(2). However, questions remain as to when that
should return 1 versus 0 (currently returns 1 whenever your namespace
is NOT the initial SELinux namespace, useful for the testsuite to
detect it is in a child, but could instead be reset to 0 by a
subsequent policy load to indicate completion of the setup of the
namespace, thus hiding from child processes that they are in a child
namespace once its policy has been loaded).
b) the abilities to get and set the maximum number of SELinux
namespaces (via a /sys/fs/selinux/maxns node) and to get and set the
maximum depth for SELinux namespaces (via a /sys/fs/selinux/maxnsdepth
node). These could be left in selinuxfs or migrated to some other LSM
management APIs since they are global in scope, not per-process
attributes.
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
include/uapi/linux/lsm.h | 1 +
security/selinux/hooks.c | 8 ++++++++
security/selinux/include/classmap.h | 4 +++-
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
index 938593dfd5da..fb1b4a8aa639 100644
--- a/include/uapi/linux/lsm.h
+++ b/include/uapi/linux/lsm.h
@@ -83,6 +83,7 @@ struct lsm_ctx {
#define LSM_ATTR_KEYCREATE 103
#define LSM_ATTR_PREV 104
#define LSM_ATTR_SOCKCREATE 105
+#define LSM_ATTR_UNSHARE 106
/*
* LSM_FLAG_XXX definitions identify special handling instructions
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c544b3e2fd5c..11b0b3c5b74a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6812,6 +6812,10 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
error = avc_has_perm(state, mysid, mysid, SECCLASS_PROCESS,
PROCESS__SETCURRENT, NULL);
break;
+ case LSM_ATTR_UNSHARE:
+ error = avc_has_perm(state, mysid, mysid, SECCLASS_PROCESS,
+ PROCESS2__UNSHARE_SELINUXNS, NULL);
+ break;
default:
error = -EOPNOTSUPP;
break;
@@ -6923,6 +6927,10 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
}
tsec->sid = sid;
+ } else if (attr == LSM_ATTR_UNSHARE) {
+ error = selinux_state_create(new);
+ if (error)
+ goto abort_change;
} else {
error = -EINVAL;
goto abort_change;
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index be52ebb6b94a..07fe316308cd 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -60,7 +60,9 @@ const struct security_class_mapping secclass_map[] = {
"siginh", "setrlimit", "rlimitinh", "dyntransition",
"setcurrent", "execmem", "execstack", "execheap",
"setkeycreate", "setsockcreate", "getrlimit", NULL } },
- { "process2", { "nnp_transition", "nosuid_transition", NULL } },
+ { "process2",
+ { "nnp_transition", "nosuid_transition", "unshare_selinuxns",
+ NULL } },
{ "system",
{ "ipc_info", "syslog_read", "syslog_mod", "syslog_console",
"module_request", "module_load", "firmware_load",
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v3 04/34] lsm: introduce looping macros for the initialization code
From: Paul Moore @ 2025-09-03 20:07 UTC (permalink / raw)
To: John Johansen
Cc: linux-security-module, linux-integrity, selinux, Mimi Zohar,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <e3a54d30-85c4-482f-be91-6d10e0ff6e0e@canonical.com>
On Tue, Sep 2, 2025 at 12:37 PM John Johansen
<john.johansen@canonical.com> wrote:
>
> On 8/14/25 15:50, Paul Moore wrote:
> > There are three common for loop patterns in the LSM initialization code
> > to loop through the ordered LSM list and the registered "early" LSMs.
> > This patch implements these loop patterns as macros to help simplify the
> > code and reduce the change for errors.
> ^
> chance
Good catch, thanks.
> otherwise looks good, and you can add my
>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Paul Moore @ 2025-09-03 20:26 UTC (permalink / raw)
To: John Johansen
Cc: linux-security-module, linux-integrity, selinux, Mimi Zohar,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <06a68323-b297-4be7-92eb-c2091207b9f0@canonical.com>
On Tue, Sep 2, 2025 at 1:20 PM John Johansen
<john.johansen@canonical.com> wrote:
> On 8/14/25 15:50, Paul Moore wrote:
> > The LSM currently has a lot of code to maintain a list of the currently
> > active LSMs in a human readable string, with the only user being the
> > "/sys/kernel/security/lsm" code. Let's drop all of that code and
> > generate the string on first use and then cache it for subsequent use.
> >
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> > include/linux/lsm_hooks.h | 1 -
> > security/inode.c | 59 +++++++++++++++++++++++++++++++++++++--
> > security/lsm_init.c | 49 --------------------------------
> > 3 files changed, 57 insertions(+), 52 deletions(-)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 7343dd60b1d5..65a8227bece7 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -172,7 +172,6 @@ struct lsm_info {
> >
> >
> > /* DO NOT tamper with these variables outside of the LSM framework */
> > -extern char *lsm_names;
> > extern struct lsm_static_calls_table static_calls_table __ro_after_init;
> >
> > /**
> > diff --git a/security/inode.c b/security/inode.c
> > index 43382ef8896e..a5e7a073e672 100644
> > --- a/security/inode.c
> > +++ b/security/inode.c
> > @@ -22,6 +22,8 @@
> > #include <linux/lsm_hooks.h>
> > #include <linux/magic.h>
> >
> > +#include "lsm.h"
> > +
> > static struct vfsmount *mount;
> > static int mount_count;
> >
> > @@ -315,12 +317,65 @@ void securityfs_remove(struct dentry *dentry)
> > EXPORT_SYMBOL_GPL(securityfs_remove);
> >
> > #ifdef CONFIG_SECURITY
> > +#include <linux/spinlock.h>
> > +
> > static struct dentry *lsm_dentry;
> > +
> > +/* NOTE: we never free the string below once it is set. */
> > +static DEFINE_SPINLOCK(lsm_read_lock);
>
> nit, this is only used on the write side, so not the best name
Fair point, I'll rename it to lsm_read_str_lock, it still has "read"
in the name, but it should be a bit more clear that it references the
lsm_read_str variable.
> > +static char *lsm_read_str = NULL;
> > +static ssize_t lsm_read_len = 0;
Similarly, I'm renaming lsm_read_len to lsm_read_str_len.
> > static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
> > loff_t *ppos)
> > {
> > - return simple_read_from_buffer(buf, count, ppos, lsm_names,
> > - strlen(lsm_names));
> > + int i;
> > + char *str;
> > + ssize_t len;
> > +
> > +restart:
> > +
> > + rcu_read_lock();
> > + if (!lsm_read_str) {
>
> should probably be
> if (!rcu_access_pointer(lsm_read_str)) {
The description for rcu_access_pointer() contains the following sentence:
"Within an RCU read-side critical section, there is little reason to
use rcu_access_pointer()."
https://elixir.bootlin.com/linux/v6.17-rc4/source/include/linux/rcupdate.h#L628
Perhaps I'm reading it wrong, but it looks like the RCU folks would
prefer we not use rcu_access_pointer() here?
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 27/34] tomoyo: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-03 20:32 UTC (permalink / raw)
To: Tetsuo Handa
Cc: linux-security-module, John Johansen, Mimi Zohar, Roberto Sassu,
Fan Wu, Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Casey Schaufler, Nicolas Bouchinet, Xiu Jianfeng,
linux-integrity, selinux
In-Reply-To: <20250814225159.275901-63-paul@paul-moore.com>
On Thu, Aug 14, 2025 at 6:54 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> security/tomoyo/common.h | 2 ++
> security/tomoyo/securityfs_if.c | 4 +---
> security/tomoyo/tomoyo.c | 1 +
> 3 files changed, 4 insertions(+), 3 deletions(-)
Tetsuo, does this look okay to you?
> diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
> index 0e8e2e959aef..3b2a97d10a5d 100644
> --- a/security/tomoyo/common.h
> +++ b/security/tomoyo/common.h
> @@ -924,6 +924,8 @@ struct tomoyo_task {
>
> /********** Function prototypes. **********/
>
> +int tomoyo_interface_init(void);
> +
> bool tomoyo_address_matches_group(const bool is_ipv6, const __be32 *address,
> const struct tomoyo_group *group);
> bool tomoyo_compare_number_union(const unsigned long value,
> diff --git a/security/tomoyo/securityfs_if.c b/security/tomoyo/securityfs_if.c
> index 7e69747b2f77..33933645f5b9 100644
> --- a/security/tomoyo/securityfs_if.c
> +++ b/security/tomoyo/securityfs_if.c
> @@ -233,7 +233,7 @@ static void __init tomoyo_create_entry(const char *name, const umode_t mode,
> *
> * Returns 0.
> */
> -static int __init tomoyo_interface_init(void)
> +int __init tomoyo_interface_init(void)
> {
> struct tomoyo_domain_info *domain;
> struct dentry *tomoyo_dir;
> @@ -269,5 +269,3 @@ static int __init tomoyo_interface_init(void)
> tomoyo_load_builtin_policy();
> return 0;
> }
> -
> -fs_initcall(tomoyo_interface_init);
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index ed0f7b052a85..a015cf0c4a00 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -617,4 +617,5 @@ DEFINE_LSM(tomoyo) = {
> .flags = LSM_FLAG_LEGACY_MAJOR,
> .blobs = &tomoyo_blob_sizes,
> .init = tomoyo_init,
> + .initcall_fs = tomoyo_interface_init,
> };
> --
> 2.50.1
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 29/34] apparmor: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-03 20:34 UTC (permalink / raw)
To: John Johansen
Cc: selinux, linux-integrity, linux-security-module, Mimi Zohar,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-65-paul@paul-moore.com>
On Thu, Aug 14, 2025 at 6:54 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> security/apparmor/apparmorfs.c | 4 +---
> security/apparmor/crypto.c | 3 +--
> security/apparmor/include/apparmorfs.h | 2 ++
> security/apparmor/include/crypto.h | 1 +
> security/apparmor/lsm.c | 9 ++++++++-
> 5 files changed, 13 insertions(+), 6 deletions(-)
Thanks for reviewing all the other patches John. Assuming you are
okay with this patch, can I get an ACK?
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 391a586d0557..ee04c1ac9d6e 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -2649,7 +2649,7 @@ static const struct inode_operations policy_link_iops = {
> *
> * Returns: error on failure
> */
> -static int __init aa_create_aafs(void)
> +int __init aa_create_aafs(void)
> {
> struct dentry *dent;
> int error;
> @@ -2728,5 +2728,3 @@ static int __init aa_create_aafs(void)
> AA_ERROR("Error creating AppArmor securityfs\n");
> return error;
> }
> -
> -fs_initcall(aa_create_aafs);
> diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> index 227d47c14907..d8a7bde94d79 100644
> --- a/security/apparmor/crypto.c
> +++ b/security/apparmor/crypto.c
> @@ -53,10 +53,9 @@ int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
> return 0;
> }
>
> -static int __init init_profile_hash(void)
> +int __init init_profile_hash(void)
> {
> if (apparmor_initialized)
> aa_info_message("AppArmor sha256 policy hashing enabled");
> return 0;
> }
> -late_initcall(init_profile_hash);
> diff --git a/security/apparmor/include/apparmorfs.h b/security/apparmor/include/apparmorfs.h
> index 1e94904f68d9..dd580594dfb7 100644
> --- a/security/apparmor/include/apparmorfs.h
> +++ b/security/apparmor/include/apparmorfs.h
> @@ -104,6 +104,8 @@ enum aafs_prof_type {
> #define prof_dir(X) ((X)->dents[AAFS_PROF_DIR])
> #define prof_child_dir(X) ((X)->dents[AAFS_PROF_PROFS])
>
> +int aa_create_aafs(void);
> +
> void __aa_bump_ns_revision(struct aa_ns *ns);
> void __aafs_profile_rmdir(struct aa_profile *profile);
> void __aafs_profile_migrate_dents(struct aa_profile *old,
> diff --git a/security/apparmor/include/crypto.h b/security/apparmor/include/crypto.h
> index 636a04e20d91..f3ffd388cc58 100644
> --- a/security/apparmor/include/crypto.h
> +++ b/security/apparmor/include/crypto.h
> @@ -13,6 +13,7 @@
> #include "policy.h"
>
> #ifdef CONFIG_SECURITY_APPARMOR_HASH
> +int init_profile_hash(void);
> unsigned int aa_hash_size(void);
> char *aa_calc_hash(void *data, size_t len);
> int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 45b3a304d525..647c13e13e63 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -32,6 +32,7 @@
> #include "include/audit.h"
> #include "include/capability.h"
> #include "include/cred.h"
> +#include "include/crypto.h"
> #include "include/file.h"
> #include "include/ipc.h"
> #include "include/net.h"
> @@ -2426,7 +2427,6 @@ static int __init apparmor_nf_ip_init(void)
>
> return 0;
> }
> -__initcall(apparmor_nf_ip_init);
> #endif
>
> static char nulldfa_src[] __aligned(8) = {
> @@ -2557,4 +2557,11 @@ DEFINE_LSM(apparmor) = {
> .enabled = &apparmor_enabled,
> .blobs = &apparmor_blob_sizes,
> .init = apparmor_init,
> + .initcall_fs = aa_create_aafs,
> +#if defined(CONFIG_NETFILTER) && defined(CONFIG_NETWORK_SECMARK)
> + .initcall_device = apparmor_nf_ip_init,
> +#endif
> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> + .initcall_late = init_profile_hash,
> +#endif
> };
> --
> 2.50.1
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] ima,evm: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-03 20:43 UTC (permalink / raw)
To: Roberto Sassu
Cc: zohar, roberto.sassu, linux-security-module, linux-integrity,
selinux, john.johansen, wufan, mic, kees, mortonm, casey,
penguin-kernel, nicolas.bouchinet, xiujianfeng
In-Reply-To: <20250902125457.2689519-1-roberto.sassu@huaweicloud.com>
On Tue, Sep 2, 2025 at 8:59 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Paul Moore <paul@paul-moore.com>
We should obviously drop the 'From:' line above ;)
> This patch converts IMA and EVM to use the LSM frameworks's initcall
> mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> and introduced integrity_fs_fini() to remove the integrity directory, if
> empty. Both integrity_fs_init() and integrity_fs_fini() support the
> scenario of being called by both the IMA and EVM LSMs.
>
> It is worth mentioning that this patch does not touch any of the
> "platform certs" code that lives in the security/integrity/platform_certs
> directory as the IMA/EVM maintainers have assured me that this code is
> unrelated to IMA/EVM, despite the location, and will be moved to a more
> relevant subsystem in the future.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
> security/integrity/evm/evm_main.c | 3 +--
> security/integrity/evm/evm_secfs.c | 11 +++++++++--
> security/integrity/iint.c | 14 ++++++++++++--
> security/integrity/ima/ima_fs.c | 11 +++++++++--
> security/integrity/ima/ima_main.c | 4 ++--
> security/integrity/integrity.h | 2 ++
> 6 files changed, 35 insertions(+), 10 deletions(-)
I'm happy to replace my patch with this one, would you like me to wait
on an ACK from Mimi, or are you okay if I go ahead with this patch?
(As an aside, I'm still not entirely clear if I should wait on ACKs
from both you and Mimi on IMA/EVM changes, or if one of the two of you
is sufficient? I'm happy to follow whatever approach the two of you
would prefer, but I don't know what that is ;) ... )
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] memfd,selinux: call security_inode_init_security_anon
From: Paul Moore @ 2025-09-03 21:26 UTC (permalink / raw)
To: Thiébaud Weksteen, James Morris, Stephen Smalley,
Hugh Dickins, Jeff Vander Stoep, Nick Kralevich, Jeff Xu,
Baolin Wang
Cc: Thiébaud Weksteen, linux-kernel, linux-security-module,
selinux, linux-mm
In-Reply-To: <20250826031824.1227551-1-tweek@google.com>
On Aug 25, 2025 "=?UTF-8?q?Thi=C3=A9baud=20Weksteen?=" <tweek@google.com> wrote:
>
> Prior to this change, no security hooks were called at the creation of a
> memfd file. It means that, for SELinux as an example, it will receive
> the default type of the filesystem that backs the in-memory inode. In
> most cases, that would be tmpfs, but if MFD_HUGETLB is passed, it will
> be hugetlbfs. Both can be considered implementation details of memfd.
>
> It also means that it is not possible to differentiate between a file
> coming from memfd_create and a file coming from a standard tmpfs mount
> point.
>
> Additionally, no permission is validated at creation, which differs from
> the similar memfd_secret syscall.
>
> Call security_inode_init_security_anon during creation. This ensures
> that the file is setup similarly to other anonymous inodes. On SELinux,
> it means that the file will receive the security context of its task.
>
> The ability to limit fexecve on memfd has been of interest to avoid
> potential pitfalls where /proc/self/exe or similar would be executed
> [1][2]. Reuse the "execute_no_trans" and "entrypoint" access vectors,
> similarly to the file class. These access vectors may not make sense for
> the existing "anon_inode" class. Therefore, define and assign a new
> class "memfd_file" to support such access vectors.
>
> Guard these changes behind a new policy capability named "memfd_class".
>
> [1] https://crbug.com/1305267
> [2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/
>
> Signed-off-by: Thiébaud Weksteen <tweek@google.com>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> Changes since RFC:
> - Remove enum argument, simply compare the anon inode name
> - Introduce a policy capability for compatility
> - Add validation of class in selinux_bprm_creds_for_exec
>
> include/linux/memfd.h | 2 ++
> mm/memfd.c | 14 +++++++++--
> security/selinux/hooks.c | 27 ++++++++++++++++++----
> security/selinux/include/classmap.h | 2 ++
> security/selinux/include/policycap.h | 1 +
> security/selinux/include/policycap_names.h | 1 +
> security/selinux/include/security.h | 5 ++++
> 7 files changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/memfd.h b/include/linux/memfd.h
> index 6f606d9573c3..cc74de3dbcfe 100644
> --- a/include/linux/memfd.h
> +++ b/include/linux/memfd.h
> @@ -4,6 +4,8 @@
>
> #include <linux/file.h>
>
> +#define MEMFD_ANON_NAME "[memfd]"
> +
> #ifdef CONFIG_MEMFD_CREATE
> extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg);
> struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx);
> diff --git a/mm/memfd.c b/mm/memfd.c
> index bbe679895ef6..63b439eb402a 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -433,6 +433,8 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> {
> unsigned int *file_seals;
> struct file *file;
> + struct inode *inode;
> + int err = 0;
>
> if (flags & MFD_HUGETLB) {
> file = hugetlb_file_setup(name, 0, VM_NORESERVE,
> @@ -444,12 +446,20 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> }
> if (IS_ERR(file))
> return file;
> +
> + inode = file_inode(file);
> + err = security_inode_init_security_anon(inode,
> + &QSTR(MEMFD_ANON_NAME), NULL);
> + if (err) {
> + fput(file);
> + file = ERR_PTR(err);
> + return file;
> + }
> +
> file->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
> file->f_flags |= O_LARGEFILE;
>
> if (flags & MFD_NOEXEC_SEAL) {
> - struct inode *inode = file_inode(file);
> -
> inode->i_mode &= ~0111;
> file_seals = memfd_file_seals_ptr(file);
> if (file_seals) {
Hugh, Baolin, and shmem/mm folks, are you okay with the changes above? If
so it would be nice to get an ACK from one of you.
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874bf7d..429b2269b35a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -93,6 +93,7 @@
> #include <linux/fanotify.h>
> #include <linux/io_uring/cmd.h>
> #include <uapi/linux/lsm.h>
> +#include <linux/memfd.h>
>
> #include "avc.h"
> #include "objsec.h"
> @@ -2366,9 +2367,12 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> ad.type = LSM_AUDIT_DATA_FILE;
> ad.u.file = bprm->file;
>
> + if (isec->sclass != SECCLASS_FILE && isec->sclass != SECCLASS_MEMFD_FILE)
> + return -EPERM;
In the interest of failing fast, this should probably be moved up in the
function to just after where @isec is set. There are also a number of
checks that happen prior to this placement, but after the isec assignment.
While I don't think any of those checks should be an issue, I'd rather
not to have to worry about those and just fail the non-FILE/MEMFD_FILE
case as soon as we can in selinux_bprm_creds_for_exec().
> if (new_tsec->sid == old_tsec->sid) {
> - rc = avc_has_perm(old_tsec->sid, isec->sid,
> - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad);
> + rc = avc_has_perm(old_tsec->sid, isec->sid, isec->sclass,
> + FILE__EXECUTE_NO_TRANS, &ad);
> if (rc)
> return rc;
> } else {
> @@ -2378,8 +2382,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> if (rc)
> return rc;
>
> - rc = avc_has_perm(new_tsec->sid, isec->sid,
> - SECCLASS_FILE, FILE__ENTRYPOINT, &ad);
> + rc = avc_has_perm(new_tsec->sid, isec->sid, isec->sclass,
> + FILE__ENTRYPOINT, &ad);
> if (rc)
> return rc;
>
> @@ -2974,10 +2978,18 @@ static int selinux_inode_init_security_anon(struct inode *inode,
> struct common_audit_data ad;
> struct inode_security_struct *isec;
> int rc;
> + bool is_memfd = false;
>
> if (unlikely(!selinux_initialized()))
> return 0;
>
> + if (name != NULL && name->name != NULL &&
> + !strcmp(name->name, MEMFD_ANON_NAME)) {
> + if (!selinux_policycap_memfd_class())
> + return 0;
> + is_memfd = true;
> + }
> +
> isec = selinux_inode(inode);
>
> /*
> @@ -2996,6 +3008,13 @@ static int selinux_inode_init_security_anon(struct inode *inode,
>
> isec->sclass = context_isec->sclass;
> isec->sid = context_isec->sid;
> + } else if (is_memfd) {
> + isec->sclass = SECCLASS_MEMFD_FILE;
> + rc = security_transition_sid(
> + sid, sid,
> + isec->sclass, name, &isec->sid);
> + if (rc)
> + return rc;
> } else {
> isec->sclass = SECCLASS_ANON_INODE;
> rc = security_transition_sid(
We're duplicating the security_transition_sid() call which seems less
than ideal, how about something like this?
if (context_inode) {
/* ... existing stuff ... */
} else {
if (is_memfd)
isec->sclass = SECCLASS_MEMFD_FILE;
else
isec->sclass = SECCLASS_ANON_INODE;
rc = security_transition_sid(...);
if (rc)
return rc;
}
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: John Johansen @ 2025-09-03 23:12 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, Mimi Zohar,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhQnFJLXrhcbQ2b7rWDNYjRRKoevqiKYchE_39ShcjgLEw@mail.gmail.com>
On 9/3/25 13:26, Paul Moore wrote:
> On Tue, Sep 2, 2025 at 1:20 PM John Johansen
> <john.johansen@canonical.com> wrote:
>> On 8/14/25 15:50, Paul Moore wrote:
>>> The LSM currently has a lot of code to maintain a list of the currently
>>> active LSMs in a human readable string, with the only user being the
>>> "/sys/kernel/security/lsm" code. Let's drop all of that code and
>>> generate the string on first use and then cache it for subsequent use.
>>>
>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>>> ---
>>> include/linux/lsm_hooks.h | 1 -
>>> security/inode.c | 59 +++++++++++++++++++++++++++++++++++++--
>>> security/lsm_init.c | 49 --------------------------------
>>> 3 files changed, 57 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>>> index 7343dd60b1d5..65a8227bece7 100644
>>> --- a/include/linux/lsm_hooks.h
>>> +++ b/include/linux/lsm_hooks.h
>>> @@ -172,7 +172,6 @@ struct lsm_info {
>>>
>>>
>>> /* DO NOT tamper with these variables outside of the LSM framework */
>>> -extern char *lsm_names;
>>> extern struct lsm_static_calls_table static_calls_table __ro_after_init;
>>>
>>> /**
>>> diff --git a/security/inode.c b/security/inode.c
>>> index 43382ef8896e..a5e7a073e672 100644
>>> --- a/security/inode.c
>>> +++ b/security/inode.c
>>> @@ -22,6 +22,8 @@
>>> #include <linux/lsm_hooks.h>
>>> #include <linux/magic.h>
>>>
>>> +#include "lsm.h"
>>> +
>>> static struct vfsmount *mount;
>>> static int mount_count;
>>>
>>> @@ -315,12 +317,65 @@ void securityfs_remove(struct dentry *dentry)
>>> EXPORT_SYMBOL_GPL(securityfs_remove);
>>>
>>> #ifdef CONFIG_SECURITY
>>> +#include <linux/spinlock.h>
>>> +
>>> static struct dentry *lsm_dentry;
>>> +
>>> +/* NOTE: we never free the string below once it is set. */
>>> +static DEFINE_SPINLOCK(lsm_read_lock);
>>
>> nit, this is only used on the write side, so not the best name
>
> Fair point, I'll rename it to lsm_read_str_lock, it still has "read"
> in the name, but it should be a bit more clear that it references the
> lsm_read_str variable.
>
>>> +static char *lsm_read_str = NULL;
>>> +static ssize_t lsm_read_len = 0;
>
> Similarly, I'm renaming lsm_read_len to lsm_read_str_len.
>
>>> static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
>>> loff_t *ppos)
>>> {
>>> - return simple_read_from_buffer(buf, count, ppos, lsm_names,
>>> - strlen(lsm_names));
>>> + int i;
>>> + char *str;
>>> + ssize_t len;
>>> +
>>> +restart:
>>> +
>>> + rcu_read_lock();
>>> + if (!lsm_read_str) {
>>
>> should probably be
>> if (!rcu_access_pointer(lsm_read_str)) {
>
> The description for rcu_access_pointer() contains the following sentence:
>
> "Within an RCU read-side critical section, there is little reason to
> use rcu_access_pointer()."
> https://elixir.bootlin.com/linux/v6.17-rc4/source/include/linux/rcupdate.h#L628
>
> Perhaps I'm reading it wrong, but it looks like the RCU folks would
> prefer we not use rcu_access_pointer() here?
>
no, I just forgot that detail
^ permalink raw reply
* Re: [PATCH v3 29/34] apparmor: move initcalls to the LSM framework
From: John Johansen @ 2025-09-03 23:15 UTC (permalink / raw)
To: Paul Moore
Cc: selinux, linux-integrity, linux-security-module, Mimi Zohar,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhRwHLaWP-qUCEVC7-6hEWf0K1H9DwbxWMW9c3a5uUF94w@mail.gmail.com>
On 9/3/25 13:34, Paul Moore wrote:
> On Thu, Aug 14, 2025 at 6:54 PM Paul Moore <paul@paul-moore.com> wrote:
>>
>> Reviewed-by: Kees Cook <kees@kernel.org>
>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>> ---
>> security/apparmor/apparmorfs.c | 4 +---
>> security/apparmor/crypto.c | 3 +--
>> security/apparmor/include/apparmorfs.h | 2 ++
>> security/apparmor/include/crypto.h | 1 +
>> security/apparmor/lsm.c | 9 ++++++++-
>> 5 files changed, 13 insertions(+), 6 deletions(-)
>
> Thanks for reviewing all the other patches John. Assuming you are
> okay with this patch, can I get an ACK?
>
I'm working on it. I managed to get down to I think 3 patches remaining to review/ack, and I wanted to get some testing on this one before acking. Hopefully will get that done today
>> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
>> index 391a586d0557..ee04c1ac9d6e 100644
>> --- a/security/apparmor/apparmorfs.c
>> +++ b/security/apparmor/apparmorfs.c
>> @@ -2649,7 +2649,7 @@ static const struct inode_operations policy_link_iops = {
>> *
>> * Returns: error on failure
>> */
>> -static int __init aa_create_aafs(void)
>> +int __init aa_create_aafs(void)
>> {
>> struct dentry *dent;
>> int error;
>> @@ -2728,5 +2728,3 @@ static int __init aa_create_aafs(void)
>> AA_ERROR("Error creating AppArmor securityfs\n");
>> return error;
>> }
>> -
>> -fs_initcall(aa_create_aafs);
>> diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c
>> index 227d47c14907..d8a7bde94d79 100644
>> --- a/security/apparmor/crypto.c
>> +++ b/security/apparmor/crypto.c
>> @@ -53,10 +53,9 @@ int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
>> return 0;
>> }
>>
>> -static int __init init_profile_hash(void)
>> +int __init init_profile_hash(void)
>> {
>> if (apparmor_initialized)
>> aa_info_message("AppArmor sha256 policy hashing enabled");
>> return 0;
>> }
>> -late_initcall(init_profile_hash);
>> diff --git a/security/apparmor/include/apparmorfs.h b/security/apparmor/include/apparmorfs.h
>> index 1e94904f68d9..dd580594dfb7 100644
>> --- a/security/apparmor/include/apparmorfs.h
>> +++ b/security/apparmor/include/apparmorfs.h
>> @@ -104,6 +104,8 @@ enum aafs_prof_type {
>> #define prof_dir(X) ((X)->dents[AAFS_PROF_DIR])
>> #define prof_child_dir(X) ((X)->dents[AAFS_PROF_PROFS])
>>
>> +int aa_create_aafs(void);
>> +
>> void __aa_bump_ns_revision(struct aa_ns *ns);
>> void __aafs_profile_rmdir(struct aa_profile *profile);
>> void __aafs_profile_migrate_dents(struct aa_profile *old,
>> diff --git a/security/apparmor/include/crypto.h b/security/apparmor/include/crypto.h
>> index 636a04e20d91..f3ffd388cc58 100644
>> --- a/security/apparmor/include/crypto.h
>> +++ b/security/apparmor/include/crypto.h
>> @@ -13,6 +13,7 @@
>> #include "policy.h"
>>
>> #ifdef CONFIG_SECURITY_APPARMOR_HASH
>> +int init_profile_hash(void);
>> unsigned int aa_hash_size(void);
>> char *aa_calc_hash(void *data, size_t len);
>> int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index 45b3a304d525..647c13e13e63 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -32,6 +32,7 @@
>> #include "include/audit.h"
>> #include "include/capability.h"
>> #include "include/cred.h"
>> +#include "include/crypto.h"
>> #include "include/file.h"
>> #include "include/ipc.h"
>> #include "include/net.h"
>> @@ -2426,7 +2427,6 @@ static int __init apparmor_nf_ip_init(void)
>>
>> return 0;
>> }
>> -__initcall(apparmor_nf_ip_init);
>> #endif
>>
>> static char nulldfa_src[] __aligned(8) = {
>> @@ -2557,4 +2557,11 @@ DEFINE_LSM(apparmor) = {
>> .enabled = &apparmor_enabled,
>> .blobs = &apparmor_blob_sizes,
>> .init = apparmor_init,
>> + .initcall_fs = aa_create_aafs,
>> +#if defined(CONFIG_NETFILTER) && defined(CONFIG_NETWORK_SECMARK)
>> + .initcall_device = apparmor_nf_ip_init,
>> +#endif
>> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
>> + .initcall_late = init_profile_hash,
>> +#endif
>> };
>> --
>> 2.50.1
>
^ permalink raw reply
* [PATCH v2 0/7] fs/9p: Reuse inode based on path (in addition to qid)
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
Hi!
This is the second version of this series. The individual commits
contains changelogs (most of them are in the first patch), but overall,
most significantly cached mode (loose or metadata) is now unchanged, there
is no longer a "don't reuse inodes at all" mode, bug fixes, using the
right functions, basic rename handling, and new documentation.
Thanks in advance for the review effort :)
v1: https://lore.kernel.org/all/cover.1743971855.git.m@maowtm.org/
Background
----------
(This section has basically the same content as the v1 cover letter)
Previously [1], I noticed that when using 9pfs filesystems, the Landlock
LSM is blocking access even for files / directories allowed by rules, and
that this has something to do with 9pfs creating new inodes despite
Landlock holding a reference to the existing one. Because Landlock uses
inodes' in-memory state (i_security) to identify allowed fs
objects/hierarchies, this causes Landlock to partially break on 9pfs, at
least in uncached mode, which is the default:
# mount -t 9p -o trans=virtio test /mnt
# env LL_FS_RO=/etc:/usr:/bin:/lib:/mnt/readme LL_FS_RW= /sandboxer bash
Executing the sandboxed command...
# cat /mnt/readme
cat: /mnt/readme: Permission denied
This, however, works if somebody is holding onto the dentry (and it also
works with cache=loose), as in both cases the inode is reused:
# tail -f /mnt/readme &
[1] 196
# env LL_FS_RO=/etc:/usr:/bin:/lib:/mnt/readme LL_FS_RW= /sandboxer bash
Executing the sandboxed command...
# cat /mnt/readme
aa
It also works on directories if one have a shell that cd into the
directory. Note that this means only certain usage of Landlock are
affected - for example, sandboxing applications that takes a list of files
to allow, landlocks itself, then evecve. On the other hand, this does not
affect applications that opens a file, then Landlocks itself while keeping
the file it needs open.
While the above is a very simple example, this is problematic in
real-world use cases if Landlock is used to sandox applications on system
that has files mounted via 9pfs, or use 9pfs as the root filesystem. In
addition, this also affects fanotify / inotify when using inode mark (for
local access):
root@d8c28a676d72:/# ./fanotify-basic-open /readme & # on virtiofs
[1] 173
root@d8c28a676d72:/# cat readme
aa
FAN_OPEN: File /readme
root@d8c28a676d72:/# mount -t 9p -o trans=virtio test /mnt
root@d8c28a676d72:/# ./fanotify-basic-open /mnt/readme & # on 9pfs
[2] 176
root@d8c28a676d72:/# cat /mnt/readme
aa
root@d8c28a676d72:/#
Same can be demonstrated with inotifywait. The source code for
fanotify-basic-open, adopted from the fanotify man page, is available at
https://fileshare.maowtm.org/9pfs-landlock-fix/20250903/fanotify-basic-open.c [2].
Note that this is not a security bug for Landlock since it can only cause
legitimate access to be denied, but might be a problem for fanotify perm
(although I do recognize that using perm on individual inodes is already
perhaps a bit unreliable?)
It seems that there was an attempt at making 9pfs reuse inodes on uncached
mode as well, based on qid.path, however it was reverted [3] due to issues
with servers that present duplicate qids, for example on a QEMU host that
has multiple filesystems mounted under a single 9pfs export without
multidevs=remap, or in the case of other servers that doesn't necessarily
support remapping qids ([4] and more). I've done some testing on
v6.12-rc4 which has the simplified 9pfs inode code before it was reverted,
and found that Landlock works (however, we of course then have the issue
demonstrated in [3]).
What this series do
-------------------
(Changes since v1: added more reasoning for the ino_path struct)
With the above in mind, I have a proposal for 9pfs to:
1. Reuse inodes even in uncached mode
2. However, reuse them based on qid.path AND the actual pathname, by doing
the appropriate testing in v9fs_test_inode(_dotl)?
The main problem here is how to store the pathname in a sensible way and
tie it to the inode. For now I opted with an array of names acquired with
take_dentry_name_snapshot, which reuses the same memory as the dcache to
store the actual strings, but doesn't tie the lifetime of the dentry with
the inode (I thought about holding a reference to the dentry in the
v9fs_inode, but it seemed like a wrong approach and would cause dentries
to not be evicted/released).
Additional discussions
----------------------
(New section)
From some QEMU documentation I read [5] it seems like there is a plan to
resolve these kind of problems in a new version of the protocol, by
expanding the qid to include the filesystem identifier of a file on the
host, so maybe this can be disabled after a successful protocol version
check with the host? For now, inodeident=path will be the default for
uncached filesystems, which can be set to 'qid' to instead to reuse based
only on server-provided inode numbers.
This patchset currently uses strncmp to compare paths but this might be
able to be optimized into a hash comparison first (not done yet).
Alternatively the path can be stored more compactly in the form of a
single string with `/` in it (like normal paths). However, we should
normally only need to do this comparison for one pair of filenames, as the
test is only done if qid.path matches in the first place.
This patchset currently does not support enabling path-based inodes in
cached mode. Additional care needs to be taken to ensure we can refresh
an inode that potentially has data cached, but since Dominique is happy
with cached mode behaving as-is (reusing inodes via qid only), this is not
done.
The current implementation will handle client-side renames of a single
file (or empty directory) correctly, but server side renames, or renaming
a non-empty directory (client or server side), will cause the files being
renamed (or files under the renamed directory) to use new inodes (unless
they are renamed back). The decision to not update the children of a
client-renamed directory is purely to reduce the complexity of this patch,
but is in principle possible.
Testing and explanations
------------------------
(New section)
# mount -t 9p -o ... test /mnt
with the following options:
- trans=virtio
- trans=virtio,inodeident=qid
- trans=virtio,cache=loose
# env LL_FS_RO=/etc:/usr:/bin:/lib:/mnt/readme LL_FS_RW= /sandboxer bash
Executing the sandboxed command...
# cat /mnt/readme
hi
^^ landlock works
# mount -t 9p -o trans=virtio test /mnt
# mkdir /mnt/dir
# mv /mnt/readme /mnt/dir/readme
# env LL_FS_RO=/etc:/usr:/bin:/lib:/mnt/dir/readme LL_FS_RW= /sandboxer bash
Executing the sandboxed command...
# cat /mnt/dir/readme
hi
^^ landlock works
# # another terminal in guest: mv /mnt/dir/readme /mnt/dir/readme.2
# cat /mnt/dir/readme.2
hi
^^ ino_path is carried with renames
# # host: mv 9pfs/dir/readme.2 9pfs/dir/readme
# cat /mnt/dir/readme.2
cat: /mnt/dir/readme.2: No such file or directory
# cat /mnt/dir/readme
cat: /mnt/dir/readme: Permission denied
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ we can't track renames on the server side
# # host: mv 9pfs/dir/readme 9pfs/dir/readme.2
# cat /mnt/dir/readme.2
hi
^^ once the file is back at its original place it works as expected.
# # another terminal in guest: mv /mnt/dir/readme.2 /mnt/dir/readme
# cat /mnt/dir/readme
hi
^^ we can track renames of the file directly...
# # another terminal in guest: mv /mnt/dir /mnt/dir.2
# cat /mnt/dir.2/readme
cat: /mnt/dir.2/readme: Permission denied
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ but not renames of the parent directory, even if done client-side
# # another terminal in guest: mv /mnt/dir.2 /mnt/dir
# cat /mnt/dir/readme
hi
^^ works once it's back
# # another terminal in guest: mv /mnt/dir /mnt/dir.2 && mkdir /mnt/dir && echo hi2 > /mnt/dir/readme
# cat /mnt/dir/readme
cat: /mnt/dir/readme: Permission denied
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a different file uses a different inode even if same path
# # another terminal in guest: mv /mnt/dir.2/readme /mnt/dir/readme
# cat /mnt/dir/readme
hi
# # host: rm 9pfs/dir/readme && echo hi3 > 9pfs/dir/readme
# cat /mnt/dir/readme
cat: /mnt/dir/readme: Permission denied
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a different file (identified by server-side qid changes) uses different inode
fanotify also works, as tested with the program attached at the end.
In addition, I ran xfstests on a uncached 9pfs mount, and while there are
some test failures, it is the same set of failures as on the current
mainline. Test logs at https://fileshare.maowtm.org/9pfs-landlock-fix/20250903/index.html
Tested also with Mickaël's new v9fs landlock tests [6] (unmerged yet):
# RUN layout3_fs.v9fs.tag_inode_dir_parent ...
# OK layout3_fs.v9fs.tag_inode_dir_parent
ok 129 layout3_fs.v9fs.tag_inode_dir_parent
# RUN layout3_fs.v9fs.tag_inode_dir_mnt ...
# OK layout3_fs.v9fs.tag_inode_dir_mnt
ok 130 layout3_fs.v9fs.tag_inode_dir_mnt
# RUN layout3_fs.v9fs.tag_inode_dir_child ...
# OK layout3_fs.v9fs.tag_inode_dir_child
ok 131 layout3_fs.v9fs.tag_inode_dir_child
# RUN layout3_fs.v9fs.tag_inode_file ...
# OK layout3_fs.v9fs.tag_inode_file
ok 132 layout3_fs.v9fs.tag_inode_file
# RUN layout3_fs.v9fs.release_inodes ...
# OK layout3_fs.v9fs.release_inodes
ok 133 layout3_fs.v9fs.release_inodes
This patch series was based on, and mostly tested on v6.17-rc1 + [7]
Kind regards,
Tingmao
[1]: https://github.com/landlock-lsm/linux/issues/45
[2]: https://fileshare.maowtm.org/9pfs-landlock-fix/20250903/fanotify-basic-open.c
[3]: https://lore.kernel.org/all/20241024-revert_iget-v1-4-4cac63d25f72@codewreck.org/
[4]: https://lore.kernel.org/all/20240923100508.GA32066@willie-the-truck/
[5]: https://wiki.qemu.org/Documentation/9p#Protocol_Plans
[6]: https://lore.kernel.org/all/20250704171345.1393451-1-mic@digikod.net/
[7]: https://lore.kernel.org/all/cover.1743956147.git.m@maowtm.org/
Tingmao Wang (7):
fs/9p: Add ability to identify inode by path for .L in uncached mode
fs/9p: add option for path-based inodes
fs/9p: Add ability to identify inode by path for non-.L in uncached
mode
fs/9p: .L: Refresh stale inodes on reuse
fs/9p: non-.L: Refresh stale inodes on reuse
fs/9p: update the target's ino_path on rename
docs: fs/9p: Document the "inodeident" option
Documentation/filesystems/9p.rst | 42 +++++++
fs/9p/Makefile | 3 +-
fs/9p/ino_path.c | 111 ++++++++++++++++++
fs/9p/v9fs.c | 59 +++++++++-
fs/9p/v9fs.h | 87 ++++++++++----
fs/9p/vfs_inode.c | 195 ++++++++++++++++++++++++++-----
fs/9p/vfs_inode_dotl.c | 171 +++++++++++++++++++++++----
fs/9p/vfs_super.c | 13 ++-
8 files changed, 611 insertions(+), 70 deletions(-)
create mode 100644 fs/9p/ino_path.c
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
prerequisite-patch-id: 3dae487a4b3d676de7c20b269553e3e2176b1e36
prerequisite-patch-id: 93ab54c52a41fa44b8d0baf55df949d0ad27e99a
prerequisite-patch-id: 5f558bf969e6eaa3d011c98de0806ca8ad369efe
--
2.51.0
^ permalink raw reply
* [PATCH v2 1/7] fs/9p: Add ability to identify inode by path for .L in uncached mode
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
In-Reply-To: <cover.1756935780.git.m@maowtm.org>
The intention of this patch is to allow features like Landlock and
fanotify (inode mark mode) to work on uncached 9pfs. These features rely
on holding a specific inode and handling further access to the same file
(as identified by that inode), however, currently in uncached mode, we
always get a new inode on each access, due to concerns regarding server
side inode number collision.
On cached mode (either CACHE_LOOSE or CACHE_META), inode is already reused
only by looking at the qid (server-side inode number). Since introducing
this additional check would regress hard links (as they will have
different path, and thus the two ends of a hard link won't be the same
inode anymore under this approach), this won't be done for cached mode.
Currently this patch doesn't actually have any effect - the next commit
will introduce a config option to control inodeident=path enablement and
default it to on for uncached mode.
Signed-off-by: Tingmao Wang <m@maowtm.org>
Cc: "Mickaël Salaün" <mic@digikod.net>
Cc: "Günther Noack" <gnoack@google.com>
Closes: https://github.com/landlock-lsm/linux/issues/45
---
Changes since v1:
- Assume inodeident=path will not be set in cached mode.
- Fix various issues (rcu usage etc) in ino_path.c with feedback from Al
Viro and Mickaël Salaün
- Use d_same_name instead of strncmp
- Instead of changing v9fs_test_new_inode_dotl to add the path check (thus
hijacking the meaning of "new" to actually mean "uncached"), we add the
path check (conditional on the right flags in v9ses) to the cached test
function (v9fs_test_inode_dotl) and use that function for both cached
and uncached mode, by adding additional conditionals within in for the
version/generation check. The v9fs_test_new_inode_dotl function is thus
used only for mknod, mkdir and atomic_open in the "need to create" case.
- Instead of never reusing inode if path-based ident is not enabled, we
always reuse in uncached mode, but if path-based ident is not enabled,
we don't check the path. This makes the code easier to reason about,
and gets rid of the complexity of having to support two quite different
mode of operation (re-using and not re-using inodes).
- Fix crash due to uninitialized v9inode->path when inode is allocated
then immediately deallocated in iget5_locked as a result of two iget
racing with each other to insert the inode. Spotted via xfstests.
- Don't allocate v9fs_ino_path within v9fs_set_inode_dotl, as iget5_locked
specifies that it can't sleep. Doing so means that we need to handle a
special case of inode being created and hashed into the inode list, and
thus may be tested by another iget5_locked call, but its v9inode->path
has not been populated yet. This is resolved via waiting for
iget5_locked to return before checking the path. This edge case was
spotted via xfstests.
fs/9p/Makefile | 3 +-
fs/9p/ino_path.c | 110 ++++++++++++++++++++++++++++++
fs/9p/v9fs.h | 74 +++++++++++++++-----
fs/9p/vfs_inode.c | 16 +++--
fs/9p/vfs_inode_dotl.c | 149 +++++++++++++++++++++++++++++++++++------
fs/9p/vfs_super.c | 13 +++-
6 files changed, 321 insertions(+), 44 deletions(-)
create mode 100644 fs/9p/ino_path.c
diff --git a/fs/9p/Makefile b/fs/9p/Makefile
index e7800a5c7395..38c3ceb26274 100644
--- a/fs/9p/Makefile
+++ b/fs/9p/Makefile
@@ -11,7 +11,8 @@ obj-$(CONFIG_9P_FS) := 9p.o
vfs_dentry.o \
v9fs.o \
fid.o \
- xattr.o
+ xattr.o \
+ ino_path.o
9p-$(CONFIG_9P_FSCACHE) += cache.o
9p-$(CONFIG_9P_FS_POSIX_ACL) += acl.o
diff --git a/fs/9p/ino_path.c b/fs/9p/ino_path.c
new file mode 100644
index 000000000000..a03145e08a9d
--- /dev/null
+++ b/fs/9p/ino_path.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Specific operations on the v9fs_ino_path structure.
+ *
+ * Copyright (C) 2025 by Tingmao Wang <m@maowtm.org>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/fs.h>
+#include <linux/string.h>
+#include <linux/dcache.h>
+
+#include <linux/posix_acl.h>
+#include <net/9p/9p.h>
+#include <net/9p/client.h>
+#include "v9fs.h"
+
+/*
+ * Must hold rename_sem due to traversing parents. Caller must hold
+ * reference to dentry.
+ */
+struct v9fs_ino_path *make_ino_path(struct dentry *dentry)
+{
+ struct v9fs_ino_path *path;
+ size_t path_components = 0;
+ struct dentry *curr = dentry;
+ ssize_t i;
+
+ lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
+ might_sleep(); /* Allocation below might block */
+
+ rcu_read_lock();
+
+ /* Don't include the root dentry */
+ while (curr->d_parent != curr) {
+ if (WARN_ON_ONCE(path_components >= SSIZE_MAX)) {
+ rcu_read_unlock();
+ return NULL;
+ }
+ path_components++;
+ curr = curr->d_parent;
+ }
+
+ /*
+ * Allocation can block so don't do it in RCU (and because the
+ * allocation might be large, since name_snapshot leaves space for
+ * inline str, not worth trying GFP_ATOMIC)
+ */
+ rcu_read_unlock();
+
+ path = kmalloc(struct_size(path, names, path_components), GFP_KERNEL);
+ if (!path)
+ return NULL;
+
+ path->nr_components = path_components;
+ curr = dentry;
+
+ rcu_read_lock();
+ for (i = path_components - 1; i >= 0; i--) {
+ take_dentry_name_snapshot(&path->names[i], curr);
+ curr = curr->d_parent;
+ }
+ WARN_ON(curr != curr->d_parent);
+ rcu_read_unlock();
+ return path;
+}
+
+void free_ino_path(struct v9fs_ino_path *path)
+{
+ if (path) {
+ for (size_t i = 0; i < path->nr_components; i++)
+ release_dentry_name_snapshot(&path->names[i]);
+ kfree(path);
+ }
+}
+
+/*
+ * Must hold rename_sem due to traversing parents. Returns whether
+ * ino_path matches with the path of a v9fs dentry. This function does
+ * not sleep.
+ */
+bool ino_path_compare(struct v9fs_ino_path *ino_path, struct dentry *dentry)
+{
+ struct dentry *curr = dentry;
+ struct name_snapshot *compare;
+ ssize_t i;
+ bool ret;
+
+ lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
+
+ rcu_read_lock();
+ for (i = ino_path->nr_components - 1; i >= 0; i--) {
+ if (curr->d_parent == curr) {
+ /* We're supposed to have more components to walk */
+ rcu_read_unlock();
+ return false;
+ }
+ compare = &ino_path->names[i];
+ if (!d_same_name(curr, curr->d_parent, &compare->name)) {
+ rcu_read_unlock();
+ return false;
+ }
+ curr = curr->d_parent;
+ }
+ /* Comparison fails if dentry is deeper than ino_path */
+ ret = (curr == curr->d_parent);
+ rcu_read_unlock();
+ return ret;
+}
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index f28bc763847a..134b55a605be 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -10,6 +10,7 @@
#include <linux/backing-dev.h>
#include <linux/netfs.h>
+#include <linux/dcache.h>
/**
* enum p9_session_flags - option flags for each 9P session
@@ -31,16 +32,17 @@
#define V9FS_ACL_MASK V9FS_POSIX_ACL
enum p9_session_flags {
- V9FS_PROTO_2000U = 0x01,
- V9FS_PROTO_2000L = 0x02,
- V9FS_ACCESS_SINGLE = 0x04,
- V9FS_ACCESS_USER = 0x08,
- V9FS_ACCESS_CLIENT = 0x10,
- V9FS_POSIX_ACL = 0x20,
- V9FS_NO_XATTR = 0x40,
- V9FS_IGNORE_QV = 0x80, /* ignore qid.version for cache hints */
- V9FS_DIRECT_IO = 0x100,
- V9FS_SYNC = 0x200
+ V9FS_PROTO_2000U = 0x01,
+ V9FS_PROTO_2000L = 0x02,
+ V9FS_ACCESS_SINGLE = 0x04,
+ V9FS_ACCESS_USER = 0x08,
+ V9FS_ACCESS_CLIENT = 0x10,
+ V9FS_POSIX_ACL = 0x20,
+ V9FS_NO_XATTR = 0x40,
+ V9FS_IGNORE_QV = 0x80, /* ignore qid.version for cache hints */
+ V9FS_DIRECT_IO = 0x100,
+ V9FS_SYNC = 0x200,
+ V9FS_INODE_IDENT_PATH = 0x400,
};
/**
@@ -133,11 +135,27 @@ struct v9fs_session_info {
/* cache_validity flags */
#define V9FS_INO_INVALID_ATTR 0x01
+struct v9fs_ino_path {
+ size_t nr_components;
+ struct name_snapshot names[] __counted_by(nr_components);
+};
+
+extern struct v9fs_ino_path *make_ino_path(struct dentry *dentry);
+extern void free_ino_path(struct v9fs_ino_path *path);
+extern bool ino_path_compare(struct v9fs_ino_path *ino_path,
+ struct dentry *dentry);
+
struct v9fs_inode {
struct netfs_inode netfs; /* Netfslib context and vfs inode */
struct p9_qid qid;
unsigned int cache_validity;
struct mutex v_mutex;
+
+ /*
+ * Stores the path of the file this inode is for, only for filesystems
+ * with inode_ident=path. Lifetime is the same as this inode.
+ */
+ struct v9fs_ino_path *path;
};
static inline struct v9fs_inode *V9FS_I(const struct inode *inode)
@@ -188,7 +206,8 @@ extern const struct inode_operations v9fs_symlink_inode_operations_dotl;
extern const struct netfs_request_ops v9fs_req_ops;
extern struct inode *v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses,
struct p9_fid *fid,
- struct super_block *sb, int new);
+ struct super_block *sb,
+ struct dentry *dentry, int new);
/* other default globals */
#define V9FS_PORT 564
@@ -217,38 +236,57 @@ static inline int v9fs_proto_dotl(struct v9fs_session_info *v9ses)
return v9ses->flags & V9FS_PROTO_2000L;
}
+static inline int v9fs_inode_ident_path(struct v9fs_session_info *v9ses)
+{
+ return v9ses->flags & V9FS_INODE_IDENT_PATH;
+}
+
/**
- * v9fs_get_inode_from_fid - Helper routine to populate an inode by
- * issuing a attribute request
+ * v9fs_get_inode_from_fid - Find or populate an inode by issuing a
+ * attribute request, reusing existing inode by qid, and additionally
+ * path, if inodeident=path is enabled.
* @v9ses: session information
* @fid: fid to issue attribute request for
* @sb: superblock on which to create inode
+ * @dentry: dentry corresponding to @fid
*
*/
static inline struct inode *
v9fs_get_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
- struct super_block *sb)
+ struct super_block *sb, struct dentry *dentry)
{
+ if (!v9fs_inode_ident_path(v9ses)) {
+ /* Only pass in a dentry if we use qid+path to identify inodes */
+ dentry = NULL;
+ } else {
+ WARN_ON_ONCE(!dentry);
+ }
if (v9fs_proto_dotl(v9ses))
- return v9fs_inode_from_fid_dotl(v9ses, fid, sb, 0);
+ return v9fs_inode_from_fid_dotl(v9ses, fid, sb, dentry, 0);
else
return v9fs_inode_from_fid(v9ses, fid, sb, 0);
}
/**
* v9fs_get_new_inode_from_fid - Helper routine to populate an inode by
- * issuing a attribute request
+ * issuing a attribute request. Always get a new inode.
* @v9ses: session information
* @fid: fid to issue attribute request for
* @sb: superblock on which to create inode
+ * @dentry: dentry corresponding to @fid. A reference will be taken and
+ * placed in the inode, if in path identification mode.
*
*/
static inline struct inode *
v9fs_get_new_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
- struct super_block *sb)
+ struct super_block *sb, struct dentry *dentry)
{
+ if (!v9fs_inode_ident_path(v9ses)) {
+ /* Only pass in a dentry if we use qid+path to identify inodes */
+ dentry = NULL;
+ }
if (v9fs_proto_dotl(v9ses))
- return v9fs_inode_from_fid_dotl(v9ses, fid, sb, 1);
+ return v9fs_inode_from_fid_dotl(v9ses, fid, sb, dentry, 1);
else
return v9fs_inode_from_fid(v9ses, fid, sb, 1);
}
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index caff65d8b2bb..5e56c13da733 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -232,6 +232,7 @@ struct inode *v9fs_alloc_inode(struct super_block *sb)
if (!v9inode)
return NULL;
v9inode->cache_validity = 0;
+ v9inode->path = NULL;
mutex_init(&v9inode->v_mutex);
return &v9inode->netfs.inode;
}
@@ -243,6 +244,7 @@ struct inode *v9fs_alloc_inode(struct super_block *sb)
void v9fs_free_inode(struct inode *inode)
{
+ free_ino_path(V9FS_I(inode)->path);
kmem_cache_free(v9fs_inode_cache, V9FS_I(inode));
}
@@ -607,15 +609,17 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir,
goto error;
}
/*
- * instantiate inode and assign the unopened fid to the dentry
+ * Instantiate inode. On .L fs, pass in dentry for inodeident=path.
*/
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb,
+ v9fs_proto_dotl(v9ses) ? dentry : NULL);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS,
"inode creation failed %d\n", err);
goto error;
}
+ /* Assign the unopened fid to the dentry */
v9fs_fid_add(dentry, &fid);
d_instantiate(dentry, inode);
}
@@ -732,14 +736,16 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
name = dentry->d_name.name;
fid = p9_client_walk(dfid, 1, &name, 1);
p9_fid_put(dfid);
+
if (fid == ERR_PTR(-ENOENT))
inode = NULL;
else if (IS_ERR(fid))
inode = ERR_CAST(fid);
- else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
- inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
+ else if (v9ses->cache & (CACHE_META | CACHE_LOOSE))
+ /* Cached fs will not use inode path identification */
+ inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, NULL);
else
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, dentry);
/*
* If we had a rename on the server and a parallel lookup
* for the new name, then make sure we instantiate with
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 0fafc603b64a..86adaf5bcc0e 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -52,44 +52,98 @@ static kgid_t v9fs_get_fsgid_for_create(struct inode *dir_inode)
return current_fsgid();
}
+struct iget_data {
+ struct p9_stat_dotl *st;
+
+ /* May be NULL */
+ struct dentry *dentry;
+
+ bool need_double_check;
+};
+
static int v9fs_test_inode_dotl(struct inode *inode, void *data)
{
struct v9fs_inode *v9inode = V9FS_I(inode);
- struct p9_stat_dotl *st = (struct p9_stat_dotl *)data;
+ struct p9_stat_dotl *st = ((struct iget_data *)data)->st;
+ struct dentry *dentry = ((struct iget_data *)data)->dentry;
+ struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
+ bool cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
- /* don't match inode of different type */
+ /*
+ * Don't reuse inode of different type, even if path matches.
+ */
if (inode_wrong_type(inode, st->st_mode))
return 0;
- if (inode->i_generation != st->st_gen)
- return 0;
-
- /* compare qid details */
- if (memcmp(&v9inode->qid.version,
- &st->qid.version, sizeof(v9inode->qid.version)))
- return 0;
-
if (v9inode->qid.type != st->qid.type)
return 0;
if (v9inode->qid.path != st->qid.path)
return 0;
+
+ if (cached) {
+ /*
+ * Server side changes are not supposed to happen in cached mode.
+ * If we fail this generation or version comparison on the inode,
+ * we don't reuse it.
+ */
+ if (inode->i_generation != st->st_gen)
+ return 0;
+
+ /* compare qid details */
+ if (memcmp(&v9inode->qid.version,
+ &st->qid.version, sizeof(v9inode->qid.version)))
+ return 0;
+ }
+
+ if (v9fs_inode_ident_path(v9ses) && dentry) {
+ if (v9inode->path) {
+ if (!ino_path_compare(v9inode->path, dentry)) {
+ p9_debug(
+ P9_DEBUG_VFS,
+ "Refusing to reuse inode %p based on path mismatch",
+ inode);
+ return 0;
+ }
+ } else if (inode->i_state & I_NEW) {
+ /*
+ * iget5_locked may call this function with a still
+ * initializing (I_NEW) inode, so we're now racing with the
+ * code in v9fs_qid_iget_dotl that prepares v9inode->path.
+ * Returning from this test function now with positive result
+ * will cause us to wait for this inode to be ready, and we
+ * can then re-check in v9fs_qid_iget_dotl.
+ */
+ ((struct iget_data *)data)->need_double_check = true;
+ } else {
+ WARN_ONCE(
+ 1,
+ "Inode %p (ino %lu) does not have v9inode->path even though fs has path-based inode identification enabled?",
+ inode, inode->i_ino);
+ }
+ }
+
return 1;
}
-/* Always get a new inode */
static int v9fs_test_new_inode_dotl(struct inode *inode, void *data)
{
return 0;
}
-static int v9fs_set_inode_dotl(struct inode *inode, void *data)
+static int v9fs_set_inode_dotl(struct inode *inode, void *data)
{
struct v9fs_inode *v9inode = V9FS_I(inode);
- struct p9_stat_dotl *st = (struct p9_stat_dotl *)data;
+ struct iget_data *idata = data;
+ struct p9_stat_dotl *st = idata->st;
memcpy(&v9inode->qid, &st->qid, sizeof(st->qid));
inode->i_generation = st->st_gen;
+ /*
+ * We can't fill v9inode->path here, because allocating an ino_path
+ * means that we might sleep, and we can't sleep here.
+ */
+ v9inode->path = NULL;
return 0;
}
@@ -97,19 +151,56 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
struct p9_qid *qid,
struct p9_fid *fid,
struct p9_stat_dotl *st,
+ struct dentry *dentry,
int new)
{
int retval;
struct inode *inode;
+ struct v9fs_inode *v9inode;
struct v9fs_session_info *v9ses = sb->s_fs_info;
int (*test)(struct inode *inode, void *data);
+ struct iget_data data = {
+ .st = st,
+ .dentry = dentry,
+ .need_double_check = false,
+ };
if (new)
test = v9fs_test_new_inode_dotl;
else
test = v9fs_test_inode_dotl;
- inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode_dotl, st);
+ if (dentry) {
+ /*
+ * If we need to compare paths to find the inode to reuse, we need
+ * to take the rename_sem for this FS. We need to take it here,
+ * instead of inside ino_path_compare, as iget5_locked has
+ * spinlock in it (inode_hash_lock)
+ */
+ down_read(&v9ses->rename_sem);
+ }
+ while (true) {
+ data.need_double_check = false;
+ inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode_dotl, &data);
+ if (!data.need_double_check)
+ break;
+ /*
+ * Need to double check path as it wasn't initialized yet when we
+ * tested it
+ */
+ if (!inode || (inode->i_state & I_NEW)) {
+ WARN_ONCE(
+ 1,
+ "Expected iget5_locked to return an existing inode");
+ break;
+ }
+ if (ino_path_compare(V9FS_I(inode)->path, dentry))
+ break;
+ iput(inode);
+ }
+ if (dentry)
+ up_read(&v9ses->rename_sem);
+
if (!inode)
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
@@ -125,6 +216,17 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
if (retval)
goto error;
+ v9inode = V9FS_I(inode);
+ if (dentry) {
+ down_read(&v9ses->rename_sem);
+ v9inode->path = make_ino_path(dentry);
+ up_read(&v9ses->rename_sem);
+ if (!v9inode->path) {
+ retval = -ENOMEM;
+ goto error;
+ }
+ }
+
v9fs_stat2inode_dotl(st, inode, 0);
v9fs_set_netfs_context(inode);
v9fs_cache_inode_get_cookie(inode);
@@ -140,9 +242,18 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
}
+/**
+ * Issues a getattr request and use the result to look up the inode for
+ * the target pointed to by @fid.
+ * @v9ses: session information
+ * @fid: fid to issue attribute request for
+ * @sb: superblock on which to create inode
+ * @dentry: if not NULL, the path of the provided dentry is compared
+ * against the path stored in the inode, to determine reuse eligibility.
+ */
struct inode *
v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses, struct p9_fid *fid,
- struct super_block *sb, int new)
+ struct super_block *sb, struct dentry *dentry, int new)
{
struct p9_stat_dotl *st;
struct inode *inode = NULL;
@@ -151,7 +262,7 @@ v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses, struct p9_fid *fid,
if (IS_ERR(st))
return ERR_CAST(st);
- inode = v9fs_qid_iget_dotl(sb, &st->qid, fid, st, new);
+ inode = v9fs_qid_iget_dotl(sb, &st->qid, fid, st, dentry, new);
kfree(st);
return inode;
}
@@ -305,7 +416,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err);
goto out;
}
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb, dentry);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n", err);
@@ -400,7 +511,7 @@ static struct dentry *v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap,
}
/* instantiate inode and assign the unopened fid to the dentry */
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb, dentry);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
@@ -838,7 +949,7 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct inode *dir,
err);
goto error;
}
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb, dentry);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 795c6388744c..bb9e66f4631e 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -141,7 +141,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
sb->s_d_flags |= DCACHE_DONTCACHE;
}
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, sb);
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, sb, NULL);
if (IS_ERR(inode)) {
retval = PTR_ERR(inode);
goto release_sb;
@@ -153,6 +153,17 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
goto release_sb;
}
sb->s_root = root;
+
+ if (v9fs_inode_ident_path(v9ses)) {
+ /*
+ * This down_read is probably not necessary, just to satisfy
+ * lockdep_assert
+ */
+ down_read(&v9ses->rename_sem);
+ V9FS_I(inode)->path = make_ino_path(root);
+ up_read(&v9ses->rename_sem);
+ }
+
retval = v9fs_get_acl(inode, fid);
if (retval)
goto release_sb;
--
2.51.0
^ permalink raw reply related
* [PATCH v2 2/7] fs/9p: add option for path-based inodes
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
In-Reply-To: <cover.1756935780.git.m@maowtm.org>
By this point we have two ways to test for inode reuse - qid and qid+path.
By default, uncached mode uses qid+path and cached mode uses qid (and in
fact does not support qid+path). This patch adds the option to control
the behaviour for uncached mode.
In a future version, if we can negotiate with the server and be sure that
it won't give us duplicate qid.path, the default for those cases can be
qid-based.
Signed-off-by: Tingmao Wang <m@maowtm.org>
Cc: "Mickaël Salaün" <mic@digikod.net>
Cc: "Günther Noack" <gnoack@google.com>
---
Changes since v1:
- Removed inodeident=none and instead supports inodeident=qid. This means
that there is no longer an option to not re-use inodes at all.
- No longer supports inodeident=path on cached mode, checks added at
option init time.
- Added explicit bits for both V9FS_INODE_IDENT_PATH and
V9FS_INODE_IDENT_QID, in order to set a default based on cache bits when
neither are set explicitly by the user.
fs/9p/v9fs.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++-
fs/9p/v9fs.h | 3 +++
2 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 77e9c4387c1d..f87d6680b85a 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -36,7 +36,7 @@ enum {
/* Options that take integer arguments */
Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid,
/* String options */
- Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag,
+ Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag, Opt_inodeident,
/* Options that take no arguments */
Opt_nodevmap, Opt_noxattr, Opt_directio, Opt_ignoreqv,
/* Access options */
@@ -63,6 +63,7 @@ static const match_table_t tokens = {
{Opt_access, "access=%s"},
{Opt_posixacl, "posixacl"},
{Opt_locktimeout, "locktimeout=%u"},
+ {Opt_inodeident, "inodeident=%s"},
{Opt_err, NULL}
};
@@ -149,6 +150,21 @@ int v9fs_show_options(struct seq_file *m, struct dentry *root)
if (v9ses->flags & V9FS_NO_XATTR)
seq_puts(m, ",noxattr");
+ switch (v9ses->flags & V9FS_INODE_IDENT_MASK) {
+ case V9FS_INODE_IDENT_QID:
+ seq_puts(m, ",inodeident=qid");
+ break;
+ case V9FS_INODE_IDENT_PATH:
+ seq_puts(m, ",inodeident=path");
+ break;
+ default:
+ /*
+ * Unspecified, will be set later in v9fs_session_init depending on
+ * cache setting
+ */
+ break;
+ }
+
return p9_show_client_options(m, v9ses->clnt);
}
@@ -369,6 +385,26 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
v9ses->session_lock_timeout = (long)option * HZ;
break;
+ case Opt_inodeident:
+ s = match_strdup(&args[0]);
+ if (!s) {
+ ret = -ENOMEM;
+ p9_debug(P9_DEBUG_ERROR,
+ "problem allocating copy of inodeident arg\n");
+ goto free_and_return;
+ }
+ v9ses->flags &= ~V9FS_INODE_IDENT_MASK;
+ if (strcmp(s, "qid") == 0) {
+ v9ses->flags |= V9FS_INODE_IDENT_QID;
+ } else if (strcmp(s, "path") == 0) {
+ v9ses->flags |= V9FS_INODE_IDENT_PATH;
+ } else {
+ ret = -EINVAL;
+ p9_debug(P9_DEBUG_ERROR, "Unknown inodeident argument %s\n", s);
+ }
+ kfree(s);
+ break;
+
default:
continue;
}
@@ -393,6 +429,7 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
{
struct p9_fid *fid;
int rc = -ENOMEM;
+ bool cached;
v9ses->uname = kstrdup(V9FS_DEFUSER, GFP_KERNEL);
if (!v9ses->uname)
@@ -427,6 +464,26 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
if (rc < 0)
goto err_clnt;
+ cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
+
+ if (cached && v9ses->flags & V9FS_INODE_IDENT_PATH) {
+ rc = -EINVAL;
+ p9_debug(P9_DEBUG_ERROR,
+ "inodeident=path not supported in cached mode\n");
+ goto err_clnt;
+ }
+
+ if (!(v9ses->flags & V9FS_INODE_IDENT_MASK)) {
+ /* Unspecified - use default */
+ if (cached) {
+ /* which is qid in cached mode (path not supported) */
+ v9ses->flags |= V9FS_INODE_IDENT_QID;
+ } else {
+ /* ...or path in uncached mode */
+ v9ses->flags |= V9FS_INODE_IDENT_PATH;
+ }
+ }
+
v9ses->maxdata = v9ses->clnt->msize - P9_IOHDRSZ;
if (!v9fs_proto_dotl(v9ses) &&
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index 134b55a605be..b4e738c1bba5 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -43,8 +43,11 @@ enum p9_session_flags {
V9FS_DIRECT_IO = 0x100,
V9FS_SYNC = 0x200,
V9FS_INODE_IDENT_PATH = 0x400,
+ V9FS_INODE_IDENT_QID = 0x800,
};
+#define V9FS_INODE_IDENT_MASK (V9FS_INODE_IDENT_PATH | V9FS_INODE_IDENT_QID)
+
/**
* enum p9_cache_shortcuts - human readable cache preferences
* @CACHE_SC_NONE: disable all caches
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox