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: <CAJfpegs0eeBNstSc-bj3HYjzvH6T-G+sVra7Ln+U1sXCGYC5-Q@mail.gmail.com>
Date: Mon, 13 Oct 2025 20:23:39 +0200
From: Miklos Szeredi <miklos@...redi.hu>
To: Brian Foster <bfoster@...hat.com>
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 Mon, 13 Oct 2025 at 19:40, Brian Foster <bfoster@...hat.com> wrote:

> If I follow the report correctly, we're basically producing an internal
> inconsistency between mtime and cache state that falsely presents as a
> remote change, so one of these attr change checks can race with a write
> in progress and invalidate cache. Do I have that right?

Yes.

>
> But still a few questions..
>
> 1. Do we know where exactly the mtime update comes from? Is it the write
> in progress that updates the file mtime on the backend and creates the
> inconsistency?

It can be a previous write.  A write will set STATX_MTIME in
fi->inval_mask, indicating that the value cached in i_mtime is
invalid.  But the auto_inval code will ignore that and use  cached
mtime to compare against the new value.

We could skip data invalidation if the cached value of mtime is not
valid, but this could easily result in remote changes being missed.

>
> 2. Is it confirmed that auto_inval is the culprit here? It seems logical
> to me, but it can also be disabled dynamically so couldn't hurt to
> confirm that if there's a reproducer.

Yes, reproducer has auto_inval_data turned on (libfuse turns it on by default).

>
> 3. I don't think we should be able to invalidate "dirty" folios like
> this. On a quick look though, it seems we don't mark folios dirty in
> this write path. Is that right?

Correct.

>
> If so, I'm a little curious if that's more of a "no apparent need" thing
> since the writeback occurs right in that path vs. that is an actual
> wrong thing to do for some reason. Hm?

Good question.  I think it's wrong, since dirtying the pages would
allow the witeback code to pick them up, which would be messy.

> Agreed in general. IIUC, this is ultimately a heuristic that isn't
> guaranteed to necessarily get things right for the backing fs. ISTM that
> maybe fuse is trying too hard to handle the distributed case correctly
> where the backing fs should be the one to implement this sort of thing
> through exposed mechanisms. OTOH so long as the heuristic exists we
> should probably at least work to make it internally consistent.

Yes, that's my problem.  How can we fix this without adding too much
complexity and without breaking existing uses?

Thanks,
Miklos