Re: [patch][regression after 3.1] minixfs: misplaced checks lead todentry leak

From: Josh Boyer
Date: Wed Jan 04 2012 - 07:20:41 EST


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@xxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxx>

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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/