Re: [PATCH v6 bpf-next 2/4] security: add security_lsmxattr_add()

On Thu, Jul 30, 2026 at 8:00 PM <[email protected]> wrote:
>
> > +int security_lsmxattr_add(struct lsm_xattrs *xattrs, u64 lsm_id,
> > +                       const char *name_extra, const void *value,
> > +                       size_t value_len)
> > +{
> [ ... ]
> > +     /* 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, suffix, suffix_len);
> > +     if (extra_len)
> > +             memcpy(xattr_value + value_len + suffix_len, name_extra,
> > +                    extra_len);
> > +     ((char *)xattr_value)[value_len + name_len] = '\0';
> > +
> > +     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;
>
> [Severity: High]
> Does this code cause a use-after-free for filesystems like ocfs2?
>
> Looking at ocfs2, the ocfs2_initxattrs() callback saves the xattr->name
> pointer directly into si->name, assuming it has a static lifetime (which
> was true for SELinux and Smack):
>
> fs/ocfs2/xattr.c:ocfs2_initxattrs() {
>     ...
>     si->name = xattr_array->name;
>     ...
> }
>
> But here xattr->name points inside the dynamically allocated xattr->value
> block. When security_inode_init_security() completes, it frees
> xattr->value:
>
> security/security.c:security_inode_init_security() {
>     ...
>     ret = initxattrs(inode, xattrs.xattrs, fs_data);
> out:
>     for (; xattrs.xattr_count > 0; xattrs.xattr_count--)
>         kfree(xattrs.xattrs[xattrs.xattr_count - 1].value);
>     ...
> }
>
> After this free, ocfs2_mknod() will call ocfs2_calc_xattr_init(), which
> dereferences the now-dangling si->name pointer:
>
> fs/ocfs2/xattr.c:ocfs2_calc_xattr_init() {
>     ...
>     s_size = ocfs2_xattr_entry_real_size(strlen(si->name),
>                                          si->value_len);
>     ...
> }
>
> Could the dynamically allocated name trigger a use-after-free here?
>

This is real. ocfs stashes the name pointer beyond the call; it's the
only fs who does so. We can special-case ocfs and be done with it.
Will do this in v7.

> > +     xattr->value_len = value_len;
> > +
> > +     return 0;
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2