[PATCH] btrfs: fix double-free 'tree_root' in 'btrfs_mount()'

2011-11-07 Thread slyich
From: Sergei Trofimovich sly...@gentoo.org

On error path 'tree_root' is treed in 'free_fs_info()'.
No need to free it explicitely. Noticed by SLUB in debug mode:

Complete reproducer under usermode linux (discovered on real
machine):

bdev=/dev/ubda
btr_root=/btr
/mkfs.btrfs $bdev
mount $bdev $btr_root
mkdir $btr_root/subvols/
cd $btr_root/subvols/
/btrfs su cr foo
/btrfs su cr bar
mount $bdev -osubvol=subvols/foo $btr_root/subvols/bar
umount $btr_root/subvols/bar

which gives

device fsid 4d55aa28-45b1-474b-b4ec-da912322195e devid 1 transid 7 /dev/ubda
=
BUG kmalloc-2048: Object already free
-

INFO: Allocated in btrfs_mount+0x389/0x7f0 age=0 cpu=0 pid=277
INFO: Freed in btrfs_mount+0x51c/0x7f0 age=0 cpu=0 pid=277
INFO: Slab 0x62886200 objects=15 used=9 fp=0x70b4d2d0 
flags=0x4081
INFO: Object 0x70b4d2d0 @offset=21200 fp=0x70b4a968
...
Call Trace:
70b31948:  [6008c522] print_trailer+0xe2/0x130
70b31978:  [6008c5aa] object_err+0x3a/0x50
70b319a8:  [6008e242] free_debug_processing+0x142/0x2a0
70b319e0:  [600ebf6f] btrfs_mount+0x55f/0x7f0
70b319f8:  [6008e5c1] __slab_free+0x221/0x2d0

Signed-off-by: Sergei Trofimovich sly...@gentoo.org
Cc: Arne Jansen sensi...@gmx.net
Cc: Chris Mason chris.ma...@oracle.com
Cc: David Sterba dste...@suse.cz
---
 fs/btrfs/super.c |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 57080df..dcd5aef 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -933,8 +933,12 @@ static struct dentry *btrfs_mount(struct file_system_type 
*fs_type, int flags,
 * then open_ctree will properly initialize everything later.
 */
fs_info = kzalloc(sizeof(struct btrfs_fs_info), GFP_NOFS);
+   if (!fs_info) {
+   error = -ENOMEM;
+   goto error_close_devices;
+   }
tree_root = kzalloc(sizeof(struct btrfs_root), GFP_NOFS);
-   if (!fs_info || !tree_root) {
+   if (!tree_root) {
error = -ENOMEM;
goto error_close_devices;
}
@@ -964,7 +968,6 @@ static struct dentry *btrfs_mount(struct file_system_type 
*fs_type, int flags,
 
btrfs_close_devices(fs_devices);
free_fs_info(fs_info);
-   kfree(tree_root);
} else {
char b[BDEVNAME_SIZE];
 
@@ -992,7 +995,6 @@ static struct dentry *btrfs_mount(struct file_system_type 
*fs_type, int flags,
 error_close_devices:
btrfs_close_devices(fs_devices);
free_fs_info(fs_info);
-   kfree(tree_root);
return ERR_PTR(error);
 }
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] btrfs: fix double-free 'tree_root' in 'btrfs_mount()'

2011-11-07 Thread Christoph Hellwig
On Mon, Nov 07, 2011 at 12:12:07PM +0300, sly...@gmail.com wrote:
 bdev=/dev/ubda
 btr_root=/btr
 /mkfs.btrfs $bdev
 mount $bdev $btr_root
 mkdir $btr_root/subvols/
 cd $btr_root/subvols/
 /btrfs su cr foo
 /btrfs su cr bar
 mount $bdev -osubvol=subvols/foo $btr_root/subvols/bar
 umount $btr_root/subvols/bar

Can you please send this as a new testcase for xfstests?

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] btrfs: fix double-free 'tree_root' in 'btrfs_mount()'

2011-11-07 Thread Sergei Trofimovich
  bdev=/dev/ubda
  btr_root=/btr
  /mkfs.btrfs $bdev
  mount $bdev $btr_root
  mkdir $btr_root/subvols/
  cd $btr_root/subvols/
  /btrfs su cr foo
  /btrfs su cr bar
  mount $bdev -osubvol=subvols/foo $btr_root/subvols/bar
  umount $btr_root/subvols/bar
 
 Can you please send this as a new testcase for xfstests?

I'll try to. Will take some time though (never seen it before).
Thanks!

-- 

  Sergei


signature.asc
Description: PGP signature


Re: [PATCH] btrfs: fix double-free 'tree_root' in 'btrfs_mount()'

2011-11-07 Thread Chris Mason
On Mon, Nov 07, 2011 at 12:12:07PM +0300, sly...@gmail.com wrote:
 From: Sergei Trofimovich sly...@gentoo.org
 
 On error path 'tree_root' is treed in 'free_fs_info()'.
 No need to free it explicitely. Noticed by SLUB in debug mode:

Nice, thanks for the patch.

-chris
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html