Re: [PATCH v6 bpf-next 2/4] security: add security_lsmxattr_add()
David Windsor <[email protected]> Thu, 30 Jul 2026 20:30:40 -0400
| Newsgroups | org.kernel.vger.selinux,dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| Message-ID | <CAEXv5_gTJcP5BkSysZujLxiUcjbbBNi_NNbAnOJsQQVLyO9HoQ@mail.gmail.com> |
On Thu, Jul 30, 2026 at 8:00=E2=80=AFPM <[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 =3D 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] =3D '\0'; > > + > > + xattr =3D lsm_get_xattr_slot(xattrs); > > + if (!xattr) { > > + kfree(xattr_value); > > + return -ENOSPC; > > + } > > + > > + xattr->value =3D xattr_value; > > + xattr->name =3D (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 =3D 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 =3D 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 =3D 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 =3D value_len; > > + > > + return 0; > > +} > > -- > Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260730234533.19= [email protected]?part=3D2