linux-kernel - Re: [patch][regression after 3.1] minixfs: misplaced checks lead to dentry leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20120104122009.GA21276@zod.bos.redhat.com>
Date:	Wed, 4 Jan 2012 07:20:09 -0500
From:	Josh Boyer <jwboyer@...hat.com>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [patch][regression after 3.1] minixfs: misplaced checks lead to
 dentry leak

On Wed, Jan 04, 2012 at 10:51:03AM +0000, Al Viro wrote:
> bitmap size sanity checks should be done *before* allocating ->s_root;
> there their cleanup on failure would be correct.  As it is, we do iput()
> on root inode, but leak the root dentry...
> 
> Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
> 
> I realize that we are *very* late in this cycle, but this is (a) obvious and
> obviously affecting only minixfs and (b) introduced in this cycle.

Ugh.  Yes.  Good catch.

Acked-by: Josh Boyer <jwboyer@...hat.com>

josh

> 
> diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> index c811c19..8e4f5d8 100644
> --- a/fs/minix/inode.c
> +++ b/fs/minix/inode.c
> @@ -262,23 +262,6 @@ static int minix_fill_super(struct super_block *s, void *data, int silent)
>  		goto out_no_root;
>  	}
>  
> -	ret = -ENOMEM;
> -	s->s_root = d_alloc_root(root_inode);
> -	if (!s->s_root)
> -		goto out_iput;
> -
> -	if (!(s->s_flags & MS_RDONLY)) {
> -		if (sbi->s_version != MINIX_V3) /* s_state is now out from V3 sb */
> -			ms->s_state &= ~MINIX_VALID_FS;
> -		mark_buffer_dirty(bh);
> -	}
> -	if (!(sbi->s_mount_state & MINIX_VALID_FS))
> -		printk("MINIX-fs: mounting unchecked file system, "
> -			"running fsck is recommended\n");
> - 	else if (sbi->s_mount_state & MINIX_ERROR_FS)
> -		printk("MINIX-fs: mounting file system with errors, "
> -			"running fsck is recommended\n");
> -
>  	/* Apparently minix can create filesystems that allocate more blocks for
>  	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
>  	 * create one with not enough blocks and bail out if so.
> @@ -299,6 +282,23 @@ static int minix_fill_super(struct super_block *s, void *data, int silent)
>  		goto out_iput;
>  	}
>  
> +	ret = -ENOMEM;
> +	s->s_root = d_alloc_root(root_inode);
> +	if (!s->s_root)
> +		goto out_iput;
> +
> +	if (!(s->s_flags & MS_RDONLY)) {
> +		if (sbi->s_version != MINIX_V3) /* s_state is now out from V3 sb */
> +			ms->s_state &= ~MINIX_VALID_FS;
> +		mark_buffer_dirty(bh);
> +	}
> +	if (!(sbi->s_mount_state & MINIX_VALID_FS))
> +		printk("MINIX-fs: mounting unchecked file system, "
> +			"running fsck is recommended\n");
> + 	else if (sbi->s_mount_state & MINIX_ERROR_FS)
> +		printk("MINIX-fs: mounting file system with errors, "
> +			"running fsck is recommended\n");
> +
>  	return 0;
>  
>  out_iput:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/