2012-05-29 23:36:05

by Salman Qazi

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

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


2012-05-29 23:36:12

by Salman Qazi

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

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 <[email protected]>
---
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;
}


2012-05-29 23:36:16

by Salman Qazi

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

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 <[email protected]>
---
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;
}


2012-06-01 04:09:25

by Theodore Ts'o

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

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 <[email protected]>

Applied, thanks.

- Ted

2012-06-01 04:09:35

by Theodore Ts'o

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

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 <[email protected]>

Applied, thanks.

- Ted