linux-kernel - Re: [PATCH V2] ovl: fsync after metadata copy-up via mount option "fsync=strict"

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJfpegtPOgowkK5EHxNZnuHDo9AZTbF2-zxMc99rvWL44rdMXQ@mail.gmail.com>
Date: Mon, 26 Aug 2024 17:59:10 +0200
From: Miklos Szeredi <miklos@...redi.hu>
To: Amir Goldstein <amir73il@...il.com>
Cc: Fei Lv <feilv@...micro.com>, linux-unionfs@...r.kernel.org, 
	linux-kernel@...r.kernel.org, lianghuxu@...micro.com
Subject: Re: [PATCH V2] ovl: fsync after metadata copy-up via mount option "fsync=strict"

On Mon, 22 Jul 2024 at 15:56, Amir Goldstein <amir73il@...il.com> wrote:
>
> On Mon, Jul 22, 2024 at 1:14 PM Fei Lv <feilv@...micro.com> wrote:
> >
> > For upper filesystem which does not enforce ordering on storing of
> > metadata changes(e.g. ubifs), when overlayfs file is modified for
> > the first time, copy up will create a copy of the lower file and
> > its parent directories in the upper layer. Permission lost of the
> > new upper parent directory was observed during power-cut stress test.
> >
> > Fix by adding new mount opion "fsync=strict", make sure data/metadata of
> > copied up directory written to disk before renaming from tmp to final
> > destination.
> >
> > Signed-off-by: Fei Lv <feilv@...micro.com>
>
> Reviewed-by: Amir Goldstein <amir73il@...il.com>
>
> but I'd also like to wait for an ACK from Miklos on this feature.

I'm okay with this.  I'm a little confused about sync=strict mode,
since most copy ups will have vfs_fsync() called twice.  Is this what
we want, or could this be consolidated into a single fsync?

Also is it worth optimizing away the fsync on the directory in cases
the filesystem is well behaved?  Maybe we should just move the
vfs_fsync() call into ovl_copy_up_metadata() and omit the complexity
related to the additional mount option?

To me it feels that it shouldn't matter in terms of performance, but
if reports of performance regressions come in, we can still make this
optional.

Thanks,
Miklos