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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241206150427.fdqnme4kqpt3ohdl@quack3> Date: Fri, 6 Dec 2024 16:04:27 +0100 From: Jan Kara <jack@...e.cz> To: Zhang Yi <yi.zhang@...weicloud.com> Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz, yi.zhang@...wei.com, chengzhihao1@...wei.com, yukuai3@...wei.com, yangerkun@...wei.com Subject: Re: [PATCH] jbd2: add a missing data flush during file and fs synchronization On Fri 06-12-24 19:13:27, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@...wei.com> > > When the filesystem performs file or filesystem synchronization (e.g., > ext4_sync_file()), it queries the journal to determine whether to flush > the file device through jbd2_trans_will_send_data_barrier(). If the > target transaction has not started committing, it assumes that the > journal will submit the flush command, allowing the filesystem to bypass > a redundant flush command. However, this assumption is not always valid. > If the journal is not located on the filesystem device, the journal > commit thread will not submit the flush command unless the variable > ->t_need_data_flush is set to 1. Consequently, the flush may be missed, > and data may be lost following a power failure or system crash, even if > the synchronization appears to succeed. > > Unfortunately, we cannot determine with certainty whether the target > transaction will flush to the filesystem device before it commits. > However, if it has not started committing, it must be the running > transaction. Therefore, fix it by always set its t_need_data_flush to 1, > ensuring that the committing thread will flush the filesystem device. > > Fixes: bbd2be369107 ("jbd2: Add function jbd2_trans_will_send_data_barrier()") > Signed-off-by: Zhang Yi <yi.zhang@...wei.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@...e.cz> Honza > --- > fs/jbd2/journal.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c > index 97f487c3d8fc..37632ae18a4e 100644 > --- a/fs/jbd2/journal.c > +++ b/fs/jbd2/journal.c > @@ -609,7 +609,7 @@ int jbd2_journal_start_commit(journal_t *journal, tid_t *ptid) > int jbd2_trans_will_send_data_barrier(journal_t *journal, tid_t tid) > { > int ret = 0; > - transaction_t *commit_trans; > + transaction_t *commit_trans, *running_trans; > > if (!(journal->j_flags & JBD2_BARRIER)) > return 0; > @@ -619,6 +619,16 @@ int jbd2_trans_will_send_data_barrier(journal_t *journal, tid_t tid) > goto out; > commit_trans = journal->j_committing_transaction; > if (!commit_trans || commit_trans->t_tid != tid) { > + running_trans = journal->j_running_transaction; > + /* > + * The query transaction hasn't started committing, > + * it must still be running. > + */ > + if (WARN_ON_ONCE(!running_trans || > + running_trans->t_tid != tid)) > + goto out; > + > + running_trans->t_need_data_flush = 1; > ret = 1; > goto out; > } > -- > 2.46.1 > -- Jan Kara <jack@...e.com> SUSE Labs, CR
Powered by blists - more mailing lists