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

On Tue, Jul 7, 2026 at 8:20 PM <[email protected]> wrote:
>
> 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 <[email protected]>
>
> 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.
>

Sorry, all paths to calling this function are guaranteed to have
XATTR_BPF_LSM_SUFFIX.

> 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/[email protected]?part=1