* [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling [not found] <20260503211835.16103-1-dwindsor@gmail.com> @ 2026-05-03 21:18 ` David Windsor 2026-05-04 20:14 ` Paul Moore 2026-05-03 21:18 ` [PATCH v2 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc David Windsor 1 sibling, 1 reply; 12+ messages in thread From: David Windsor @ 2026-05-03 21:18 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler Cc: Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set xattrs via the inode_init_security hook using lsm_get_xattr_slot(). The inode_init_security hook previously took the xattr array and count as two separate output parameters (struct xattr *xattrs, int *xattr_count), which BPF programs cannot write to. Pass the xattr state as a single context object (struct lsm_xattr_ctx) instead, and have bpf_init_inode_xattr() take that context directly. Update the existing in-tree callers of inode_init_security to take and forward the new lsm_xattr_ctx. Because we rely on the hook-specific ctx layout, the kfunc is restricted to lsm/inode_init_security. Restrict the xattr names that may be set via this kfunc to the bpf.* namespace. Suggested-by: Song Liu <song@kernel.org> Signed-off-by: David Windsor <dwindsor@gmail.com> --- fs/bpf_fs_kfuncs.c | 106 +++++++++++++++++++++++++++++- include/linux/bpf_lsm.h | 3 + include/linux/evm.h | 9 +-- include/linux/lsm_hook_defs.h | 4 +- include/linux/lsm_hooks.h | 16 ++--- include/linux/security.h | 5 ++ kernel/bpf/bpf_lsm.c | 1 + security/bpf/hooks.c | 1 + security/integrity/evm/evm_main.c | 8 ++- security/security.c | 7 +- security/selinux/hooks.c | 4 +- security/smack/smack_lsm.c | 13 ++-- 12 files changed, 147 insertions(+), 30 deletions(-) diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c index 9d27be058494..193accc00796 100644 --- a/fs/bpf_fs_kfuncs.c +++ b/fs/bpf_fs_kfuncs.c @@ -10,6 +10,7 @@ #include <linux/fsnotify.h> #include <linux/file.h> #include <linux/kernfs.h> +#include <linux/lsm_hooks.h> #include <linux/mm.h> #include <linux/xattr.h> @@ -353,6 +354,97 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s } #endif /* CONFIG_CGROUPS */ +static int bpf_xattrs_used(const struct lsm_xattr_ctx *ctx) +{ + const size_t prefix_len = sizeof(XATTR_BPF_LSM_SUFFIX) - 1; + int i, n = 0; + + for (i = 0; i < *ctx->xattr_count; i++) { + const char *name = ctx->xattrs[i].name; + + if (name && !strncmp(name, XATTR_BPF_LSM_SUFFIX, prefix_len)) + n++; + } + return n; +} + +static int __bpf_init_inode_xattr(struct lsm_xattr_ctx *xattr_ctx, + const char *name__str, + const struct bpf_dynptr *value_p) +{ + struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p; + size_t name_len; + void *xattr_value; + struct xattr *xattr; + struct xattr *xattrs; + int *xattr_count; + const void *value; + u32 value_len; + + if (!xattr_ctx || !name__str) + return -EINVAL; + + xattrs = xattr_ctx->xattrs; + xattr_count = xattr_ctx->xattr_count; + if (!xattrs || !xattr_count) + return -EINVAL; + if (bpf_xattrs_used(xattr_ctx) >= BPF_LSM_INODE_INIT_XATTRS) + return -ENOSPC; + + name_len = strlen(name__str); + if (name_len == 0 || name_len > XATTR_NAME_MAX) + return -EINVAL; + if (strncmp(name__str, XATTR_BPF_LSM_SUFFIX, + sizeof(XATTR_BPF_LSM_SUFFIX) - 1)) + return -EPERM; + + value_len = __bpf_dynptr_size(value_ptr); + if (value_len == 0 || value_len > XATTR_SIZE_MAX) + return -EINVAL; + + value = __bpf_dynptr_data(value_ptr, value_len); + if (!value) + return -EINVAL; + + /* Combine xattr value + name into one allocation. */ + xattr_value = kmalloc(value_len + name_len + 1, GFP_KERNEL); + if (!xattr_value) + return -ENOMEM; + + memcpy(xattr_value, value, value_len); + memcpy(xattr_value + value_len, name__str, name_len); + ((char *)xattr_value)[value_len + name_len] = '\0'; + + xattr = lsm_get_xattr_slot(xattr_ctx); + if (!xattr) { + kfree(xattr_value); + return -ENOSPC; + } + + xattr->value = xattr_value; + xattr->name = (const char *)xattr_value + value_len; + xattr->value_len = value_len; + + return 0; +} + +/** + * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security + * @xattr_ctx: inode_init_security xattr state from the hook context + * @name__str: xattr name (e.g., "bpf.file_label") + * @value_p: dynptr containing the xattr value + * + * Only callable from lsm/inode_init_security programs. + * + * Return: 0 on success, negative error on failure. + */ +__bpf_kfunc int bpf_init_inode_xattr(struct lsm_xattr_ctx *xattr_ctx, + const char *name__str, + const struct bpf_dynptr *value_p) +{ + return __bpf_init_inode_xattr(xattr_ctx, name__str, value_p); +} + __bpf_kfunc_end_defs(); BTF_KFUNCS_START(bpf_fs_kfunc_set_ids) @@ -363,13 +455,25 @@ BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE) +BTF_ID_FLAGS(func, bpf_init_inode_xattr, KF_SLEEPABLE) BTF_KFUNCS_END(bpf_fs_kfunc_set_ids) +BTF_ID_LIST(bpf_lsm_inode_init_security_btf_ids) +BTF_ID(func, bpf_lsm_inode_init_security) + +BTF_ID_LIST(bpf_init_inode_xattr_btf_ids) +BTF_ID(func, bpf_init_inode_xattr) + static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id) { if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) || - prog->type == BPF_PROG_TYPE_LSM) + prog->type == BPF_PROG_TYPE_LSM) { + /* bpf_init_inode_xattr only attaches to inode_init_security. */ + if (kfunc_id == bpf_init_inode_xattr_btf_ids[0] && + prog->aux->attach_btf_id != bpf_lsm_inode_init_security_btf_ids[0]) + return -EACCES; return 0; + } return -EACCES; } diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h index 643809cc78c3..b97a3d79529d 100644 --- a/include/linux/bpf_lsm.h +++ b/include/linux/bpf_lsm.h @@ -19,6 +19,9 @@ #include <linux/lsm_hook_defs.h> #undef LSM_HOOK +/* max bpf xattrs per inode */ +#define BPF_LSM_INODE_INIT_XATTRS 1 + struct bpf_storage_blob { struct bpf_local_storage __rcu *storage; }; diff --git a/include/linux/evm.h b/include/linux/evm.h index 913f4573b203..dff930bc10ba 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -12,6 +12,8 @@ #include <linux/integrity.h> #include <linux/xattr.h> +struct lsm_xattr_ctx; + #ifdef CONFIG_EVM extern int evm_set_key(void *key, size_t keylen); extern enum integrity_status evm_verifyxattr(struct dentry *dentry, @@ -21,8 +23,8 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry, int evm_fix_hmac(struct dentry *dentry, const char *xattr_name, const char *xattr_value, size_t xattr_value_len); int evm_inode_init_security(struct inode *inode, struct inode *dir, - const struct qstr *qstr, struct xattr *xattrs, - int *xattr_count); + const struct qstr *qstr, + struct lsm_xattr_ctx *xattr_ctx); extern bool evm_revalidate_status(const char *xattr_name); extern int evm_protected_xattr_if_enabled(const char *req_xattr_name); extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, @@ -63,8 +65,7 @@ static inline int evm_fix_hmac(struct dentry *dentry, const char *xattr_name, static inline int evm_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, - struct xattr *xattrs, - int *xattr_count) + struct lsm_xattr_ctx *xattr_ctx) { return 0; } diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 2b8dfb35caed..0df364ebb0a5 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -116,8 +116,8 @@ LSM_HOOK(int, 0, inode_alloc_security, struct inode *inode) LSM_HOOK(void, LSM_RET_VOID, inode_free_security, struct inode *inode) LSM_HOOK(void, LSM_RET_VOID, inode_free_security_rcu, void *inode_security) LSM_HOOK(int, -EOPNOTSUPP, inode_init_security, struct inode *inode, - struct inode *dir, const struct qstr *qstr, struct xattr *xattrs, - int *xattr_count) + struct inode *dir, const struct qstr *qstr, + struct lsm_xattr_ctx *xattr_ctx) LSM_HOOK(int, 0, inode_init_security_anon, struct inode *inode, const struct qstr *name, const struct inode *context_inode) LSM_HOOK(int, 0, inode_create, struct inode *dir, struct dentry *dentry, diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index b4f8cad53ddb..2133b729e87d 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -200,20 +200,18 @@ extern struct lsm_static_calls_table static_calls_table __ro_after_init; /** * lsm_get_xattr_slot - Return the next available slot and increment the index - * @xattrs: array storing LSM-provided xattrs - * @xattr_count: number of already stored xattrs (updated) + * @ctx: xattr state shared by inode_init_security hooks * - * Retrieve the first available slot in the @xattrs array to fill with an xattr, - * and increment @xattr_count. + * Retrieve the first available slot in the @ctx->xattrs array to fill with an + * xattr, and increment @ctx->xattr_count. * - * Return: The slot to fill in @xattrs if non-NULL, NULL otherwise. + * Return: The slot to fill in @ctx->xattrs if non-NULL, NULL otherwise. */ -static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs, - int *xattr_count) +static inline struct xattr *lsm_get_xattr_slot(struct lsm_xattr_ctx *ctx) { - if (unlikely(!xattrs)) + if (unlikely(!ctx || !ctx->xattrs || !ctx->xattr_count)) return NULL; - return &xattrs[(*xattr_count)++]; + return &ctx->xattrs[(*ctx->xattr_count)++]; } #endif /* ! __LINUX_LSM_HOOKS_H */ diff --git a/include/linux/security.h b/include/linux/security.h index 41d7367cf403..a2fc72e63ada 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -68,6 +68,11 @@ struct watch; struct watch_notification; struct lsm_ctx; +struct lsm_xattr_ctx { + struct xattr *xattrs; + int *xattr_count; +}; + /* Default (no) options for the capable function */ #define CAP_OPT_NONE 0x0 /* If capable should audit the security request */ diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c index c5c925f00202..fbbb4e1c04fc 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -315,6 +315,7 @@ BTF_ID(func, bpf_lsm_inode_create) BTF_ID(func, bpf_lsm_inode_free_security) BTF_ID(func, bpf_lsm_inode_getattr) BTF_ID(func, bpf_lsm_inode_getxattr) +BTF_ID(func, bpf_lsm_inode_init_security) BTF_ID(func, bpf_lsm_inode_mknod) BTF_ID(func, bpf_lsm_inode_need_killpriv) BTF_ID(func, bpf_lsm_inode_post_setxattr) diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c index 40efde233f3a..d7c44c5c0e30 100644 --- a/security/bpf/hooks.c +++ b/security/bpf/hooks.c @@ -30,6 +30,7 @@ static int __init bpf_lsm_init(void) struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = { .lbs_inode = sizeof(struct bpf_storage_blob), + .lbs_xattr_count = BPF_LSM_INODE_INIT_XATTRS, }; DEFINE_LSM(bpf) = { diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b59e3f121b8a..c25301f25a0a 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -1062,14 +1062,16 @@ static int evm_inode_copy_up_xattr(struct dentry *src, const char *name) * evm_inode_init_security - initializes security.evm HMAC value */ int evm_inode_init_security(struct inode *inode, struct inode *dir, - const struct qstr *qstr, struct xattr *xattrs, - int *xattr_count) + const struct qstr *qstr, + struct lsm_xattr_ctx *xattr_ctx) { struct evm_xattr *xattr_data; struct xattr *xattr, *evm_xattr; + struct xattr *xattrs; bool evm_protected_xattrs = false; int rc; + xattrs = xattr_ctx ? xattr_ctx->xattrs : NULL; if (!(evm_initialized & EVM_INIT_HMAC) || !xattrs) return 0; @@ -1087,7 +1089,7 @@ int evm_inode_init_security(struct inode *inode, struct inode *dir, if (!evm_protected_xattrs) return 0; - evm_xattr = lsm_get_xattr_slot(xattrs, xattr_count); + evm_xattr = lsm_get_xattr_slot(xattr_ctx); /* * Array terminator (xattr name = NULL) must be the first non-filled * xattr slot. diff --git a/security/security.c b/security/security.c index 4e999f023651..4cd43914ce93 100644 --- a/security/security.c +++ b/security/security.c @@ -1334,6 +1334,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir, { struct lsm_static_call *scall; struct xattr *new_xattrs = NULL; + struct lsm_xattr_ctx xattr_ctx; int ret = -EOPNOTSUPP, xattr_count = 0; if (unlikely(IS_PRIVATE(inode))) @@ -1349,10 +1350,12 @@ int security_inode_init_security(struct inode *inode, struct inode *dir, if (!new_xattrs) return -ENOMEM; } + xattr_ctx.xattrs = new_xattrs; + xattr_ctx.xattr_count = &xattr_count; lsm_for_each_hook(scall, inode_init_security) { - ret = scall->hl->hook.inode_init_security(inode, dir, qstr, new_xattrs, - &xattr_count); + ret = scall->hl->hook.inode_init_security(inode, dir, qstr, + &xattr_ctx); if (ret && ret != -EOPNOTSUPP) goto out; /* diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 97801966bf32..dca81a22bf83 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2962,11 +2962,11 @@ static int selinux_dentry_create_files_as(struct dentry *dentry, int mode, static int selinux_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, - struct xattr *xattrs, int *xattr_count) + struct lsm_xattr_ctx *xattr_ctx) { const struct cred_security_struct *crsec = selinux_cred(current_cred()); struct superblock_security_struct *sbsec; - struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count); + struct xattr *xattr = lsm_get_xattr_slot(xattr_ctx); u32 newsid, clen; u16 newsclass; int rc; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 3f9ae05039a2..ea9549c666a1 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -981,10 +981,10 @@ smk_rule_transmutes(struct smack_known *subject, } static int -xattr_dupval(struct xattr *xattrs, int *xattr_count, +xattr_dupval(struct lsm_xattr_ctx *xattr_ctx, const char *name, const void *value, unsigned int vallen) { - struct xattr * const xattr = lsm_get_xattr_slot(xattrs, xattr_count); + struct xattr * const xattr = lsm_get_xattr_slot(xattr_ctx); if (!xattr) return 0; @@ -1003,14 +1003,13 @@ xattr_dupval(struct xattr *xattrs, int *xattr_count, * @inode: the newly created inode * @dir: containing directory object * @qstr: unused - * @xattrs: where to put the attributes - * @xattr_count: current number of LSM-provided xattrs (updated) + * @xattr_ctx: where to put attributes and update count * * Returns 0 if it all works out, -ENOMEM if there's no memory */ static int smack_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, - struct xattr *xattrs, int *xattr_count) + struct lsm_xattr_ctx *xattr_ctx) { struct task_smack *tsp = smack_cred(current_cred()); struct inode_smack * const issp = smack_inode(inode); @@ -1057,7 +1056,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, if (S_ISDIR(inode->i_mode)) { transflag = SMK_INODE_TRANSMUTE; - if (xattr_dupval(xattrs, xattr_count, + if (xattr_dupval(xattr_ctx, XATTR_SMACK_TRANSMUTE, TRANS_TRUE, TRANS_TRUE_SIZE @@ -1067,7 +1066,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, } if (rc == 0) - if (xattr_dupval(xattrs, xattr_count, + if (xattr_dupval(xattr_ctx, XATTR_SMACK_SUFFIX, issp->smk_inode->smk_known, strlen(issp->smk_inode->smk_known) -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-03 21:18 ` [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling David Windsor @ 2026-05-04 20:14 ` Paul Moore 2026-05-04 21:40 ` Song Liu 0 siblings, 1 reply; 12+ messages in thread From: Paul Moore @ 2026-05-04 20:14 UTC (permalink / raw) To: David Windsor Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Sun, May 3, 2026 at 5:18 PM David Windsor <dwindsor@gmail.com> wrote: > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > xattrs via the inode_init_security hook using lsm_get_xattr_slot(). > > The inode_init_security hook previously took the xattr array and count > as two separate output parameters (struct xattr *xattrs, int > *xattr_count), which BPF programs cannot write to. Pass the xattr state > as a single context object (struct lsm_xattr_ctx) instead, and have > bpf_init_inode_xattr() take that context directly. Update the existing > in-tree callers of inode_init_security to take and forward the new > lsm_xattr_ctx. > > Because we rely on the hook-specific ctx layout, the kfunc is > restricted to lsm/inode_init_security. Restrict the xattr names that > may be set via this kfunc to the bpf.* namespace. > > Suggested-by: Song Liu <song@kernel.org> > Signed-off-by: David Windsor <dwindsor@gmail.com> > --- > fs/bpf_fs_kfuncs.c | 106 +++++++++++++++++++++++++++++- > include/linux/bpf_lsm.h | 3 + > include/linux/evm.h | 9 +-- > include/linux/lsm_hook_defs.h | 4 +- > include/linux/lsm_hooks.h | 16 ++--- > include/linux/security.h | 5 ++ > kernel/bpf/bpf_lsm.c | 1 + > security/bpf/hooks.c | 1 + > security/integrity/evm/evm_main.c | 8 ++- > security/security.c | 7 +- > security/selinux/hooks.c | 4 +- > security/smack/smack_lsm.c | 13 ++-- > 12 files changed, 147 insertions(+), 30 deletions(-) Comments below ... > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c > index 9d27be058494..193accc00796 100644 > --- a/fs/bpf_fs_kfuncs.c > +++ b/fs/bpf_fs_kfuncs.c > @@ -10,6 +10,7 @@ > #include <linux/fsnotify.h> > #include <linux/file.h> > #include <linux/kernfs.h> > +#include <linux/lsm_hooks.h> > #include <linux/mm.h> > #include <linux/xattr.h> > > @@ -353,6 +354,97 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s > } > #endif /* CONFIG_CGROUPS */ > > +static int bpf_xattrs_used(const struct lsm_xattr_ctx *ctx) > +{ > + const size_t prefix_len = sizeof(XATTR_BPF_LSM_SUFFIX) - 1; > + int i, n = 0; > + > + for (i = 0; i < *ctx->xattr_count; i++) { > + const char *name = ctx->xattrs[i].name; > + > + if (name && !strncmp(name, XATTR_BPF_LSM_SUFFIX, prefix_len)) > + n++; > + } > + return n; > +} > + > +static int __bpf_init_inode_xattr(struct lsm_xattr_ctx *xattr_ctx, > + const char *name__str, > + const struct bpf_dynptr *value_p) > +{ > + struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p; > + size_t name_len; > + void *xattr_value; > + struct xattr *xattr; > + struct xattr *xattrs; > + int *xattr_count; > + const void *value; > + u32 value_len; > + > + if (!xattr_ctx || !name__str) > + return -EINVAL; > + > + xattrs = xattr_ctx->xattrs; > + xattr_count = xattr_ctx->xattr_count; > + if (!xattrs || !xattr_count) > + return -EINVAL; > + if (bpf_xattrs_used(xattr_ctx) >= BPF_LSM_INODE_INIT_XATTRS) > + return -ENOSPC; > + > + name_len = strlen(name__str); > + if (name_len == 0 || name_len > XATTR_NAME_MAX) > + return -EINVAL; > + if (strncmp(name__str, XATTR_BPF_LSM_SUFFIX, > + sizeof(XATTR_BPF_LSM_SUFFIX) - 1)) > + return -EPERM; > + > + value_len = __bpf_dynptr_size(value_ptr); > + if (value_len == 0 || value_len > XATTR_SIZE_MAX) > + return -EINVAL; > + > + value = __bpf_dynptr_data(value_ptr, value_len); > + if (!value) > + return -EINVAL; > + > + /* Combine xattr value + name into one allocation. */ > + xattr_value = kmalloc(value_len + name_len + 1, GFP_KERNEL); > + if (!xattr_value) > + return -ENOMEM; > + > + memcpy(xattr_value, value, value_len); > + memcpy(xattr_value + value_len, name__str, name_len); > + ((char *)xattr_value)[value_len + name_len] = '\0'; > + > + xattr = lsm_get_xattr_slot(xattr_ctx); > + if (!xattr) { > + kfree(xattr_value); > + return -ENOSPC; > + } > + > + xattr->value = xattr_value; > + xattr->name = (const char *)xattr_value + value_len; > + xattr->value_len = value_len; > + > + return 0; > +} > + > +/** > + * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security > + * @xattr_ctx: inode_init_security xattr state from the hook context > + * @name__str: xattr name (e.g., "bpf.file_label") > + * @value_p: dynptr containing the xattr value > + * > + * Only callable from lsm/inode_init_security programs. > + * > + * Return: 0 on success, negative error on failure. > + */ > +__bpf_kfunc int bpf_init_inode_xattr(struct lsm_xattr_ctx *xattr_ctx, > + const char *name__str, > + const struct bpf_dynptr *value_p) > +{ > + return __bpf_init_inode_xattr(xattr_ctx, name__str, value_p); > +} > + > __bpf_kfunc_end_defs(); > > BTF_KFUNCS_START(bpf_fs_kfunc_set_ids) > @@ -363,13 +455,25 @@ BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE) > BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE) > BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE) > BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE) > +BTF_ID_FLAGS(func, bpf_init_inode_xattr, KF_SLEEPABLE) > BTF_KFUNCS_END(bpf_fs_kfunc_set_ids) > > +BTF_ID_LIST(bpf_lsm_inode_init_security_btf_ids) > +BTF_ID(func, bpf_lsm_inode_init_security) > + > +BTF_ID_LIST(bpf_init_inode_xattr_btf_ids) > +BTF_ID(func, bpf_init_inode_xattr) > + > static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id) > { > if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) || > - prog->type == BPF_PROG_TYPE_LSM) > + prog->type == BPF_PROG_TYPE_LSM) { > + /* bpf_init_inode_xattr only attaches to inode_init_security. */ > + if (kfunc_id == bpf_init_inode_xattr_btf_ids[0] && > + prog->aux->attach_btf_id != bpf_lsm_inode_init_security_btf_ids[0]) > + return -EACCES; > return 0; > + } > return -EACCES; > } Perhaps I'm simply not seeing it, but is there a check to ensure that there is only one BPF LSM calling into security_inode_init_security() at any given time? With the BPF LSM only reserving a single xattr slot, multiple loaded BPF LSM programs providing security_inode_init_security() callbacks will be a problem. > diff --git a/include/linux/security.h b/include/linux/security.h > index 41d7367cf403..a2fc72e63ada 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -68,6 +68,11 @@ struct watch; > struct watch_notification; > struct lsm_ctx; > > +struct lsm_xattr_ctx { > + struct xattr *xattrs; > + int *xattr_count; > +}; I'd prefer this to be simply "struct lsm_xattrs" as "ctx" is an overloaded term in the LSM space. > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 97801966bf32..dca81a22bf83 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2962,11 +2962,11 @@ static int selinux_dentry_create_files_as(struct dentry *dentry, int mode, > > static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > const struct qstr *qstr, > - struct xattr *xattrs, int *xattr_count) > + struct lsm_xattr_ctx *xattr_ctx) > { > const struct cred_security_struct *crsec = selinux_cred(current_cred()); > struct superblock_security_struct *sbsec; > - struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count); > + struct xattr *xattr = lsm_get_xattr_slot(xattr_ctx); > u32 newsid, clen; > u16 newsclass; > int rc; In case you didn't see it, your fix for the above lsm_get_xattr_slot() usage is now in Linus' tree. It's a trivial bit of merge fuzz, but you might want to rebase your next revision. -- paul-moore.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-04 20:14 ` Paul Moore @ 2026-05-04 21:40 ` Song Liu 2026-05-04 22:42 ` Paul Moore 0 siblings, 1 reply; 12+ messages in thread From: Song Liu @ 2026-05-04 21:40 UTC (permalink / raw) To: Paul Moore Cc: David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Mon, May 4, 2026 at 10:14 PM Paul Moore <paul@paul-moore.com> wrote: [...] > > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c > > index 9d27be058494..193accc00796 100644 > > --- a/fs/bpf_fs_kfuncs.c > > +++ b/fs/bpf_fs_kfuncs.c > > @@ -10,6 +10,7 @@ > > #include <linux/fsnotify.h> > > #include <linux/file.h> > > #include <linux/kernfs.h> > > +#include <linux/lsm_hooks.h> > > #include <linux/mm.h> > > #include <linux/xattr.h> > > > > @@ -353,6 +354,97 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s > > } > > #endif /* CONFIG_CGROUPS */ > > > > +static int bpf_xattrs_used(const struct lsm_xattr_ctx *ctx) > > +{ > > + const size_t prefix_len = sizeof(XATTR_BPF_LSM_SUFFIX) - 1; > > + int i, n = 0; > > + > > + for (i = 0; i < *ctx->xattr_count; i++) { > > + const char *name = ctx->xattrs[i].name; > > + > > + if (name && !strncmp(name, XATTR_BPF_LSM_SUFFIX, prefix_len)) > > + n++; > > + } > > + return n; > > +} [...] > > + > > static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id) > > { > > if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) || > > - prog->type == BPF_PROG_TYPE_LSM) > > + prog->type == BPF_PROG_TYPE_LSM) { > > + /* bpf_init_inode_xattr only attaches to inode_init_security. */ > > + if (kfunc_id == bpf_init_inode_xattr_btf_ids[0] && > > + prog->aux->attach_btf_id != bpf_lsm_inode_init_security_btf_ids[0]) > > + return -EACCES; We need to mark bpf_init_inode_xattr with KF_RCU (requires a trusted pointer), then we can remove this check above. > > return 0; > > + } > > return -EACCES; > > } > > Perhaps I'm simply not seeing it, but is there a check to ensure that > there is only one BPF LSM calling into security_inode_init_security() > at any given time? With the BPF LSM only reserving a single xattr > slot, multiple loaded BPF LSM programs providing > security_inode_init_security() callbacks will be a problem. I don't think there is such a check. Also, a single BPF LSM function may call the kfunc multiple times, which is also problematic. I think we will need to make the default bigger, and also introduce some realloc mechanism for the worst case scenario. This should work, but the code might be a bit messy. Thanks, Song > > > diff --git a/include/linux/security.h b/include/linux/security.h > > index 41d7367cf403..a2fc72e63ada 100644 > > --- a/include/linux/security.h > > +++ b/include/linux/security.h > > @@ -68,6 +68,11 @@ struct watch; > > struct watch_notification; > > struct lsm_ctx; > > [...] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-04 21:40 ` Song Liu @ 2026-05-04 22:42 ` Paul Moore 2026-05-04 23:09 ` Song Liu 0 siblings, 1 reply; 12+ messages in thread From: Paul Moore @ 2026-05-04 22:42 UTC (permalink / raw) To: Song Liu Cc: David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Mon, May 4, 2026 at 5:40 PM Song Liu <song@kernel.org> wrote: > On Mon, May 4, 2026 at 10:14 PM Paul Moore <paul@paul-moore.com> wrote: > [...] > > > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c > > > index 9d27be058494..193accc00796 100644 > > > --- a/fs/bpf_fs_kfuncs.c > > > +++ b/fs/bpf_fs_kfuncs.c > > > @@ -10,6 +10,7 @@ > > > #include <linux/fsnotify.h> > > > #include <linux/file.h> > > > #include <linux/kernfs.h> > > > +#include <linux/lsm_hooks.h> > > > #include <linux/mm.h> > > > #include <linux/xattr.h> > > > > > > @@ -353,6 +354,97 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s > > > } > > > #endif /* CONFIG_CGROUPS */ > > > > > > +static int bpf_xattrs_used(const struct lsm_xattr_ctx *ctx) > > > +{ > > > + const size_t prefix_len = sizeof(XATTR_BPF_LSM_SUFFIX) - 1; > > > + int i, n = 0; > > > + > > > + for (i = 0; i < *ctx->xattr_count; i++) { > > > + const char *name = ctx->xattrs[i].name; > > > + > > > + if (name && !strncmp(name, XATTR_BPF_LSM_SUFFIX, prefix_len)) > > > + n++; > > > + } > > > + return n; > > > +} > [...] > > > + > > > static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id) > > > { > > > if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) || > > > - prog->type == BPF_PROG_TYPE_LSM) > > > + prog->type == BPF_PROG_TYPE_LSM) { > > > + /* bpf_init_inode_xattr only attaches to inode_init_security. */ > > > + if (kfunc_id == bpf_init_inode_xattr_btf_ids[0] && > > > + prog->aux->attach_btf_id != bpf_lsm_inode_init_security_btf_ids[0]) > > > + return -EACCES; > > We need to mark bpf_init_inode_xattr with KF_RCU (requires a trusted > pointer), then we can remove this check above. > > > > return 0; > > > + } > > > return -EACCES; > > > } > > > > Perhaps I'm simply not seeing it, but is there a check to ensure that > > there is only one BPF LSM calling into security_inode_init_security() > > at any given time? With the BPF LSM only reserving a single xattr > > slot, multiple loaded BPF LSM programs providing > > security_inode_init_security() callbacks will be a problem. > > I don't think there is such a check. Also, a single BPF LSM function > may call the kfunc multiple times, which is also problematic. > > I think we will need to make the default bigger, and also introduce > some realloc mechanism for the worst case scenario. This should > work, but the code might be a bit messy. Thanks for the clarification, that is what I was afraid of when looking at the code, but I was hoping I was just missing it. Increasing the default is an option, but I don't think we want to support a dynamic reallocation scheme for the xattr slots, that will likely get extremely messy with synchronization between the LSM framework and BPF LSM hook registrations as well as special code to handle inodes with lifetimes that are disjoint from the BPF LSM programs ... I suppose there may be a way to do it, but it will surely be ugly and come at a cost. -- paul-moore.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-04 22:42 ` Paul Moore @ 2026-05-04 23:09 ` Song Liu 2026-05-05 1:07 ` David Windsor 2026-05-05 2:02 ` Paul Moore 0 siblings, 2 replies; 12+ messages in thread From: Song Liu @ 2026-05-04 23:09 UTC (permalink / raw) To: Paul Moore Cc: David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Tue, May 5, 2026 at 12:42 AM Paul Moore <paul@paul-moore.com> wrote: [...] > > > Perhaps I'm simply not seeing it, but is there a check to ensure that > > > there is only one BPF LSM calling into security_inode_init_security() > > > at any given time? With the BPF LSM only reserving a single xattr > > > slot, multiple loaded BPF LSM programs providing > > > security_inode_init_security() callbacks will be a problem. > > > > I don't think there is such a check. Also, a single BPF LSM function > > may call the kfunc multiple times, which is also problematic. > > > > I think we will need to make the default bigger, and also introduce > > some realloc mechanism for the worst case scenario. This should > > work, but the code might be a bit messy. > > Thanks for the clarification, that is what I was afraid of when > looking at the code, but I was hoping I was just missing it. > > Increasing the default is an option, but I don't think we want to > support a dynamic reallocation scheme for the xattr slots, that will > likely get extremely messy with synchronization between the LSM > framework and BPF LSM hook registrations as well as special code to > handle inodes with lifetimes that are disjoint from the BPF LSM > programs ... I suppose there may be a way to do it, but it will surely > be ugly and come at a cost. BPF trampoline already handles all the synchronizations, such as add hook, remove hook, etc. properly. So this is not that hard. All we really need is to allocate a new array, copy pointers, and free the old array. And we only really need this in the worst case scenarios. Thanks, Song ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-04 23:09 ` Song Liu @ 2026-05-05 1:07 ` David Windsor 2026-05-05 2:02 ` Paul Moore 1 sibling, 0 replies; 12+ messages in thread From: David Windsor @ 2026-05-05 1:07 UTC (permalink / raw) To: Song Liu Cc: Paul Moore, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Mon, May 4, 2026 at 7:09 PM Song Liu <song@kernel.org> wrote: > > On Tue, May 5, 2026 at 12:42 AM Paul Moore <paul@paul-moore.com> wrote: > [...] > > > > Perhaps I'm simply not seeing it, but is there a check to ensure that > > > > there is only one BPF LSM calling into security_inode_init_security() > > > > at any given time? With the BPF LSM only reserving a single xattr > > > > slot, multiple loaded BPF LSM programs providing > > > > security_inode_init_security() callbacks will be a problem. > > > > > > I don't think there is such a check. Also, a single BPF LSM function > > > may call the kfunc multiple times, which is also problematic. > > > bpf_xattrs_used() guards against this. The lsm_xattr_ctx is shared between all callers, so xattr additions by another LSM (or by calling it multiple times in the same function) will be tracked by this. > > > I think we will need to make the default bigger, and also introduce > > > some realloc mechanism for the worst case scenario. This should > > > work, but the code might be a bit messy. > > > > Thanks for the clarification, that is what I was afraid of when > > looking at the code, but I was hoping I was just missing it. > > > > Increasing the default is an option, but I don't think we want to > > support a dynamic reallocation scheme for the xattr slots, that will > > likely get extremely messy with synchronization between the LSM > > framework and BPF LSM hook registrations as well as special code to > > handle inodes with lifetimes that are disjoint from the BPF LSM > > programs ... I suppose there may be a way to do it, but it will surely > > be ugly and come at a cost. > > BPF trampoline already handles all the synchronizations, such as > add hook, remove hook, etc. properly. So this is not that hard. > All we really need is to allocate a new array, copy pointers, and free > the old array. And we only really need this in the worst case > scenarios. > How many bpf-lsm programs do we envision being attached at once? I'd think that stacking of bpf-lsms would be difficult to reason about (moreso than static LSMs) and won't work that well in practice, but may be wrong. Most LSMs use 1 xattr, Smack is the only one who uses 2 IIRC. > Thanks, > Song ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-04 23:09 ` Song Liu 2026-05-05 1:07 ` David Windsor @ 2026-05-05 2:02 ` Paul Moore 2026-05-05 2:05 ` Paul Moore 2026-05-05 9:00 ` Song Liu 1 sibling, 2 replies; 12+ messages in thread From: Paul Moore @ 2026-05-05 2:02 UTC (permalink / raw) To: Song Liu Cc: David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Mon, May 4, 2026 at 7:09 PM Song Liu <song@kernel.org> wrote: > On Tue, May 5, 2026 at 12:42 AM Paul Moore <paul@paul-moore.com> wrote: > [...] > > > > Perhaps I'm simply not seeing it, but is there a check to ensure that > > > > there is only one BPF LSM calling into security_inode_init_security() > > > > at any given time? With the BPF LSM only reserving a single xattr > > > > slot, multiple loaded BPF LSM programs providing > > > > security_inode_init_security() callbacks will be a problem. > > > > > > I don't think there is such a check. Also, a single BPF LSM function > > > may call the kfunc multiple times, which is also problematic. > > > > > > I think we will need to make the default bigger, and also introduce > > > some realloc mechanism for the worst case scenario. This should > > > work, but the code might be a bit messy. > > > > Thanks for the clarification, that is what I was afraid of when > > looking at the code, but I was hoping I was just missing it. > > > > Increasing the default is an option, but I don't think we want to > > support a dynamic reallocation scheme for the xattr slots, that will > > likely get extremely messy with synchronization between the LSM > > framework and BPF LSM hook registrations as well as special code to > > handle inodes with lifetimes that are disjoint from the BPF LSM > > programs ... I suppose there may be a way to do it, but it will surely > > be ugly and come at a cost. > > BPF trampoline already handles all the synchronizations, such as > add hook, remove hook, etc. properly. So this is not that hard. How do you plan to handle the issue of disjoint lifetimes? > All we really need is to allocate a new array, copy pointers, and free > the old array. And we only really need this in the worst case > scenarios. Oh, is that all? :D Keep in mind that the code must also handle arbitrary ordering of LSMs; in other words, you must handle a BPF LSM that isn't at the end of the LSM order. While a BPF LSM at the end of the LSM list is the most common, and recommended ordering for the vast majority of users, we've been working to make the ordering as generalized as possible. -- paul-moore.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-05 2:02 ` Paul Moore @ 2026-05-05 2:05 ` Paul Moore 2026-05-05 9:00 ` Song Liu 1 sibling, 0 replies; 12+ messages in thread From: Paul Moore @ 2026-05-05 2:05 UTC (permalink / raw) To: Song Liu Cc: David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Mon, May 4, 2026 at 10:02 PM Paul Moore <paul@paul-moore.com> wrote: > On Mon, May 4, 2026 at 7:09 PM Song Liu <song@kernel.org> wrote: > > On Tue, May 5, 2026 at 12:42 AM Paul Moore <paul@paul-moore.com> wrote: > > [...] > > > > > Perhaps I'm simply not seeing it, but is there a check to ensure that > > > > > there is only one BPF LSM calling into security_inode_init_security() > > > > > at any given time? With the BPF LSM only reserving a single xattr > > > > > slot, multiple loaded BPF LSM programs providing > > > > > security_inode_init_security() callbacks will be a problem. > > > > > > > > I don't think there is such a check. Also, a single BPF LSM function > > > > may call the kfunc multiple times, which is also problematic. > > > > > > > > I think we will need to make the default bigger, and also introduce > > > > some realloc mechanism for the worst case scenario. This should > > > > work, but the code might be a bit messy. > > > > > > Thanks for the clarification, that is what I was afraid of when > > > looking at the code, but I was hoping I was just missing it. > > > > > > Increasing the default is an option, but I don't think we want to > > > support a dynamic reallocation scheme for the xattr slots, that will > > > likely get extremely messy with synchronization between the LSM > > > framework and BPF LSM hook registrations as well as special code to > > > handle inodes with lifetimes that are disjoint from the BPF LSM > > > programs ... I suppose there may be a way to do it, but it will surely > > > be ugly and come at a cost. > > > > BPF trampoline already handles all the synchronizations, such as > > add hook, remove hook, etc. properly. So this is not that hard. > > How do you plan to handle the issue of disjoint lifetimes? > > > All we really need is to allocate a new array, copy pointers, and free > > the old array. And we only really need this in the worst case > > scenarios. > > Oh, is that all? :D > > Keep in mind that the code must also handle arbitrary ordering of > LSMs; in other words, you must handle a BPF LSM that isn't at the end > of the LSM order. While a BPF LSM at the end of the LSM list is the > most common, and recommended ordering for the vast majority of users, > we've been working to make the ordering as generalized as possible. I just realized I probably wasn't as clear as I should have been ... I really don't like telling people not to go experiment with things and play with the code as that feels wrong for many reasons, but I do want to warn you that if the code to handle this ends up looking like I think it will, I'm not going to want to support it in the LSM framework. -- paul-moore.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-05 2:02 ` Paul Moore 2026-05-05 2:05 ` Paul Moore @ 2026-05-05 9:00 ` Song Liu 2026-05-05 13:49 ` Paul Moore 1 sibling, 1 reply; 12+ messages in thread From: Song Liu @ 2026-05-05 9:00 UTC (permalink / raw) To: Paul Moore Cc: David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Tue, May 5, 2026 at 4:02 AM Paul Moore <paul@paul-moore.com> wrote: > > On Mon, May 4, 2026 at 7:09 PM Song Liu <song@kernel.org> wrote: > > On Tue, May 5, 2026 at 12:42 AM Paul Moore <paul@paul-moore.com> wrote: > > [...] > > > > > Perhaps I'm simply not seeing it, but is there a check to ensure that > > > > > there is only one BPF LSM calling into security_inode_init_security() > > > > > at any given time? With the BPF LSM only reserving a single xattr > > > > > slot, multiple loaded BPF LSM programs providing > > > > > security_inode_init_security() callbacks will be a problem. > > > > > > > > I don't think there is such a check. Also, a single BPF LSM function > > > > may call the kfunc multiple times, which is also problematic. > > > > > > > > I think we will need to make the default bigger, and also introduce > > > > some realloc mechanism for the worst case scenario. This should > > > > work, but the code might be a bit messy. > > > > > > Thanks for the clarification, that is what I was afraid of when > > > looking at the code, but I was hoping I was just missing it. > > > > > > Increasing the default is an option, but I don't think we want to > > > support a dynamic reallocation scheme for the xattr slots, that will > > > likely get extremely messy with synchronization between the LSM > > > framework and BPF LSM hook registrations as well as special code to > > > handle inodes with lifetimes that are disjoint from the BPF LSM > > > programs ... I suppose there may be a way to do it, but it will surely > > > be ugly and come at a cost. > > > > BPF trampoline already handles all the synchronizations, such as > > add hook, remove hook, etc. properly. So this is not that hard. > > How do you plan to handle the issue of disjoint lifetimes? > > > All we really need is to allocate a new array, copy pointers, and free > > the old array. And we only really need this in the worst case > > scenarios. > > Oh, is that all? :D > > Keep in mind that the code must also handle arbitrary ordering of > LSMs; in other words, you must handle a BPF LSM that isn't at the end > of the LSM order. While a BPF LSM at the end of the LSM list is the > most common, and recommended ordering for the vast majority of users, > we've been working to make the ordering as generalized as possible. All the BPF LSM hooks are called together, so it should be fine. Maybe I missed some corner cases. Either way, I agree with David that we don't need too many xattrs. Since BPF LSM is reserved to the privileged users only, it is safe to put a reasonable limit, say 4 or 8, and do not handle the realloc. If the admin would like to brick a system with BPF LSM, there are many other ways to do it. Thanks, Song ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-05-05 9:00 ` Song Liu @ 2026-05-05 13:49 ` Paul Moore 0 siblings, 0 replies; 12+ messages in thread From: Paul Moore @ 2026-05-05 13:49 UTC (permalink / raw) To: Song Liu Cc: David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, James Morris, Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Stephen Smalley, Casey Schaufler, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, Eric Snowberg, Ondrej Mosnacek, linux-fsdevel, linux-kernel, bpf, linux-security-module, linux-integrity, selinux On Tue, May 5, 2026 at 5:00 AM Song Liu <song@kernel.org> wrote: > On Tue, May 5, 2026 at 4:02 AM Paul Moore <paul@paul-moore.com> wrote: > > On Mon, May 4, 2026 at 7:09 PM Song Liu <song@kernel.org> wrote: > > > On Tue, May 5, 2026 at 12:42 AM Paul Moore <paul@paul-moore.com> wrote: > > > [...] > > > > > > Perhaps I'm simply not seeing it, but is there a check to ensure that > > > > > > there is only one BPF LSM calling into security_inode_init_security() > > > > > > at any given time? With the BPF LSM only reserving a single xattr > > > > > > slot, multiple loaded BPF LSM programs providing > > > > > > security_inode_init_security() callbacks will be a problem. > > > > > > > > > > I don't think there is such a check. Also, a single BPF LSM function > > > > > may call the kfunc multiple times, which is also problematic. > > > > > > > > > > I think we will need to make the default bigger, and also introduce > > > > > some realloc mechanism for the worst case scenario. This should > > > > > work, but the code might be a bit messy. > > > > > > > > Thanks for the clarification, that is what I was afraid of when > > > > looking at the code, but I was hoping I was just missing it. > > > > > > > > Increasing the default is an option, but I don't think we want to > > > > support a dynamic reallocation scheme for the xattr slots, that will > > > > likely get extremely messy with synchronization between the LSM > > > > framework and BPF LSM hook registrations as well as special code to > > > > handle inodes with lifetimes that are disjoint from the BPF LSM > > > > programs ... I suppose there may be a way to do it, but it will surely > > > > be ugly and come at a cost. > > > > > > BPF trampoline already handles all the synchronizations, such as > > > add hook, remove hook, etc. properly. So this is not that hard. > > > > How do you plan to handle the issue of disjoint lifetimes? > > > > > All we really need is to allocate a new array, copy pointers, and free > > > the old array. And we only really need this in the worst case > > > scenarios. > > > > Oh, is that all? :D > > > > Keep in mind that the code must also handle arbitrary ordering of > > LSMs; in other words, you must handle a BPF LSM that isn't at the end > > of the LSM order. While a BPF LSM at the end of the LSM list is the > > most common, and recommended ordering for the vast majority of users, > > we've been working to make the ordering as generalized as possible. > > All the BPF LSM hooks are called together, so it should be fine. > Maybe I missed some corner cases. I was thinking about the LSM framework as a whole, not just the potential for multiple BPF programs attached to the BPF LSM. > Either way, I agree with David that we don't need too many xattrs. > Since BPF LSM is reserved to the privileged users only, it is safe > to put a reasonable limit, say 4 or 8, and do not handle the realloc. > If the admin would like to brick a system with BPF LSM, there are > many other ways to do it. That is definitely the easier route. However, please add code to ensure the number of attached BPF programs is limited to the number of requested slots. -- paul-moore.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc [not found] <20260503211835.16103-1-dwindsor@gmail.com> 2026-05-03 21:18 ` [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling David Windsor @ 2026-05-03 21:18 ` David Windsor 2026-05-03 21:52 ` bot+bpf-ci 1 sibling, 1 reply; 12+ messages in thread From: David Windsor @ 2026-05-03 21:18 UTC (permalink / raw) To: Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov, Daniel Borkmann, Kumar Kartikeya Dwivedi, Shuah Khan Cc: Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, linux-kernel, bpf, linux-kselftest Test bpf atomic inode xattr labeling in inode_init_security. Signed-off-by: David Windsor <dwindsor@gmail.com> --- tools/testing/selftests/bpf/bpf_kfuncs.h | 5 ++ .../selftests/bpf/prog_tests/fs_kfuncs.c | 49 +++++++++++++++++++ .../bpf/progs/test_init_inode_xattr.c | 32 ++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/test_init_inode_xattr.c diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h index ae71e9b69051..5d67eb773e44 100644 --- a/tools/testing/selftests/bpf/bpf_kfuncs.h +++ b/tools/testing/selftests/bpf/bpf_kfuncs.h @@ -92,4 +92,9 @@ extern int bpf_set_dentry_xattr(struct dentry *dentry, const char *name__str, const struct bpf_dynptr *value_p, int flags) __ksym __weak; extern int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name__str) __ksym __weak; +struct lsm_xattr_ctx; +extern int bpf_init_inode_xattr(struct lsm_xattr_ctx *xattr_ctx, + const char *name__str, + const struct bpf_dynptr *value_p) __ksym __weak; + #endif diff --git a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c index 43a26ec69a8e..26daef116ee2 100644 --- a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c +++ b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c @@ -9,6 +9,7 @@ #include <test_progs.h> #include "test_get_xattr.skel.h" #include "test_set_remove_xattr.skel.h" +#include "test_init_inode_xattr.skel.h" #include "test_fsverity.skel.h" static const char testfile[] = "/tmp/test_progs_fs_kfuncs"; @@ -268,6 +269,51 @@ static void test_fsverity(void) remove(testfile); } +static void test_init_inode_xattr(void) +{ + struct test_init_inode_xattr *skel = NULL; + int fd = -1, err; + char value_out[32]; + const char *testfile_new = "/tmp/test_progs_fs_kfuncs_new"; + + skel = test_init_inode_xattr__open_and_load(); + if (!ASSERT_OK_PTR(skel, "test_init_inode_xattr__open_and_load")) + return; + + skel->bss->monitored_pid = getpid(); + err = test_init_inode_xattr__attach(skel); + if (!ASSERT_OK(err, "test_init_inode_xattr__attach")) + goto out; + + /* Create a new file — this triggers inode_init_security */ + fd = open(testfile_new, O_CREAT | O_RDWR, 0644); + if (!ASSERT_GE(fd, 0, "create_file")) + goto out; + + ASSERT_EQ(skel->data->init_result, 0, "init_result"); + + /* The initxattrs callback prepends "security." to the name */ + err = getxattr(testfile_new, "security.bpf.test_label", value_out, + sizeof(value_out)); + if (err < 0 && errno == ENODATA) { + printf("%s:SKIP:filesystem did not apply LSM xattrs\n", + __func__); + test__skip(); + goto out; + } + if (!ASSERT_GE(err, 0, "getxattr")) + goto out; + + ASSERT_EQ(err, (int)sizeof(skel->data->xattr_value), "xattr_size"); + ASSERT_EQ(strncmp(value_out, "test_value", + sizeof("test_value")), 0, "xattr_value"); + +out: + close(fd); + test_init_inode_xattr__destroy(skel); + remove(testfile_new); +} + void test_fs_kfuncs(void) { /* Matches xattr_names in progs/test_get_xattr.c */ @@ -286,6 +332,9 @@ void test_fs_kfuncs(void) if (test__start_subtest("set_remove_xattr")) test_set_remove_xattr(); + if (test__start_subtest("init_inode_xattr")) + test_init_inode_xattr(); + if (test__start_subtest("fsverity")) test_fsverity(); } diff --git a/tools/testing/selftests/bpf/progs/test_init_inode_xattr.c b/tools/testing/selftests/bpf/progs/test_init_inode_xattr.c new file mode 100644 index 000000000000..5bc5b90a98f2 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_init_inode_xattr.c @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025 Isovalent, a Cisco company. */ + +#include "vmlinux.h" +#include <errno.h> +#include <bpf/bpf_tracing.h> +#include "bpf_kfuncs.h" + +char _license[] SEC("license") = "GPL"; + +__u32 monitored_pid; +int init_result = -1; + +const char xattr_name[] = "bpf.test_label"; +char xattr_value[] = "test_value"; + +SEC("lsm.s/inode_init_security") +int BPF_PROG(test_init_inode_xattr, struct inode *inode, struct inode *dir, + const struct qstr *qstr, struct lsm_xattr_ctx *xattr_ctx) +{ + struct bpf_dynptr value_ptr; + __u32 pid; + + pid = bpf_get_current_pid_tgid() >> 32; + if (pid != monitored_pid) + return 0; + + bpf_dynptr_from_mem(xattr_value, sizeof(xattr_value), 0, &value_ptr); + init_result = bpf_init_inode_xattr(xattr_ctx, xattr_name, &value_ptr); + + return 0; +} -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc 2026-05-03 21:18 ` [PATCH v2 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc David Windsor @ 2026-05-03 21:52 ` bot+bpf-ci 0 siblings, 0 replies; 12+ messages in thread From: bot+bpf-ci @ 2026-05-03 21:52 UTC (permalink / raw) To: dwindsor, andrii, eddyz87, ast, daniel, memxor, shuah Cc: martin.lau, song, yonghong.song, jolsa, linux-kernel, bpf, linux-kselftest, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 3563 bytes --] > diff --git a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c > --- a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c > +++ b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c > @@ -9,6 +9,7 @@ > #include <test_progs.h> > #include "test_get_xattr.skel.h" > #include "test_set_remove_xattr.skel.h" > +#include "test_init_inode_xattr.skel.h" > #include "test_fsverity.skel.h" > > static const char testfile[] = "/tmp/test_progs_fs_kfuncs"; > @@ -268,6 +269,51 @@ struct fsverity_enable_arg arg = {0}; > remove(testfile); > } > > +static void test_init_inode_xattr(void) > +{ > + struct test_init_inode_xattr *skel = NULL; > + int fd = -1, err; > + char value_out[32]; Could value_out be read while uninitialized? If getxattr() succeeds but returns fewer bytes than expected, the size ASSERT_EQ check will fail but execution will continue. The subsequent strncmp() might then read uninitialized stack memory. Would it be better to zero-initialize this buffer, or use if (!ASSERT_EQ(...)) to abort the test early? > + const char *testfile_new = "/tmp/test_progs_fs_kfuncs_new"; > + > + skel = test_init_inode_xattr__open_and_load(); > + if (!ASSERT_OK_PTR(skel, "test_init_inode_xattr__open_and_load")) > + return; > + > + skel->bss->monitored_pid = getpid(); > + err = test_init_inode_xattr__attach(skel); > + if (!ASSERT_OK(err, "test_init_inode_xattr__attach")) > + goto out; > + > + /* Create a new file — this triggers inode_init_security */ > + fd = open(testfile_new, O_CREAT | O_RDWR, 0644); If the test file already exists from a previous failed run, will open() just open the existing file without triggering the inode_init_security hook? Since the hook only fires when a new inode is instantiated, this would cause the test to persistently fail. Also, since this runs as root in the world-writable /tmp directory, could this predictable path be vulnerable to a symlink attack? Would adding O_EXCL to the open flags ensure both that the hook is triggered and prevent symlink issues? > + if (!ASSERT_GE(fd, 0, "create_file")) > + goto out; > + > + ASSERT_EQ(skel->data->init_result, 0, "init_result"); > + > + /* The initxattrs callback prepends "security." to the name */ > + err = getxattr(testfile_new, "security.bpf.test_label", value_out, > + sizeof(value_out)); > + if (err < 0 && errno == ENODATA) { If the underlying /tmp filesystem does not support extended attributes at all, getxattr() returns -1 with errno set to EOPNOTSUPP (or ENOTSUP). Should this condition also be handled to correctly skip the test instead of failing on the next ASSERT_GE check? > + printf("%s:SKIP:filesystem did not apply LSM xattrs\n", > + __func__); > + test__skip(); > + goto out; > + } > + if (!ASSERT_GE(err, 0, "getxattr")) > + goto out; > + > + ASSERT_EQ(err, (int)sizeof(skel->data->xattr_value), "xattr_size"); > + ASSERT_EQ(strncmp(value_out, "test_value", > + sizeof("test_value")), 0, "xattr_value"); > + > +out: > + close(fd); If an error occurs before the file is opened (e.g. during skeleton load or attach), fd will be -1. Does this result in an unnecessary close(-1) which returns -EBADF? Should this be guarded with if (fd >= 0)? > + test_init_inode_xattr__destroy(skel); > + remove(testfile_new); > +} --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25291381237 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-05-05 13:49 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260503211835.16103-1-dwindsor@gmail.com>
2026-05-03 21:18 ` [PATCH v2 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling David Windsor
2026-05-04 20:14 ` Paul Moore
2026-05-04 21:40 ` Song Liu
2026-05-04 22:42 ` Paul Moore
2026-05-04 23:09 ` Song Liu
2026-05-05 1:07 ` David Windsor
2026-05-05 2:02 ` Paul Moore
2026-05-05 2:05 ` Paul Moore
2026-05-05 9:00 ` Song Liu
2026-05-05 13:49 ` Paul Moore
2026-05-03 21:18 ` [PATCH v2 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc David Windsor
2026-05-03 21:52 ` bot+bpf-ci
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox