mirror of
https://github.com/torvalds/linux.git
synced 2024-11-10 14:11:52 +00:00
ext2: fix filesystem deadlock while reading corrupted xattr block
This bug can be reproducible with fsfuzzer, although, I couldn't reproduce it 100% of my tries, it is quite easily reproducible. During the deletion of an inode, ext2_xattr_delete_inode() does not check if the block pointed by EXT2_I(inode)->i_file_acl is a valid data block, this might lead to a deadlock, when i_file_acl == 1, and the filesystem block size is 1024. In that situation, ext2_xattr_delete_inode, will load the superblock's buffer head (instead of a valid i_file_acl block), and then lock that buffer head, which, ext2_sync_super will also try to lock, making the filesystem deadlock in the following stack trace: root 17180 0.0 0.0 113660 660 pts/0 D+ 07:08 0:00 rmdir /media/test/dir1 [<ffffffff8125da9f>] __sync_dirty_buffer+0xaf/0x100 [<ffffffff8125db03>] sync_dirty_buffer+0x13/0x20 [<ffffffffa03f0d57>] ext2_sync_super+0xb7/0xc0 [ext2] [<ffffffffa03f10b9>] ext2_error+0x119/0x130 [ext2] [<ffffffffa03e9d93>] ext2_free_blocks+0x83/0x350 [ext2] [<ffffffffa03f3d03>] ext2_xattr_delete_inode+0x173/0x190 [ext2] [<ffffffffa03ee9e9>] ext2_evict_inode+0xc9/0x130 [ext2] [<ffffffff8123fd23>] evict+0xb3/0x180 [<ffffffff81240008>] iput+0x1b8/0x240 [<ffffffff8123c4ac>] d_delete+0x11c/0x150 [<ffffffff8122fa7e>] vfs_rmdir+0xfe/0x120 [<ffffffff812340ee>] do_rmdir+0x17e/0x1f0 [<ffffffff81234dd6>] SyS_rmdir+0x16/0x20 [<ffffffff81838cf2>] entry_SYSCALL_64_fastpath+0x1a/0xa4 [<ffffffffffffffff>] 0xffffffffffffffff Fix this by using the same approach ext4 uses to test data blocks validity, implementing ext2_data_block_valid. An another possibility when the superblock is very corrupted, is that i_file_acl is 1, block_count is 1 and first_data_block is 0. For such situations, we might have i_file_acl pointing to a 'valid' block, but still step over the superblock. The approach I used was to also test if the superblock is not in the range described by ext2_data_block_valid() arguments Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
This commit is contained in:
parent
079788d01e
commit
ff0031d848
@ -1193,6 +1193,27 @@ static int ext2_has_free_blocks(struct ext2_sb_info *sbi)
|
||||
return 1;
|
||||
}
|
||||
|
||||
/*
|
||||
* Returns 1 if the passed-in block region is valid; 0 if some part overlaps
|
||||
* with filesystem metadata blocksi.
|
||||
*/
|
||||
int ext2_data_block_valid(struct ext2_sb_info *sbi, ext2_fsblk_t start_blk,
|
||||
unsigned int count)
|
||||
{
|
||||
if ((start_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
|
||||
(start_blk + count < start_blk) ||
|
||||
(start_blk > le32_to_cpu(sbi->s_es->s_blocks_count)))
|
||||
return 0;
|
||||
|
||||
/* Ensure we do not step over superblock */
|
||||
if ((start_blk <= sbi->s_sb_block) &&
|
||||
(start_blk + count >= sbi->s_sb_block))
|
||||
return 0;
|
||||
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
||||
/*
|
||||
* ext2_new_blocks() -- core block(s) allocation function
|
||||
* @inode: file inode
|
||||
|
@ -367,6 +367,7 @@ struct ext2_inode {
|
||||
*/
|
||||
#define EXT2_VALID_FS 0x0001 /* Unmounted cleanly */
|
||||
#define EXT2_ERROR_FS 0x0002 /* Errors detected */
|
||||
#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
|
||||
|
||||
/*
|
||||
* Mount flags
|
||||
@ -739,6 +740,8 @@ extern unsigned long ext2_bg_num_gdb(struct super_block *sb, int group);
|
||||
extern ext2_fsblk_t ext2_new_block(struct inode *, unsigned long, int *);
|
||||
extern ext2_fsblk_t ext2_new_blocks(struct inode *, unsigned long,
|
||||
unsigned long *, int *);
|
||||
extern int ext2_data_block_valid(struct ext2_sb_info *sbi, ext2_fsblk_t start_blk,
|
||||
unsigned int count);
|
||||
extern void ext2_free_blocks (struct inode *, unsigned long,
|
||||
unsigned long);
|
||||
extern unsigned long ext2_count_free_blocks (struct super_block *);
|
||||
|
@ -1389,6 +1389,16 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
|
||||
ei->i_frag_size = raw_inode->i_fsize;
|
||||
ei->i_file_acl = le32_to_cpu(raw_inode->i_file_acl);
|
||||
ei->i_dir_acl = 0;
|
||||
|
||||
if (ei->i_file_acl &&
|
||||
!ext2_data_block_valid(EXT2_SB(sb), ei->i_file_acl, 1)) {
|
||||
ext2_error(sb, "ext2_iget", "bad extended attribute block %u",
|
||||
ei->i_file_acl);
|
||||
brelse(bh);
|
||||
ret = -EFSCORRUPTED;
|
||||
goto bad_inode;
|
||||
}
|
||||
|
||||
if (S_ISREG(inode->i_mode))
|
||||
inode->i_size |= ((__u64)le32_to_cpu(raw_inode->i_size_high)) << 32;
|
||||
else
|
||||
|
@ -759,10 +759,19 @@ void
|
||||
ext2_xattr_delete_inode(struct inode *inode)
|
||||
{
|
||||
struct buffer_head *bh = NULL;
|
||||
struct ext2_sb_info *sbi = EXT2_SB(inode->i_sb);
|
||||
|
||||
down_write(&EXT2_I(inode)->xattr_sem);
|
||||
if (!EXT2_I(inode)->i_file_acl)
|
||||
goto cleanup;
|
||||
|
||||
if (!ext2_data_block_valid(sbi, EXT2_I(inode)->i_file_acl, 0)) {
|
||||
ext2_error(inode->i_sb, "ext2_xattr_delete_inode",
|
||||
"inode %ld: xattr block %d is out of data blocks range",
|
||||
inode->i_ino, EXT2_I(inode)->i_file_acl);
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
bh = sb_bread(inode->i_sb, EXT2_I(inode)->i_file_acl);
|
||||
if (!bh) {
|
||||
ext2_error(inode->i_sb, "ext2_xattr_delete_inode",
|
||||
|
Loading…
Reference in New Issue
Block a user