linux-kernel - Re: [PATCH v2 2/4] fs: allow all writers to be frozen

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: <20250402-gutbesuchten-miniatur-7d8be831e3d0@brauner>
Date: Wed, 2 Apr 2025 18:13:56 +0200
From: Christian Brauner <brauner@...nel.org>
To: James Bottomley <James.Bottomley@...senpartnership.com>
Cc: jack@...e.cz, peterz@...radead.org, Ard Biesheuvel <ardb@...nel.org>, 
	linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org, mcgrof@...nel.org, 
	hch@...radead.org, david@...morbit.com, rafael@...nel.org, djwong@...nel.org, 
	pavel@...nel.org, mingo@...hat.com, will@...nel.org, boqun.feng@...il.com, 
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2 2/4] fs: allow all writers to be frozen

On Wed, Apr 02, 2025 at 12:03:24PM -0400, James Bottomley wrote:
> On Wed, 2025-04-02 at 17:32 +0200, Christian Brauner wrote:
> [...]
> > Jan, one more thought about freezability here. We know that there
> > will can be at least one process during hibernation that ends up
> > generating page faults and that's systemd-journald. When systemd-
> > sleep requests writing a hibernation image via /sys/power/ files it
> > will inevitably end up freezing systemd-journald and it may be
> > generating a page fault with ->mmap_lock held. systemd-journald is
> > now sleeping with SB_FREEZE_PAGEFAULT and TASK_FREEZABLE. We know
> > this can cause hibernation to fail. That part is fine. What isn't is
> > that we will very likely always trigger:
> > 
> > #ifdef CONFIG_LOCKDEP
> >         /*
> >          * It's dangerous to freeze with locks held; there be dragons
> > there.
> >          */
> >         if (!(state & __TASK_FREEZABLE_UNSAFE))
> >                 WARN_ON_ONCE(debug_locks && p->lockdep_depth);
> > #endif
> > 
> > with lockdep enabled.
> > 
> > So we really actually need percpu_rswem_read_freezable_unsafe(),
> > i.e., TASK_FREEZABLE_UNSAFE.
> 
> The sched people have pretty strong views about people not doing this,
> expressed in the comment in sched.h and commit f5d39b020809
> ("freezer,sched: Rewrite core freezer logic") where most of the _unsafe
> variants got removed with prejudice.
> 
> If we do get into this situation the worst that can happen is that
> another upper lock acquisition triggers a hibernate failure and we thaw
> everything, thus we can never truly deadlock, which is the fear, so

Yes, I know that it's harmless but we need to not generate misleading
lockdep splats when lockdep is turned on and confuse users.

> perhaps they might be OK with this.  Note that Rafael's solution to
> this was to disable lockdep around hibernate/suspend and resume, which
> is another possibility.

It can be done as a follow-up. I'm just saying it needs treatment.