* [patch][regression after 3.1] minixfs: misplaced checks lead to dentry leak
@ 2012-01-04 10:51 Al Viro
2012-01-04 12:20 ` Josh Boyer
0 siblings, 1 reply; 2+ messages in thread
From: Al Viro @ 2012-01-04 10:51 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, Josh Boyer
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@zeniv.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.
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:
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [patch][regression after 3.1] minixfs: misplaced checks lead to dentry leak
2012-01-04 10:51 [patch][regression after 3.1] minixfs: misplaced checks lead to dentry leak Al Viro
@ 2012-01-04 12:20 ` Josh Boyer
0 siblings, 0 replies; 2+ messages in thread
From: Josh Boyer @ 2012-01-04 12:20 UTC (permalink / raw)
To: Al Viro; +Cc: Linus Torvalds, linux-kernel
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@zeniv.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@redhat.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:
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-01-04 12:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-04 10:51 [patch][regression after 3.1] minixfs: misplaced checks lead to dentry leak Al Viro
2012-01-04 12:20 ` Josh Boyer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.