maillist inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IC8HLD CVE: NA Reference: https://lore.kernel.org/linux-xfs/20250523063046.564170-1-leo.lilong@huawei.... -------------------------------- When the AIL is empty, we track ail_head_lsn as the tail LSN, where ail_head_lsn records the commit LSN of Checkpoint N, the last checkpoint inserted into the AIL. There are two possible scenarios when the AIL is empty: 1. Items from Checkpoint N were previously inserted into the AIL, have been written back, and subsequently removed from the AIL. 2. Items from Checkpoint N have not yet been inserted into the AIL, but the preparation for insertion is complete, and ail_head_lsn has already been updated to Checkpoint N's commit LSN. For scenario 1, the items in Checkpoint N have already been written to the metadata area. Even in the event of a crash, Checkpoint N does not require recovery, so forwarding the tail LSN to Checkpoint N's commit LSN is reasonable. For scenario 2, the items in Checkpoint N have not been written to the metadata area. If new logs (ie., Checkpoint N+1) are flushed to disk with the tail LSN recorded as Checkpoint N's commit LSN, a crash would make it impossible to recover Checkpoint N. Checkpoint N start N commit N +-------------+------------+--------------+ Checkpoint N+1 start N+1 commit N+1 Scenario 2 is possible. I encountered this issue in the Linux 6.6, where l_last_sync_lsn was used to track the tail LSN when the AIL is empty. Although the code has been refactored and I have not reproduced this issue in the latest mainline kernel, I believe the problem still exists. In the function xlog_cil_ail_insert(), which inserts items from the ctx into the AIL, the update of ail_head_lsn and the actual insertion of items into the AIL are not atomic. This process is not protected by `ail_lock` or `log->l_icloglock`, leaving a window that could result in the iclog being filled with an incorrect tail LSN. When the AIL is empty, the tail LSN should be set to Checkpoint N's start LSN before the items from Checkpoint N are inserted into the AIL. This ensures that Checkpoint N can be recovered in case of a crash. After the items from Checkpoint N are inserted into the AIL, the tail LSN tracked for an empty AIL can then be updated to Checkpoint N's commit LSN. This cannot be achieved with ail_head_lsn alone, so a new variable, ail_tail_lsn, is introduced specifically to track the tail LSN when the AIL is empty. Fixes: 14e15f1bcd73 ("xfs: push the grant head when the log head moves forward") # further than this Signed-off-by: Long Li <leo.lilong@huawei.com> --- fs/xfs/xfs_log_cil.c | 2 ++ fs/xfs/xfs_log_recover.c | 3 +++ fs/xfs/xfs_trans_ail.c | 2 +- fs/xfs/xfs_trans_priv.h | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c index 4b8079af8901..3d36066215be 100644 --- a/fs/xfs/xfs_log_cil.c +++ b/fs/xfs/xfs_log_cil.c @@ -783,6 +783,7 @@ xlog_cil_ail_insert( xfs_trans_ail_cursor_last(ailp, &cur, ctx->start_lsn); old_head = ailp->ail_head_lsn; ailp->ail_head_lsn = ctx->commit_lsn; + ailp->ail_tail_lsn = ctx->start_lsn; /* xfs_ail_update_finish() drops the ail_lock */ xfs_ail_update_finish(ailp, NULLCOMMITLSN); @@ -865,6 +866,7 @@ xlog_cil_ail_insert( spin_lock(&ailp->ail_lock); xfs_trans_ail_cursor_done(&cur); + ailp->ail_tail_lsn = ctx->commit_lsn; spin_unlock(&ailp->ail_lock); } diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index e6a5fa03a530..05eff7c086de 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -1179,6 +1179,8 @@ xlog_check_unmount_rec( log->l_curr_cycle, after_umount_blk); log->l_ailp->ail_head_lsn = atomic64_read(&log->l_tail_lsn); + log->l_ailp->ail_tail_lsn = + atomic64_read(&log->l_tail_lsn); *tail_blk = after_umount_blk; *clean = true; @@ -1212,6 +1214,7 @@ xlog_set_state( if (bump_cycle) log->l_curr_cycle++; atomic64_set(&log->l_tail_lsn, be64_to_cpu(rhead->h_tail_lsn)); + log->l_ailp->ail_tail_lsn = be64_to_cpu(rhead->h_lsn); log->l_ailp->ail_head_lsn = be64_to_cpu(rhead->h_lsn); } diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c index 9ce88d96daa5..2542c5e2c1cd 100644 --- a/fs/xfs/xfs_trans_ail.c +++ b/fs/xfs/xfs_trans_ail.c @@ -745,7 +745,7 @@ __xfs_ail_assign_tail_lsn( tail_lsn = __xfs_ail_min_lsn(ailp); if (!tail_lsn) - tail_lsn = ailp->ail_head_lsn; + tail_lsn = ailp->ail_tail_lsn; WRITE_ONCE(log->l_tail_space, xlog_lsn_sub(log, ailp->ail_head_lsn, tail_lsn)); diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h index bd841df93021..89ad29036ccb 100644 --- a/fs/xfs/xfs_trans_priv.h +++ b/fs/xfs/xfs_trans_priv.h @@ -56,6 +56,7 @@ struct xfs_ail { spinlock_t ail_lock; xfs_lsn_t ail_last_pushed_lsn; xfs_lsn_t ail_head_lsn; + xfs_lsn_t ail_tail_lsn; int ail_log_flush; unsigned long ail_opstate; struct list_head ail_buf_list; -- 2.39.2