Re: [PATCH bpf-next 1/8] ocfs2: Copy the xattr name in ocfs2_initxattrs

[Heming Zhao <heming.zhao@suse.com>]

On Wed, Sep 16, 2026 at 09:07:13AM +0200, Daniel Borkmann wrote:
> On 9/16/26 4:47 AM, Heming Zhao wrote:
> > On Tue, Sep 15, 2026 at 05:07:32PM +0200, Daniel Borkmann wrote:
> > > ocfs2_mknod() and ocfs2_symlink() ask for the security xattr up front
> > > through a struct ocfs2_security_xattr_info, so that they can size the
> > > transaction before setting it. ocfs2_initxattrs() duplicates the value
> > > since the array security_inode_init_security() hands is freed on return,
> > > but keeps the name pointer as-is, given so far every LSM stored a string
> > > constant there. bpf_inode_init_xattr() places the name in the same
> > 
> > Typo? I am not familiar with BPF, but I only found inode_init_security in
> > include/linux/lsm_hook_defs.h.
> 
> Its part of the series here; should have added "upcoming" in front:
> 
>   https://lore.kernel.org/bpf/20260915150739.284189-1-daniel@iogearbox.net/
> 
> > > allocation as the value, which security_inode_init_security() frees on
> > > its way out. Copy the name alongside the value and free both together.
> > > This is the only special case wrt xattrs in the bpf_inode_init_xattr()
> > > context.
> > > 
> > > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> > > Cc: Zhan Xusheng <zhanxusheng@xiaomi.com>
> > > Cc: Joseph Qi <joseph.qi@linux.alibaba.com>
> > > Cc: ocfs2-devel@lists.linux.dev
> > > ---
> > >   fs/ocfs2/namei.c | 2 ++
> > >   fs/ocfs2/xattr.c | 5 +++--
> > >   2 files changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> > > index e9c7774ccf91..e24f0e337a56 100644
> > > --- a/fs/ocfs2/namei.c
> > > +++ b/fs/ocfs2/namei.c
> > > @@ -480,6 +480,7 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
> > >   	brelse(new_fe_bh);
> > >   	brelse(parent_fe_bh);
> > > +	kfree(si.name);
> > >   	kfree(si.value);
> > >   	ocfs2_acl_init_release(&acl_state);
> > > @@ -2068,6 +2069,7 @@ static int ocfs2_symlink(struct mnt_idmap *idmap,
> > >   	brelse(new_fe_bh);
> > >   	brelse(parent_fe_bh);
> > > +	kfree(si.name);
> > >   	kfree(si.value);
> > >   	ocfs2_free_dir_lookup_result(&lookup);
> > >   	if (inode_ac)
> > > diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> > > index 35bcbb0ff607..d83840b6bed9 100644
> > > --- a/fs/ocfs2/xattr.c
> > > +++ b/fs/ocfs2/xattr.c
> > > @@ -7524,8 +7524,9 @@ static int ocfs2_initxattrs(struct inode *inode, const struct xattr *xattr_array
> > >   				    GFP_KERNEL);
> > >   		if (!si->value)
> > >   			return -ENOMEM;
> > > -
> > > -		si->name = xattr_array->name;
> > > +		si->name = kstrdup(xattr_array->name, GFP_KERNEL);
> > > +		if (!si->name)
> > > +			return -ENOMEM;
> > 
> > The ->name is a constant string (i.e., XATTR_NAME_SELINUX). We can refer to
> > ext4_initxattrs() => ext4_xattr_set_handle(), which also uses '=' to assign ->name.
> ext4 is not affected since nothing is used outside security_inode_init_security
> callback. ocfs2 is the only case in the tree affected, see this sashiko report
> which this small patch is addressing:
> 
>   https://lore.kernel.org/bpf/CAEXv5_gTJcP5BkSysZujLxiUcjbbBNi_NNbAnOJsQQVLyO9HoQ@mail.gmail.com/
> 
> Thanks,
> Daniel

Got it, the patch looks good to me.

Thanks,
Heming