On 2022/8/19 17:54, Jan Kara wrote:
On Wed 10-08-22 09:34:42, Ye Bin wrote:
As https://github.com/tytso/e2fsprogs/issues/120 describe tune2fs do not update
j_tail_sequence when do journal recovery. This maybe recover old journal record,
then will lead to file system corruption.
To avoid file system corruption in this case, if detect current transaction's
commit time earlier than previous transaction's commit time when do journal
scan, just return error.
Signed-off-by: Ye Bin <yebin10@xxxxxxxxxx>
Thanks for the patch! Let me see if I understand your concern right. You
are concerned about the following scenario:
1) Kernel uses the filesystem, there's a crash.
2) E2fsprogs replays the journal but fails to update sb->s_sequence in the
journal superblock.
3) Kernel mounts the fs again - however note that even if kernel skips
recovery, it does scan the journal jbd2_journal_skip_recovery() and
journal->j_transaction_sequence is set based on the last transaction found
in the journal.
So I don't think there is really possibility we will quickly reuse some
transaction IDs and thus possibility of corruption on replay? Am I missing
something?
Honza
The file system corruption I encountered was indeed because e2fsprogs
did not update
journal - > J_ transaction_ Sequence leads to replay the old transaction.
So I wonder whether the kernel should detect this kind of exception, at
least when there
is a file system corruption, there are clues to trace.
---
fs/jbd2/recovery.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index f548479615c6..f3def21a96a5 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -812,8 +812,17 @@ static int do_one_pass(journal_t *journal,
break;
}
}
- if (pass == PASS_SCAN)
+ if (pass == PASS_SCAN) {
+ if (commit_time < last_trans_commit_time) {
+ pr_err("JBD2: old journal record found "
+ "in transaction %u\n",
+ next_commit_ID);
+ err = -EFSBADCRC;
+ brelse(bh);
+ goto failed;
+ }
last_trans_commit_time = commit_time;
+ }
brelse(bh);
next_commit_ID++;
continue;
--
2.31.1
