linux-kernel - Re: [RFC 3/6] fs: add automatic kernel fs freeze / thaw and remove kthread freezing

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: <827c1ff030dd3b208e7a14be63160703b67e7031.camel@HansenPartnership.com>
Date: Wed, 26 Mar 2025 07:53:10 -0400
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Luis Chamberlain <mcgrof@...nel.org>, jack@...e.cz, hch@...radead.org, 
 david@...morbit.com, rafael@...nel.org, djwong@...nel.org,
 pavel@...nel.org,  song@...nel.org
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	gost.dev@...sung.com
Subject: Re: [RFC 3/6] fs: add automatic kernel fs freeze / thaw and remove
 kthread freezing

On Wed, 2025-03-26 at 04:22 -0700, Luis Chamberlain wrote:
> Add support to automatically handle freezing and thawing filesystems
> during the kernel's suspend/resume cycle.
> 
> This is needed so that we properly really stop IO in flight without
> races after userspace has been frozen. Without this we rely on
> kthread freezing and its semantics are loose and error prone.
> For instance, even though a kthread may use try_to_freeze() and end
> up being frozen we have no way of being sure that everything that
> has been spawned asynchronously from it (such as timers) have also
> been stopped as well.
> 
> A long term advantage of also adding filesystem freeze / thawing
> supporting during suspend / hibernation is that long term we may
> be able to eventually drop the kernel's thread freezing completely
> as it was originally added to stop disk IO in flight as we hibernate
> or suspend.
> 
> This does not remove the superfluous freezer calls on all
> filesystems.
> Each filesystem must remove all the kthread freezer stuff and peg
> the fs_type flags as supporting auto-freezing with the FS_AUTOFREEZE
> flag.
> 
> Subsequent patches remove the kthread freezer usage from each
> filesystem, one at a time to make all this work bisectable.
> Once all filesystems remove the usage of the kthread freezer we
> can remove the FS_AUTOFREEZE flag.
> 
> Reviewed-by: Jan Kara <jack@...e.cz>
> Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
> ---
>  fs/super.c             | 50
> ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fs.h     | 14 ++++++++++++
>  kernel/power/process.c | 15 ++++++++++++-
>  3 files changed, 78 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 9995546cf159..7428f0b2251c 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -2279,3 +2279,53 @@ int sb_init_dio_done_wq(struct super_block
> *sb)
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(sb_init_dio_done_wq);
> +
> +#ifdef CONFIG_PM_SLEEP
> +static bool super_should_freeze(struct super_block *sb)
> +{
> +	if (!(sb->s_type->fs_flags & FS_AUTOFREEZE))
> +		return false;
> +	/*
> +	 * We don't freeze virtual filesystems, we skip those
> filesystems with
> +	 * no backing device.
> +	 */
> +	if (sb->s_bdi == &noop_backing_dev_info)
> +		return false;


This logic won't work for me because efivarfs is a pseudofilesystem and
will have a noop bdi (or simply a null s_bdev, which is easier to check
for).  I was thinking of allowing freeze/thaw to continue for a s_bdev
== NULL filesystem if it provided a freeze or thaw callback, which will
cover efivarfs.

> +
> +	return true;
> +}
> +
> +int fs_suspend_freeze_sb(struct super_block *sb, void *priv)
> +{
> +	int error = 0;
> +
> +	if (!super_should_freeze(sb))
> +		goto out;
> +
> +	pr_info("%s (%s): freezing\n", sb->s_type->name, sb->s_id);
> +
> +	error = freeze_super(sb, false);

This is actually not wholly correct now.  If the fs provides a sb-
>freeze() method, you should use that instead of freeze_super() ... see
how fs_bdev_freeze() is doing it.

Additionally, the first thing freeze_super() does is take the
superblock lock exclusively.  Since you've already taken it exclusively
in your iterate super, how does this not deadlock?

You also need to handle the hibernate deadlock I ran into where a
process (and some of the systemd processes are very fast at doing this)
touches the filesystem and gets blocked on uninterruptible wait before
the remainder of freeze_processes() runs.  Once a task is
uninterruptible hibernate fails.  I came up with a simplistic solution:

https://lore.kernel.org/linux-fsdevel/1af829aa7a65eb5ebc0614a00f7019615ed0f62b.camel@HansenPartnership.com/

But there should probably be a freezable percpu_rwsem that
sb_write_started() can use to get these semantics rather than making
every use of percpu_rwsem freezable.

Regards,

James