On Jul 7, 2026 David Windsor <[email protected]> wrote:
>
> In preparation for bpf_init_inode_xattr(), a kfunc that lets bpf LSM
> programs atomically label new inodes, rework how inode_init_security
> xattrs are managed.
>
> inode_init_security receives the LSM xattr array and its count as
> separate parameters. For better compatibility with the bpf verifier,
> update inode_init_security and its callers to consolidate these
> parameters into a single context object: struct lsm_xattrs.
>
> Also, add security_lsmxattr_add(), which claims a slot in the
> inode_init_security xattr array on behalf of the calling LSM and
> fills it with a copy of the given name and value.
>
> Suggested-by: Paul Moore <[email protected]>
> Signed-off-by: David Windsor <[email protected]>
> ---
> 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 | 15 +++++
> security/bpf/hooks.c | 1 +
> security/integrity/evm/evm_main.c | 8 ++-
> security/security.c | 108 ++++++++++++++++++++++++++----
> security/selinux/hooks.c | 4 +-
> security/smack/smack_lsm.c | 27 ++++----
> 10 files changed, 148 insertions(+), 47 deletions(-)
...
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 153e9043058f..647f7b88358b 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_xattrs {
> + struct xattr *xattrs;
> + unsigned int xattr_count;
> +};
Please separate out the 'struct lsm_xattrs' related changes into a
separate patch from the security_lsmxattr_add() changes. I know they
are related, but they are different things.
> diff --git a/security/security.c b/security/security.c
> index 71aea8fdf014..261f68e17cfd 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -12,6 +12,7 @@
> #define pr_fmt(fmt) "LSM: " fmt
>
> #include <linux/bpf.h>
> +#include <linux/bpf_lsm.h>
> #include <linux/capability.h>
> #include <linux/dcache.h>
> #include <linux/export.h>
> @@ -1333,8 +1334,8 @@ int security_inode_init_security(struct inode *inode,
> struct inode *dir,
> const initxattrs initxattrs, void *fs_data)
> {
> struct lsm_static_call *scall;
> - struct xattr *new_xattrs = NULL;
> - int ret = -EOPNOTSUPP, xattr_count = 0;
> + struct lsm_xattrs xattrs = {};
> + int ret = -EOPNOTSUPP;
>
> if (unlikely(IS_PRIVATE(inode)))
> return 0;
> @@ -1344,15 +1345,15 @@ int security_inode_init_security(struct inode *inode,
> struct inode *dir,
>
> if (initxattrs) {
> /* Allocate +1 as terminator. */
> - new_xattrs = kcalloc(blob_sizes.lbs_xattr_count + 1,
> - sizeof(*new_xattrs), GFP_NOFS);
> - if (!new_xattrs)
> + xattrs.xattrs = kcalloc(blob_sizes.lbs_xattr_count + 1,
> + sizeof(*xattrs.xattrs), GFP_NOFS);
> + if (!xattrs.xattrs)
> return -ENOMEM;
> }
>
> 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,
> + &xattrs);
> if (ret && ret != -EOPNOTSUPP)
> goto out;
> /*
> @@ -1364,18 +1365,101 @@ int security_inode_init_security(struct inode
> *inode, struct inode *dir,
> }
>
> /* If initxattrs() is NULL, xattr_count is zero, skip the call. */
> - if (!xattr_count)
> + if (!xattrs.xattr_count)
> goto out;
>
> - ret = initxattrs(inode, new_xattrs, fs_data);
> + ret = initxattrs(inode, xattrs.xattrs, fs_data);
> out:
> - for (; xattr_count > 0; xattr_count--)
> - kfree(new_xattrs[xattr_count - 1].value);
> - kfree(new_xattrs);
> + for (; xattrs.xattr_count > 0; xattrs.xattr_count--)
> + kfree(xattrs.xattrs[xattrs.xattr_count - 1].value);
> + kfree(xattrs.xattrs);
> return (ret == -EOPNOTSUPP) ? 0 : ret;
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
> +#ifdef CONFIG_BPF_LSM
> +static unsigned int lsm_xattrs_used(const struct lsm_xattrs *xattrs,
> + const char *prefix)
> +{
> + size_t prefix_len = strlen(prefix);
> + unsigned int i, n = 0;
> +
> + for (i = 0; i < xattrs->xattr_count; i++) {
> + const char *name = xattrs->xattrs[i].name;
> +
> + if (name && !strncmp(name, prefix, prefix_len))
> + n++;
> + }
> + return n;
> +}
> +#endif /* CONFIG_BPF_LSM */
More on this below, but this function isn't strictly BPF LSM related so
let's drop the CONFIG_BPF_LSM macro bracketing.
> +/**
> + * security_lsmxattr_add() - Add an xattr during inode_init_security
> + * @xattrs: xattr state shared by inode_init_security hooks
> + * @lsm_id: LSM_ID_* value identifying the calling LSM
> + * @name: xattr name suffix
For BPF you currently pass "bpf.foo" for the name and other LSMs would
pass their own xattr name, likely without a LSM specific sub-namespace
(for example, SELinux would be just "selinux"). In both cases the
'name' parameter always starts with a well known suffix as defined by
the 'lsm_id" parameter.
Since we are already passing the lsm_id parameter, let's do away with
the standard LSM suffixes, e.g. XATTR_BPF_LSM_SUFFIX, and just pass in
any additional name components. For example, instead of passing
"bpf.foo" in the BPF LSM case, you would just pass "foo"; LSMs without
their own sub-namespace, e.g. SELinux, would pass NULL for the name
parameter as XATTR_SELINUX_SUFFIX is all that is needed. While doing
this I would also suggest changing the name of the 'name' parameter to
'name_extra', 'namespace_extra', or something similar to indicate that
it isn't the full name, but rather an additional suffix beyond the
standard suffix associated with the given LSM.
> + * @value: xattr value
> + * @value_len: length of @value
> + *
> + * Claim an xattr slot in @xattrs on behalf of the LSM identified by
> + * @lsm_id and fill it with a copy of @name and @value. Callers can invoke
> + * this function from non-sleepable context.
> + *
> + * Return: Returns 0 on success, -ENOSPC if the calling LSM's slot budget
> + * is exhausted, negative values on other errors.
> + */
> +int security_lsmxattr_add(struct lsm_xattrs *xattrs, u64 lsm_id,
> + const char *name, const void *value,
> + size_t value_len)
> +{
> + struct xattr *xattr;
> + void *xattr_value;
> + size_t name_len;
> +
> + if (!xattrs || !xattrs->xattrs || !name || !value)
> + return -EINVAL;
Sashiko raised a good point about xattrs->xattrs being NULL not
necessarily being a good reason for -EINVAL. If xattrs is NULL, yes,
something has gone wrong and -EINVAL seems reasonable, but the
xattr->xattrs NULL case does seem like it should simply return early
with a value of 0 (see SELinux's handling of this case as an example).
> + name_len = strlen(name);
> + if (name_len == 0 || name_len > XATTR_NAME_MAX)
> + return -EINVAL;
> + if (value_len == 0 || value_len > XATTR_SIZE_MAX)
> + return -EINVAL;
> +
> + switch (lsm_id) {
> +#ifdef CONFIG_BPF_LSM
> + case LSM_ID_BPF:
> + if (lsm_xattrs_used(xattrs, XATTR_BPF_LSM_SUFFIX) >=
> + BPF_LSM_INODE_INIT_XATTRS)
> + return -ENOSPC;
> + break;
> +#endif /* CONFIG_BPF_LSM */
I like to avoid macro conditional code inside functions whenever
possible, and I think this is a case where we could avoid the conditional
block with a little work.
The LSM_ID_BPF macro is already defined as part of the UAPI so that will
always be available. While BPF_LSM_INODE_INIT_XATTRS is dependent on
CONFIG_BPF_LSM in this revision, that should be easy enough to move
outside the CONFIG_BPF_LSM conditional in bpf_lsm.h. Eventually we
should probably expand the lsm_id struct to carry this info, likely just
a permanent/local copy of the LSM's lsm_blob_sizes passed during
registration, but I can take care of that later; just make the
BPF_LSM_INODE_INIT_XATTRS macro always accessible now.
We should also probably record the xattr suffix/length when the LSM is
registered, but that can also be done later with the other lsm_id
additions.
> + default:
> + return -EINVAL;
> + }
> +
> + /* Combine xattr value + name into one allocation. */
> + xattr_value = kmalloc(value_len + name_len + 1, GFP_NOWAIT);
> + if (!xattr_value)
> + return -ENOMEM;
> +
> + memcpy(xattr_value, value, value_len);
> + memcpy(xattr_value + value_len, name, name_len);
> + ((char *)xattr_value)[value_len + name_len] = '\0';
You'll need to add an additional memcpy() here, likely in a switch
statement to handle the different LSMs, to copy over the LSM specific
prefix. It's a little ugly, but when we have things captured in the
lsm_id struct it will get a lot cleaner.
> + xattr = lsm_get_xattr_slot(xattrs);
> + 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;
> +}
--
paul-moore.com
