Re: [PATCH v5 bpf-next 1/3] security: rework inode_init_security xattr handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Heap buffer overflow in `security_lsmxattr_add()` due to xattr quota bypass by BPF programs.
- [Medium] `security_lsmxattr_add()` incorrectly returns `-EINVAL` instead of `-EOPNOTSUPP` when the xattr array is not allocated.
--

--- Patch [1]: [PATCH v5 bpf-next 1/3] security: rework inode_init_security xattr handling ---
commit 871b3020e8c69372708886992716ee08d2e36bc2
Author: David Windsor <dwindsor@gmail.com>

security: rework inode_init_security xattr handling

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.

> diff --git a/security/security.c b/security/security.c
> --- a/security/security.c
> +++ b/security/security.c

[ ... ]

> +/**
> + * 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
> + * @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;

[Severity: Medium]
Should this return -EOPNOTSUPP instead of -EINVAL when the xattrs array
is not allocated?

During file creation on filesystems without xattr support,
security_inode_init_security() is called with initxattrs == NULL,
deliberately leaving xattrs.xattrs unallocated.

If a BPF program then calls security_lsmxattr_add(), it will receive
-EINVAL. If the BPF program propagates this error, it will abort the
inode creation entirely because security_inode_init_security() only
treats -EOPNOTSUPP as a graceful fallback.

> +
> +	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;

[Severity: High]
Can this limit check be bypassed, leading to a heap buffer overflow?

If a BPF program repeatedly calls security_lsmxattr_add() with a name
that does not begin with XATTR_BPF_LSM_SUFFIX, lsm_xattrs_used() will
not count them.

This allows bypassing the BPF_LSM_INODE_INIT_XATTRS limit. Since
lsm_get_xattr_slot() does not check against the actual allocated heap
size, it will increment ctx->xattr_count and return out-of-bounds
pointers:

include/linux/lsm_hooks.h:lsm_get_xattr_slot() {
    ...
    return &ctx->xattrs[ctx->xattr_count++];
}

Could this cause out-of-bounds writes when assigning xattr->value and
xattr->name further down in this function?

> +		break;
> +#endif /* CONFIG_BPF_LSM */
> +	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;

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