linux-kernel - Re: [PATCH 1/2] f2fs: fix changing cursegs if recovery fails on zoned device

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c1e90e9a-3b93-4caa-b28b-83a3380c8865@kernel.org>
Date: Mon, 18 Nov 2024 14:50:15 +0800
From: Chao Yu <chao@...nel.org>
To: Sheng Yong <shengyong@...o.com>, jaegeuk@...nel.org
Cc: Chao Yu <chao@...nel.org>, linux-f2fs-devel@...ts.sourceforge.net,
 linux-kernel@...r.kernel.org, Song Feng <songfeng@...o.com>,
 Yongpeng Yang <yangyongpeng1@...o.com>
Subject: Re: [PATCH 1/2] f2fs: fix changing cursegs if recovery fails on zoned
 device

On 2024/11/11 16:50, Sheng Yong wrote:
> Fsync data recovery attempts to check and fix write pointer consistency
> of cursegs and all other zones. If the write pointers of cursegs are
> unaligned, cursegs are changed to new sections.
> 
> If recovery fails, zone write pointers are still checked and fixed,
> but the latest checkpoint cannot be written back. Additionally, retry-
> mount skips recovery and rolls back to reuse the old cursegs whose
> zones are already finished. This can lead to unaligned write later.
> 
> This patch addresses the issue by leaving writer pointers untouched if
> recovery fails. When retry-mount is performed, cursegs and other zones
> are checked and fixed after skipping recovery.
> 
> Signed-off-by: Song Feng <songfeng@...o.com>
> Signed-off-by: Yongpeng Yang <yangyongpeng1@...o.com>
> Signed-off-by: Sheng Yong <shengyong@...o.com>
> ---
>   fs/f2fs/recovery.c |  2 +-
>   fs/f2fs/super.c    | 11 ++++++++---
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index e4d81b8705d1..324f948247ae 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -899,7 +899,7 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
>   	 * and the f2fs is not read only, check and fix zoned block devices'
>   	 * write pointer consistency.
>   	 */
> -	if (f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sbi->sb)) {
> +	if (!err && f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sbi->sb)) {
>   		int err2 = f2fs_fix_curseg_write_pointer(sbi);
>   
>   		if (!err2)
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 87ab5696bd48..42224c71ae20 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4738,13 +4738,18 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>   reset_checkpoint:
>   	/*
>   	 * If the f2fs is not readonly and fsync data recovery succeeds,
> -	 * check zoned block devices' write pointer consistency.
> +	 * write pointer consistency of cursegs and other zones are already
> +	 * checked and fixed during recovery. However, if recovery fails,
> +	 * write pointers are left untouched, and retry-mount should check
> +	 * them here.
>   	 */
> -	if (f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sb)) {
> +	if (skip_recovery && f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sb)) {
>   		int err2;
>   
>   		f2fs_notice(sbi, "Checking entire write pointers");
> -		err2 = f2fs_check_write_pointer(sbi);
> +		err2 = f2fs_fix_curseg_write_pointer(sbi);
> +		if (!err2)
> +			err2 = f2fs_check_write_pointer(sbi);
>   		if (err2)
>   			err = err2;

What about wrapping above logic into f2fs_fix_and_check_write_pointer() and reuse in
f2fs_fill_super() and f2fs_recover_fsync_data()?

int f2fs_fix_and_check_write_pointer()
{
	int err;

	if (!f2fs_sb_has_blkzoned(sbi) || f2fs_readonly(sbi->sb))
		return 0;

	err = f2fs_fix_curseg_write_pointer(sbi);
	if (!err)
		err = f2fs_check_write_pointer(sbi);

	return err;
}

Thanks,

>   	}