linux-ext4 - Re: [PATCH] fs: i_version mntopt gets visible through /proc/mounts

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: <20200619024455.GN2005@dread.disaster.area>
Date:   Fri, 19 Jun 2020 12:44:55 +1000
From:   Dave Chinner <david@...morbit.com>
To:     "J. Bruce Fields" <bfields@...ldses.org>
Cc:     Masayoshi Mizuma <msys.mizuma@...il.com>,
        Eric Sandeen <sandeen@...deen.net>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Christoph Hellwig <hch@...radead.org>,
        Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Masayoshi Mizuma <m.mizuma@...fujitsu.com>,
        linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-xfs <linux-xfs@...r.kernel.org>, jlayton@...hat.com
Subject: Re: [PATCH] fs: i_version mntopt gets visible through /proc/mounts

On Thu, Jun 18, 2020 at 10:20:05PM -0400, J. Bruce Fields wrote:
> On Fri, Jun 19, 2020 at 08:39:48AM +1000, Dave Chinner wrote:
> > On Wed, Jun 17, 2020 at 11:45:35PM -0400, Masayoshi Mizuma wrote:
> > > Thank you for pointed it out.
> > > How about following change? I believe it works both xfs and btrfs...
> > > 
> > > diff --git a/fs/super.c b/fs/super.c
> > > index b0a511bef4a0..42fc6334d384 100644
> > > --- a/fs/super.c
> > > +++ b/fs/super.c
> > > @@ -973,6 +973,9 @@ int reconfigure_super(struct fs_context *fc)
> > >                 }
> > >         }
> > > 
> > > +       if (sb->s_flags & SB_I_VERSION)
> > > +               fc->sb_flags |= MS_I_VERSION;
> > > +
> > >         WRITE_ONCE(sb->s_flags, ((sb->s_flags & ~fc->sb_flags_mask) |
> > >                                  (fc->sb_flags & fc->sb_flags_mask)));
> > >         /* Needs to be ordered wrt mnt_is_readonly() */
> > 
> > This will prevent SB_I_VERSION from being turned off at all. That
> > will break existing filesystems that allow SB_I_VERSION to be turned
> > off on remount, such as ext4.
> > 
> > The manipulations here need to be in the filesystem specific code;
> > we screwed this one up so badly there is no "one size fits all"
> > behaviour that we can implement in the generic code...
> 
> My memory was that after Jeff Layton's i_version patches, there wasn't
> really a significant performance hit any more, so the ability to turn it
> off is no longer useful.

Yes, I completely agree with you here. However, with some
filesystems allowing it to be turned off, we can't just wave our
hands and force enable the option. Those filesystems - if the
maintainers chose to always enable iversion - will have to go
through a mount option deprecation period before permanently
enabling it.

> But looking back through Jeff's postings, I don't see him claiming that;
> e.g. in:
> 
> 	https://lore.kernel.org/lkml/20171222120556.7435-1-jlayton@kernel.org/
> 	https://lore.kernel.org/linux-nfs/20180109141059.25929-1-jlayton@kernel.org/
> 	https://lore.kernel.org/linux-nfs/1517228795.5965.24.camel@redhat.com/
> 
> he reports comparing old iversion behavior to new iversion behavior, but
> not new iversion behavior to new noiversion behavior.

Yeah, it's had to compare noiversion behaviour on filesystems where
it was understood that it couldn't actually be turned off. And,
realistically, the comaprison to noiversion wasn't really relevant
to the problem Jeff's patchset was addressing...

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com