linux-kernel - Re: [PATCH 5.15] fuse: Fix race condition in writethrough path A race

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: <aO5XvcuhEpw6BmiV@bfoster>
Date: Tue, 14 Oct 2025 10:01:33 -0400
From: Brian Foster <bfoster@...hat.com>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: lu gu <giveme.gulu@...il.com>, Joanne Koong <joannelkoong@...il.com>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	Bernd Schubert <bernd@...ernd.com>
Subject: Re: [PATCH 5.15] fuse: Fix race condition in writethrough path A race

On Tue, Oct 14, 2025 at 09:48:34AM +0200, Miklos Szeredi wrote:
> On Mon, 13 Oct 2025 at 20:49, Brian Foster <bfoster@...hat.com> wrote:
> 
> > Hrm Ok. But even if we did miss remote changes, whose to say we can even
> > resolve that correctly from the kernel anyways..?
> 
> No, I'm worrying about the case of
> 
> - range1 cached locally,
> - range1 changed remotely (mtime changed)
> - range2 changed locally (mtime changed, cached mtime invalidated)
> - range1 read locally
> 
> That last one will update mtime in cache, see that old cached mtime is
> stale and happily read the stale data.
> 
> What we currently have is more correct in the sense that it will
> invalidate data on any mtime change, be it of local or remote origin.
> 

Yeah, I guess. IMO if you made it policy that this sort of thing is
userspace responsibility, then something like the above is not
necessarily incorrect in fuse, it's more a failure of userspace.

I'd guess the challenge is more how to manage the change in behavior.
Maybe that would need an opt-in flag or something for userspace to
signify it understands it is responsible for external changes, and then
have a notify call or whatever that can tie into cache truncation
(where'd you'd explicitly punch out cache even if dirty).

But TBH, if the writeback thing or something similarly simple works for
resolving the immediate bug, I wouldnt worry too much about it
until/unless there are userspace fs' explicitly looking for that sort of
behavior. Just my .02.

Brian

> > > Yes, reproducer has auto_inval_data turned on (libfuse turns it on by default).
> > >
> >
> > I was more wondering if the problem goes away if it were disabled..
> 
> I haven't tried, @guangming?
> 
> > Ah, yeah that makes sense. Though invalidate waits on writeback. Any
> > reason this path couldn't skip the dirty state but mark the pages as
> > under writeback across the op?
> 
> Maybe that'd work.  It *is* under writeback after all.
> 
> Maybe the solution is to change the write-through to regular cached
> write + fsync range?  That could even be a complexity reduction.
> 
> Thanks,
> Miklos
>