linux-kernel - Re: [PATCH] md: prevent incoreect update of resync/recovery offset

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: <2eadd8d3-7705-4000-982e-cafd222977c1@molgen.mpg.de>
Date: Sat, 30 Aug 2025 11:41:01 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Li Nan <linan666@...weicloud.com>
Cc: song@...nel.org, yukuai3@...wei.com, linux-raid@...r.kernel.org,
 linux-kernel@...r.kernel.org, yangerkun@...wei.com, yi.zhang@...wei.com
Subject: Re: [PATCH] md: prevent incoreect update of resync/recovery offset

Dear Nan,


Thank you for your patch. I have some formal comments. In the 
summary/title: incor*r*ect.

Am 30.08.25 um 11:02 schrieb linan666@...weicloud.com:
> From: Li Nan <linan122@...wei.com>
> 
> In md_do_sync(), when md_sync_action returns ACTION_FROZEN, subsequent
> call to md_sync_position() will return MaxSector. This causes
> 'curr_resync' (and later 'recovery_offset') to be set to MaxSector too,
> which incorrectly signals that recovery/resync has completed even though
> disk data has not actually been updated.
> 
> To fix this issue, skip updating any offset values when the sync acion

ac*t*ion

> is either FROZEN or IDLE.

Maybe state that the same holds true for IDLE?

Should these two cases be handled differently in `md_sync_position()`. 
Does it semantically make sense, that the default of MaxSector is returned?

> Fixes: 7d9f107a4e94 ("md: use new helpers in md_do_sync()")
> Signed-off-by: Li Nan <linan122@...wei.com>
> ---
>   drivers/md/md.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index e78f80d39271..6828a569e819 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9397,6 +9397,9 @@ void md_do_sync(struct md_thread *thread)
>   	}
>   
>   	action = md_sync_action(mddev);
> +	if (action == ACTION_FROZEN || action == ACTION_IDLE)
> +		goto skip;
> +
>   	desc = md_sync_action_name(action);
>   	mddev->last_sync_action = action;
>   


Kind regards,

Paul