xfs
[Top] [All Lists]

Re: [PATCH] xfs: don't leak root inode reference

To: Ben Myers <bpm@xxxxxxx>
Subject: Re: [PATCH] xfs: don't leak root inode reference
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 27 Aug 2013 07:24:23 +1000
Cc: xfs@xxxxxxxxxxx, Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130826204730.GZ7153@xxxxxxx>
References: <20130826204730.GZ7153@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Aug 26, 2013 at 03:47:30PM -0500, Ben Myers wrote:
> Looks like in 48fde701 we removed the iput of the root inode in
> xfs_fs_fill_super for the error case.  Add it back.
> 
> Signed-off-by: Ben Myers <bpm@xxxxxxx>
> 
> ---
>  fs/xfs/xfs_super.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: b/fs/xfs/xfs_super.c
> ===================================================================
> --- a/fs/xfs/xfs_super.c      2013-08-26 15:36:09.170848579 -0500
> +++ b/fs/xfs/xfs_super.c      2013-08-26 15:40:19.450817933 -0500
> @@ -1493,12 +1493,12 @@ xfs_fs_fill_super(
>       }
>       if (is_bad_inode(root)) {
>               error = EINVAL;
> -             goto out_unmount;
> +             goto out_iput;
>       }
>       sb->s_root = d_make_root(root);
>       if (!sb->s_root) {
>               error = ENOMEM;
> -             goto out_unmount;
> +             goto out_iput;
>       }

That's wrong. d_make_root() drops the reference to the inode on
failure itself, and so the change in 48fde701 is correct and valid.

The leak on bad inodes (which, AFAICT, can never happen on XFS) has
been around a lot longer than Al's change - this commit introduced
it:

        2bcf6e9 xfs: start periodic workers later

with this hunk:

        if (is_bad_inode(root)) {
                error = EINVAL;
-               goto fail_vnrele;
+               goto out_syncd_stop;
        }

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>