From: "Luis Henriques (SUSE)" <luis.henriques@linux.dev>
To: Theodore Ts'o <tytso@mit.edu>, Andreas Dilger <adilger@dilger.ca>,
Zhang Yi <yi.zhang@huaweicloud.com>,
Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc: linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org,
"Luis Henriques (SUSE)" <luis.henriques@linux.dev>
Subject: [PATCH] ext4: fix infinite loop when replaying fast_commit
Date: Fri, 10 May 2024 12:52:52 +0100 [thread overview]
Message-ID: <20240510115252.11850-1-luis.henriques@linux.dev> (raw)
When doing fast_commit replay an infinite loop may occur due to an
uninitialized extent_status struct. ext4_ext_determine_insert_hole() does
not detect the replay and calls ext4_es_find_extent_range(), which will
return immediately without initializing the 'es' variable.
Because 'es' contains garbage, an integer overflow may happen causing an
infinite loop in this function, easily reproducible using fstest generic/039.
This commit fixes this issue by detecting the replay in function
ext4_ext_determine_insert_hole(). It also adds initialization code to the
error path in function ext4_es_find_extent_range().
Thanks to Zhang Yi, for figuring out the real problem!
Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Signed-off-by: Luis Henriques (SUSE) <luis.henriques@linux.dev>
---
Hi!
Two comments:
1) The change in ext4_ext_map_blocks() could probably use the min_not_zero
macro instead. I decided not to do so simply because I wasn't sure if
that would be safe, but I'm fine changing that if you think it is.
2) I thought about returning 'EXT_MAX_BLOCKS' instead of '0' in
ext4_lblk_t ext4_ext_determine_insert_hole(), which would then avoid
the extra change to ext4_ext_map_blocks(). '0' sounds like the right
value to return, but I'm also OK using 'EXT_MAX_BLOCKS' instead.
And again thanks to Zhang Yi for pointing me the *real* problem!
fs/ext4/extents.c | 6 +++++-
fs/ext4/extents_status.c | 5 ++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index e57054bdc5fd..b5bfcb6c18a0 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4052,6 +4052,9 @@ static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
ext4_lblk_t hole_start, len;
struct extent_status es;
+ if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
+ return 0;
+
hole_start = lblk;
len = ext4_ext_find_hole(inode, path, &hole_start);
again:
@@ -4226,7 +4229,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
len = ext4_ext_determine_insert_hole(inode, path, map->m_lblk);
map->m_pblk = 0;
- map->m_len = min_t(unsigned int, map->m_len, len);
+ if (len > 0)
+ map->m_len = min_t(unsigned int, map->m_len, len);
goto out;
}
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 4a00e2f019d9..acb9616ca119 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -310,8 +310,11 @@ void ext4_es_find_extent_range(struct inode *inode,
ext4_lblk_t lblk, ext4_lblk_t end,
struct extent_status *es)
{
- if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
+ if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY) {
+ /* Initialize extent to zero */
+ es->es_lblk = es->es_len = es->es_pblk = 0;
return;
+ }
trace_ext4_es_find_extent_range_enter(inode, lblk);
next reply other threads:[~2024-05-10 11:52 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 11:52 Luis Henriques (SUSE) [this message]
2024-05-11 6:24 ` [PATCH] ext4: fix infinite loop when replaying fast_commit Zhang Yi
2024-05-14 13:04 ` Luis Henriques
2024-05-15 4:59 ` Zhang Yi
2024-05-15 8:28 ` Luis Henriques
2024-05-15 8:52 ` Zhang Yi
2024-05-15 9:13 ` Luis Henriques
2024-05-15 12:24 ` Zhang Yi
2024-05-12 16:44 ` Markus Elfring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240510115252.11850-1-luis.henriques@linux.dev \
--to=luis.henriques@linux.dev \
--cc=adilger@dilger.ca \
--cc=harshadshirwadkar@gmail.com \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tytso@mit.edu \
--cc=yi.zhang@huaweicloud.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.