lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 28 Apr 2021 16:51:58 +0800 From: Ye Bin <yebin10@...wei.com> To: <tytso@....edu>, <adilger.kernel@...ger.ca>, <jack@...e.cz>, <linux-ext4@...r.kernel.org>, <linux-kernel@...r.kernel.org> CC: Ye Bin <yebin10@...wei.com> Subject: [PATCH v3] ext4: Fix bug on in ext4_es_cache_extent as ext4_split_extent_at failed We got follow bug_on when run fsstress with injecting IO fault: [130747.323114] kernel BUG at fs/ext4/extents_status.c:762! [130747.323117] Internal error: Oops - BUG: 0 [#1] SMP ...... [130747.334329] Call trace: [130747.334553] ext4_es_cache_extent+0x150/0x168 [ext4] [130747.334975] ext4_cache_extents+0x64/0xe8 [ext4] [130747.335368] ext4_find_extent+0x300/0x330 [ext4] [130747.335759] ext4_ext_map_blocks+0x74/0x1178 [ext4] [130747.336179] ext4_map_blocks+0x2f4/0x5f0 [ext4] [130747.336567] ext4_mpage_readpages+0x4a8/0x7a8 [ext4] [130747.336995] ext4_readpage+0x54/0x100 [ext4] [130747.337359] generic_file_buffered_read+0x410/0xae8 [130747.337767] generic_file_read_iter+0x114/0x190 [130747.338152] ext4_file_read_iter+0x5c/0x140 [ext4] [130747.338556] __vfs_read+0x11c/0x188 [130747.338851] vfs_read+0x94/0x150 [130747.339110] ksys_read+0x74/0xf0 If call ext4_ext_insert_extent failed but new extent already inserted, we just update "ex->ee_len = orig_ex.ee_len", this will lead to extent overlap, then cause bug on when cache extent. If call ext4_ext_insert_extent failed don't update ex->ee_len with old value. Maybe there will lead to block leak, but it can be fixed by fsck later. After we fixed above issue with v2 patch, but we got the same issue. ext4_split_extent_at: { ...... err = ext4_ext_insert_extent(handle, inode, ppath, &newex, flags); if (err == -ENOSPC && (EXT4_EXT_MAY_ZEROOUT & split_flag)) { ...... ext4_ext_try_to_merge(handle, inode, path, ex); ->step(1) err = ext4_ext_dirty(handle, inode, path + path->p_depth); ->step(2) if (err) goto fix_extent_len; ...... } ...... fix_extent_len: ex->ee_len = orig_ex.ee_len; ->step(3) ...... } If step(1) have been merged, but step(2) dirty extent failed, then go to fix_extent_len label to fix ex->ee_len with orig_ex.ee_len. But "ex" may not be old one, will cause overwritten. Then will trigger the same issue as previous. If step(2) failed, just return error, don't fix ex->ee_len with old value. Signed-off-by: Ye Bin <yebin10@...wei.com> --- fs/ext4/extents.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 77c84d6f1af6..d4aa24a09d8b 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -3238,15 +3238,12 @@ static int ext4_split_extent_at(handle_t *handle, ex->ee_len = cpu_to_le16(ee_len); ext4_ext_try_to_merge(handle, inode, path, ex); err = ext4_ext_dirty(handle, inode, path + path->p_depth); - if (err) - goto fix_extent_len; - - /* update extent status tree */ - err = ext4_zeroout_es(inode, &zero_ex); - - goto out; - } else if (err) + if (!err) + /* update extent status tree */ + err = ext4_zeroout_es(inode, &zero_ex); + } else if (err && err != -EROFS) { goto fix_extent_len; + } out: ext4_ext_show_leaf(inode, path); -- 2.25.4
Powered by blists - more mailing lists