* [PATCH] ext4: do not use extent after put_bh
@ 2020-10-28 5:56 yangerkun
2020-10-29 4:14 ` Theodore Y. Ts'o
2020-10-29 16:08 ` Ritesh Harjani
0 siblings, 2 replies; 4+ messages in thread
From: yangerkun @ 2020-10-28 5:56 UTC (permalink / raw)
To: tytso, jack, adilger; +Cc: linux-ext4, yangerkun
ext4_ext_search_right will read more extent block and call put_bh after
we get the information we need. However ret_ex will break this and may
cause use-after-free once pagecache has been freed. Fix it by dup the
extent we need.
Signed-off-by: yangerkun <yangerkun@huawei.com>
---
fs/ext4/extents.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 559100f3e23c..4ba8131a5629 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -1471,16 +1471,16 @@ static int ext4_ext_search_left(struct inode *inode,
}
/*
- * search the closest allocated block to the right for *logical
- * and returns it at @logical + it's physical address at @phys
- * if *logical is the largest allocated block, the function
- * returns 0 at @phys
- * return value contains 0 (success) or error code
+ * Search the closest allocated block to the right for *logical
+ * and returns it at @logical + it's physical address at @phys.
+ * If not exists, return 0 and @phys is set to 0. We will return
+ * 1 which means we found that and ret_ex may be valid. Or return
+ * the error code.
*/
static int ext4_ext_search_right(struct inode *inode,
struct ext4_ext_path *path,
ext4_lblk_t *logical, ext4_fsblk_t *phys,
- struct ext4_extent **ret_ex)
+ struct ext4_extent *ret_ex)
{
struct buffer_head *bh = NULL;
struct ext4_extent_header *eh;
@@ -1574,10 +1574,11 @@ static int ext4_ext_search_right(struct inode *inode,
found_extent:
*logical = le32_to_cpu(ex->ee_block);
*phys = ext4_ext_pblock(ex);
- *ret_ex = ex;
+ if (ret_ex)
+ *ret_ex = *ex;
if (bh)
put_bh(bh);
- return 0;
+ return 1;
}
/*
@@ -2868,8 +2869,8 @@ int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
*/
lblk = ex_end + 1;
err = ext4_ext_search_right(inode, path, &lblk, &pblk,
- &ex);
- if (err)
+ NULL);
+ if (err < 0)
goto out;
if (pblk) {
partial.pclu = EXT4_B2C(sbi, pblk);
@@ -4039,7 +4040,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
struct ext4_map_blocks *map, int flags)
{
struct ext4_ext_path *path = NULL;
- struct ext4_extent newex, *ex, *ex2;
+ struct ext4_extent newex, *ex, ex2;
struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
ext4_fsblk_t newblock = 0, pblk;
int err = 0, depth, ret;
@@ -4175,15 +4176,14 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
if (err)
goto out;
ar.lright = map->m_lblk;
- ex2 = NULL;
err = ext4_ext_search_right(inode, path, &ar.lright, &ar.pright, &ex2);
- if (err)
+ if (err < 0)
goto out;
/* Check if the extent after searching to the right implies a
* cluster we can use. */
- if ((sbi->s_cluster_ratio > 1) && ex2 &&
- get_implied_cluster_alloc(inode->i_sb, map, ex2, path)) {
+ if ((sbi->s_cluster_ratio > 1) && err &&
+ get_implied_cluster_alloc(inode->i_sb, map, &ex2, path)) {
ar.len = allocated = map->m_len;
newblock = map->m_pblk;
goto got_allocated_blocks;
--
2.25.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: do not use extent after put_bh
2020-10-28 5:56 [PATCH] ext4: do not use extent after put_bh yangerkun
@ 2020-10-29 4:14 ` Theodore Y. Ts'o
2020-10-29 16:08 ` Ritesh Harjani
1 sibling, 0 replies; 4+ messages in thread
From: Theodore Y. Ts'o @ 2020-10-29 4:14 UTC (permalink / raw)
To: yangerkun; +Cc: jack, adilger, linux-ext4
On Wed, Oct 28, 2020 at 01:56:17PM +0800, yangerkun wrote:
> ext4_ext_search_right will read more extent block and call put_bh after
> we get the information we need. However ret_ex will break this and may
> cause use-after-free once pagecache has been freed. Fix it by dup the
> extent we need.
>
> Signed-off-by: yangerkun <yangerkun@huawei.com>
Good catch!
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: do not use extent after put_bh
2020-10-28 5:56 [PATCH] ext4: do not use extent after put_bh yangerkun
2020-10-29 4:14 ` Theodore Y. Ts'o
@ 2020-10-29 16:08 ` Ritesh Harjani
2020-10-30 1:23 ` yangerkun
1 sibling, 1 reply; 4+ messages in thread
From: Ritesh Harjani @ 2020-10-29 16:08 UTC (permalink / raw)
To: yangerkun, adilger; +Cc: tytso, jack, linux-ext4
On 10/28/20 11:26 AM, yangerkun wrote:
> ext4_ext_search_right will read more extent block and call put_bh after
> we get the information we need. However ret_ex will break this and may
> cause use-after-free once pagecache has been freed. Fix it by dup the
> extent we need.
It would be good if we have a test case to reproduce it. Do you?
Ideally it should go in fstests, if you have some way to forcefully
reproduce it/simulate it. Let me know, if needed, I can as well help to
get those into fstests.
-ritesh
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: do not use extent after put_bh
2020-10-29 16:08 ` Ritesh Harjani
@ 2020-10-30 1:23 ` yangerkun
0 siblings, 0 replies; 4+ messages in thread
From: yangerkun @ 2020-10-30 1:23 UTC (permalink / raw)
To: Ritesh Harjani, adilger; +Cc: tytso, jack, linux-ext4
在 2020/10/30 0:08, Ritesh Harjani 写道:
>
>
> On 10/28/20 11:26 AM, yangerkun wrote:
>> ext4_ext_search_right will read more extent block and call put_bh after
>> we get the information we need. However ret_ex will break this and may
>> cause use-after-free once pagecache has been freed. Fix it by dup the
>> extent we need.
>
>
> It would be good if we have a test case to reproduce it. Do you?
> Ideally it should go in fstests, if you have some way to forcefully
> reproduce it/simulate it. Let me know, if needed, I can as well help to
> get those into fstests.
Sorry for that. I found this bug while reading source code. Not with a
testcase.
And time leave for drop pagecache is so small(time between
get_implied_cluster_alloc and ext4_ext_search_right in
ext4_ext_map_blocks, other caller for ext4_ext_search_right won't use
@ret_ex). It may difficult to reproduce it expect a delay injection.
Thanks,
Kun.
>
> -ritesh
> .
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-30 1:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-28 5:56 [PATCH] ext4: do not use extent after put_bh yangerkun
2020-10-29 4:14 ` Theodore Y. Ts'o
2020-10-29 16:08 ` Ritesh Harjani
2020-10-30 1:23 ` yangerkun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox