Re: [PATCH 02/12] btrfs: free the path earlier when creating a new inode

[Filipe Manana <fdmanana@kernel.org>]

On Wed, Jun 01, 2022 at 04:52:54AM +0530, Anand Jain wrote:
> On 5/31/22 20:36, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> > 
> > When creating an inode, through btrfs_create_new_inode(), we release the
> > path we allocated before once we don't need it anymore. But we keep it
> > allocated until we return from that function, which is wasteful because
> > after we release the path we do several things that can allocate yet
> > another path: inheriting properties, setting the xattrs used by ACLs and
> > secutiry modules, adding an orphan item (O_TMPFILE case) or adding a
> > dir item (for the non-O_TMPFILE case).
> > 
> > So instead of releasing the path once we don't need it anymore, free it
> > instead. This way we avoid having two paths allocated until we return
> > from btrfs_create_new_inode().
> > 
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >   fs/btrfs/inode.c | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 06d5bfa84d38..3ede3e873c2a 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -6380,7 +6380,13 @@ int btrfs_create_new_inode(struct btrfs_trans_handle *trans,
> >   	}
> >   	btrfs_mark_buffer_dirty(path->nodes[0]);
> > -	btrfs_release_path(path);
> > +	/*
> > +	 * We don't need the path anymore, plus inheriting properties, adding
> > +	 * ACLs, security xattrs, orphan item or adding the link, will result in
> > +	 * allocating yet another path. So just free our path.
> > +	 */
> > +	btrfs_free_path(path);
> > +	path = NULL;
> >   	if (args->subvol) {
> >   		struct inode *parent;
> 
> 
> 
> > @@ -6437,8 +6443,7 @@ int btrfs_create_new_inode(struct btrfs_trans_handle *trans,
> >   		goto discard;
> >   	}
> 
> At discard, we free path again and leads to double free.

No, there's no double free. The path was set to NULL after being freed.

> 
> Thanks, Anand
> 
> >  > -	ret = 0;
> > -	goto out;
> > +	return 0;
> >   discard:
> >   	/*
>