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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 27 Aug 2022 11:01:22 +0300 From: Amir Goldstein <amir73il@...il.com> To: Jeff Layton <jlayton@...nel.org> Cc: Theodore Tso <tytso@....edu>, Andreas Dilger <adilger.kernel@...ger.ca>, "Darrick J. Wong" <djwong@...nel.org>, Dave Chinner <david@...morbit.com>, Trond Myklebust <trondmy@...merspace.com>, Neil Brown <neilb@...e.de>, Al Viro <viro@...iv.linux.org.uk>, Mimi Zohar <zohar@...ux.ibm.com>, xiubli@...hat.com, Chuck Lever <chuck.lever@...cle.com>, Lukas Czerner <lczerner@...hat.com>, Jan Kara <jack@...e.cz>, Christian Brauner <brauner@...nel.org>, Linux API <linux-api@...r.kernel.org>, Linux Btrfs <linux-btrfs@...r.kernel.org>, linux-fsdevel <linux-fsdevel@...r.kernel.org>, linux-kernel <linux-kernel@...r.kernel.org>, Ext4 <linux-ext4@...r.kernel.org>, Linux NFS Mailing List <linux-nfs@...r.kernel.org>, linux-xfs <linux-xfs@...r.kernel.org>, David Wysochanski <dwysocha@...hat.com>, ceph-devel <ceph-devel@...r.kernel.org> Subject: Re: [PATCH v3 4/7] xfs: don't bump the i_version on an atime update in xfs_vn_update_time On Sat, Aug 27, 2022 at 10:26 AM Amir Goldstein <amir73il@...il.com> wrote: > > On Sat, Aug 27, 2022 at 12:49 AM Jeff Layton <jlayton@...nel.org> wrote: > > > > xfs will update the i_version when updating only the atime value, which > > is not desirable for any of the current consumers of i_version. Doing so > > leads to unnecessary cache invalidations on NFS and extra measurement > > activity in IMA. > > > > Add a new XFS_ILOG_NOIVER flag, and use that to indicate that the > > transaction should not update the i_version. Set that value in > > xfs_vn_update_time if we're only updating the atime. > > > > Cc: Dave Chinner <david@...morbit.com> > > Cc: NeilBrown <neilb@...e.de> > > Cc: Trond Myklebust <trondmy@...merspace.com> > > Cc: David Wysochanski <dwysocha@...hat.com> > > Signed-off-by: Jeff Layton <jlayton@...nel.org> > > --- > > fs/xfs/libxfs/xfs_log_format.h | 2 +- > > fs/xfs/libxfs/xfs_trans_inode.c | 2 +- > > fs/xfs/xfs_iops.c | 11 +++++++++-- > > 3 files changed, 11 insertions(+), 4 deletions(-) > > > > Dave has NACK'ed this patch, but I'm sending it as a way to illustrate > > the problem. I still think this approach should at least fix the worst > > problems with atime updates being counted. We can look to carve out > > other "spurious" i_version updates as we identify them. > > > > AFAIK, "spurious" is only inode blocks map changes due to writeback > of dirty pages. Anybody know about other cases? > > Regarding inode blocks map changes, first of all, I don't think that there is > any practical loss from invalidating NFS client cache on dirty data writeback, > because NFS server should be serving cold data most of the time. > If there are a few unneeded cache invalidations they would only be temporary. > Unless there is an issue with a writer NFS client that invalidates its own attribute caches on server data writeback? > One may even consider if NFSv4 server should not flush dirty data of an inode > before granting a read lease to client. > After all, if read lease was granted, client cached data and then server crashed > before persisting the dirty data, then client will have cached a > "future" version > of the data and if i_version on the server did not roll back in that situation, > we are looking at possible data corruptions. > > Same goes for IMA. IIUC, IMA data checksum would be stored in xattr? > Storing in xattr a data checksum for data that is not persistent on disk > would be an odd choice. > > So in my view, I only see benefits to current i_version users in the xfs > i_version implementations and I don't think that it contradicts the > i_version definition in the man page patch. > > > If however there are offline analysis tools that require atime updates > > to be counted, then we won't be able to do this. If that's the case, how > > can we fix this such that serving xfs via NFSv4 doesn't suck? > > > > If I read the arguments correctly, implicit atime updates could be relaxed > as long as this behavior is clearly documented and coherent on all > implementations. > > Forensics and other applications that care about atime updates can and > should check atime and don't need i_version to know that it was changed. > The reliability of atime as an audit tool has dropped considerably since > the default in relatime. > If we want to be paranoid, maybe we can leave i_version increment on > atime updates in case the user opted-in to strict '-o atime' updates, but > IMO, there is no need for that. > > Thanks, > Amir.
Powered by blists - more mailing lists