[PATCH 0/2] ext4: Fix some crashes on umount

[PATCH 0/2] ext4: Fix some crashes on umount

[Salman Qazi]

The following series fixes two potential ways to get crashes
during umount;  The crash signature is:

[  189.910292] RIP: 0010:[<ffffffff8118a490>]  [<ffffffff8118a490>] clear_inode+0x60/0x70
.
.
.
[  189.910292]  [<ffffffff812342fe>] ext4_clear_inode+0x1e/0x80
[  189.910292]  [<ffffffff8121944e>] ext4_evict_inode+0x7e/0x4a0
[  189.910292]  [<ffffffff8118af5f>] evict+0xaf/0x1b0
[  189.910292]  [<ffffffff8118b163>] iput+0x103/0x210
[  189.910292]  [<ffffffff81248ddb>] ext4_mb_release+0x26b/0x3c0
[  189.910292]  [<ffffffff815479bd>] ? wait_for_completion+0x1d/0x20
[  189.910292]  [<ffffffff81233abb>] ext4_put_super+0x9b/0x350
[  189.910292]  [<ffffffff8118bc8f>] ? evict_inodes+0xbf/0x120
[  189.910292]  [<ffffffff81173f12>] generic_shutdown_super+0x62/0xf0
[  189.910292]  [<ffffffff81173fd0>] kill_block_super+0x30/0x80
[  189.910292]  [<ffffffff811741e5>] deactivate_locked_super+0x45/0x70
[  189.910292]  [<ffffffff8117444e>] deactivate_super+0x4e/0x70
[  189.910292]  [<ffffffff8118efa1>] mntput_no_expire+0xf1/0x140
[  189.910292]  [<ffffffff8118fd4e>] sys_umount+0x6e/0x380
[  189.910292]  [<ffffffff810750da>] ? sys32_stat64+0x1a/0x40
[  189.910292]  [<ffffffff81190070>] sys_oldumount+0x10/0x20
[  189.910292]  [<ffffffff81551edf>] sysenter_dispatch+0x7/0x1a

and happens because the buddy cache inode still has pages at the
time of umount.  One way for this to happen is for pages to leak
in an error path.  The second way, which we reproduced involves
a race with reading the mb_groups proc file.


---

Salman Qazi (2):
      ext4: Add ext4_mb_unload_buddy in the error path
      ext4: remove mb_groups before tearing buddy_cache


 fs/ext4/mballoc.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

-- 
Salman Qazi

[PATCH 1/2] ext4: Add ext4_mb_unload_buddy in the error path

[Salman Qazi]

ext4_free_blocks fails to pair an ext4_mb_load_buddy with a matching
ext4_mb_unload_buddy when it fails a memory allocation.

Signed-off-by: Salman Qazi <sqazi@google.com>
---
 fs/ext4/mballoc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 99ab428..5c315ab 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4636,6 +4636,7 @@ do_more:
 		 */
 		new_entry = kmem_cache_alloc(ext4_free_data_cachep, GFP_NOFS);
 		if (!new_entry) {
+			ext4_mb_unload_buddy(&e4b);
 			err = -ENOMEM;
 			goto error_return;
 		}

Re: [PATCH 1/2] ext4: Add ext4_mb_unload_buddy in the error path

[Ted Ts'o]

On Tue, May 29, 2012 at 04:36:09PM -0700, Salman Qazi wrote:
> ext4_free_blocks fails to pair an ext4_mb_load_buddy with a matching
> ext4_mb_unload_buddy when it fails a memory allocation.
> 
> Signed-off-by: Salman Qazi <sqazi@google.com>

Applied, thanks.

					- Ted

[PATCH 2/2] ext4: remove mb_groups before tearing buddy_cache

[Salman Qazi]

We can't have references held on pages in the s_buddy_cache while we are
trying to truncate its pages and put the inode.  All the pages must be
gone before we reach clear_inode.  This can only be gauranteed if we
can prevent new users from grabbing references to s_buddy_cache's pages.

The original bug can be reproduced and the bug fix can be verified by:

while true; do mount -t ext4 /dev/ram0 /export/hda3/ram0; \
	umount /export/hda3/ram0; done &

while true; do cat /proc/fs/ext4/ram0/mb_groups; done

Signed-off-by: Salman Qazi <sqazi@google.com>
---
 fs/ext4/mballoc.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 5c315ab..6b0a57e 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2517,6 +2517,9 @@ int ext4_mb_release(struct super_block *sb)
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct kmem_cache *cachep = get_groupinfo_cache(sb->s_blocksize_bits);
 
+	if (sbi->s_proc)
+		remove_proc_entry("mb_groups", sbi->s_proc);
+
 	if (sbi->s_group_info) {
 		for (i = 0; i < ngroups; i++) {
 			grinfo = ext4_get_group_info(sb, i);
@@ -2564,8 +2567,6 @@ int ext4_mb_release(struct super_block *sb)
 	}
 
 	free_percpu(sbi->s_locality_groups);
-	if (sbi->s_proc)
-		remove_proc_entry("mb_groups", sbi->s_proc);
 
 	return 0;
 }

Re: [PATCH 2/2] ext4: remove mb_groups before tearing buddy_cache

[Ted Ts'o]

On Tue, May 29, 2012 at 04:36:14PM -0700, Salman Qazi wrote:
> We can't have references held on pages in the s_buddy_cache while we are
> trying to truncate its pages and put the inode.  All the pages must be
> gone before we reach clear_inode.  This can only be gauranteed if we
> can prevent new users from grabbing references to s_buddy_cache's pages.
> 
> The original bug can be reproduced and the bug fix can be verified by:
> 
> while true; do mount -t ext4 /dev/ram0 /export/hda3/ram0; \
> 	umount /export/hda3/ram0; done &
> 
> while true; do cat /proc/fs/ext4/ram0/mb_groups; done
> 
> Signed-off-by: Salman Qazi <sqazi@google.com>

Applied, thanks.

						- Ted