[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240822023545.1994557-9-libaokun@huaweicloud.com>
Date: Thu, 22 Aug 2024 10:35:28 +0800
From: libaokun@...weicloud.com
To: linux-ext4@...r.kernel.org
Cc: tytso@....edu,
adilger.kernel@...ger.ca,
jack@...e.cz,
ritesh.list@...il.com,
ojaswin@...ux.ibm.com,
linux-kernel@...r.kernel.org,
yi.zhang@...wei.com,
yangerkun@...wei.com,
libaokun@...weicloud.com,
Baokun Li <libaokun1@...wei.com>,
stable@...nel.org
Subject: [PATCH v2 08/25] ext4: fix double brelse() the buffer of the extents path
From: Baokun Li <libaokun1@...wei.com>
In ext4_ext_try_to_merge_up(), set path[1].p_bh to NULL after it has been
released, otherwise it may be released twice. An example of what triggers
this is as follows:
split2 map split1
|--------|-------|--------|
ext4_ext_map_blocks
ext4_ext_handle_unwritten_extents
ext4_split_convert_extents
// path->p_depth == 0
ext4_split_extent
// 1. do split1
ext4_split_extent_at
|ext4_ext_insert_extent
| ext4_ext_create_new_leaf
| ext4_ext_grow_indepth
| le16_add_cpu(&neh->eh_depth, 1)
| ext4_find_extent
| // return -ENOMEM
|// get error and try zeroout
|path = ext4_find_extent
| path->p_depth = 1
|ext4_ext_try_to_merge
| ext4_ext_try_to_merge_up
| path->p_depth = 0
| brelse(path[1].p_bh) ---> not set to NULL here
|// zeroout success
// 2. update path
ext4_find_extent
// 3. do split2
ext4_split_extent_at
ext4_ext_insert_extent
ext4_ext_create_new_leaf
ext4_ext_grow_indepth
le16_add_cpu(&neh->eh_depth, 1)
ext4_find_extent
path[0].p_bh = NULL;
path->p_depth = 1
read_extent_tree_block ---> return err
// path[1].p_bh is still the old value
ext4_free_ext_path
ext4_ext_drop_refs
// path->p_depth == 1
brelse(path[1].p_bh) ---> brelse a buffer twice
Finally got the following WARRNING when removing the buffer from lru:
============================================
VFS: brelse: Trying to free free buffer
WARNING: CPU: 2 PID: 72 at fs/buffer.c:1241 __brelse+0x58/0x90
CPU: 2 PID: 72 Comm: kworker/u19:1 Not tainted 6.9.0-dirty #716
RIP: 0010:__brelse+0x58/0x90
Call Trace:
<TASK>
__find_get_block+0x6e7/0x810
bdev_getblk+0x2b/0x480
__ext4_get_inode_loc+0x48a/0x1240
ext4_get_inode_loc+0xb2/0x150
ext4_reserve_inode_write+0xb7/0x230
__ext4_mark_inode_dirty+0x144/0x6a0
ext4_ext_insert_extent+0x9c8/0x3230
ext4_ext_map_blocks+0xf45/0x2dc0
ext4_map_blocks+0x724/0x1700
ext4_do_writepages+0x12d6/0x2a70
[...]
============================================
Fixes: ecb94f5fdf4b ("ext4: collapse a single extent tree block into the inode if possible")
Cc: stable@...nel.org
Signed-off-by: Baokun Li <libaokun1@...wei.com>
Reviewed-by: Jan Kara <jack@...e.cz>
Reviewed-by: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
Tested-by: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
---
fs/ext4/extents.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index cd2e71123a25..c4a4573440c1 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -1890,6 +1890,7 @@ static void ext4_ext_try_to_merge_up(handle_t *handle,
path[0].p_hdr->eh_max = cpu_to_le16(max_root);
brelse(path[1].p_bh);
+ path[1].p_bh = NULL;
ext4_free_blocks(handle, inode, NULL, blk, 1,
EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
}
--
2.39.2
Powered by blists - more mailing lists