Christoph Hellwig
2011-Nov-07 09:10 UTC
Re: [PATCH] btrfs: fix double-free ''tree_root'' in ''btrfs_mount()''
On Mon, Nov 07, 2011 at 12:12:07PM +0300, slyich@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/barCan 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
slyich@gmail.com
2011-Nov-07 09:12 UTC
[PATCH] btrfs: fix double-free ''tree_root'' in ''btrfs_mount()''
From: Sergei Trofimovich <slyfox@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 0x0000000062886200 objects=15 used=9 fp=0x0000000070b4d2d0
flags=0x4081
INFO: Object 0x0000000070b4d2d0 @offset=21200 fp=0x0000000070b4a968
...
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 <slyfox@gentoo.org>
Cc: Arne Jansen <sensille@gmx.net>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: David Sterba <dsterba@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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sergei Trofimovich
2011-Nov-07 09:37 UTC
Re: [PATCH] btrfs: fix double-free ''tree_root'' in ''btrfs_mount()''
> > 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
Chris Mason
2011-Nov-07 15:57 UTC
Re: [PATCH] btrfs: fix double-free ''tree_root'' in ''btrfs_mount()''
On Mon, Nov 07, 2011 at 12:12:07PM +0300, slyich@gmail.com wrote:> From: Sergei Trofimovich <slyfox@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html