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: Mon, 29 Aug 2022 17:56:51 +1000 From: Dave Chinner <david@...morbit.com> To: Jeff Layton <jlayton@...nel.org> Cc: tytso@....edu, adilger.kernel@...ger.ca, djwong@...nel.org, trondmy@...merspace.com, neilb@...e.de, viro@...iv.linux.org.uk, zohar@...ux.ibm.com, xiubli@...hat.com, chuck.lever@...cle.com, lczerner@...hat.com, jack@...e.cz, brauner@...nel.org, linux-api@...r.kernel.org, linux-btrfs@...r.kernel.org, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, linux-ceph@...r.kernel.org, linux-ext4@...r.kernel.org, linux-nfs@...r.kernel.org, linux-xfs@...r.kernel.org, Colin Walters <walters@...bum.org> Subject: Re: [PATCH v3 1/7] iversion: update comments with info about atime updates On Fri, Aug 26, 2022 at 05:46:57PM -0400, Jeff Layton wrote: > The i_version field in the kernel has had different semantics over > the decades, but we're now proposing to expose it to userland via > statx. This means that we need a clear, consistent definition of > what it means and when it should change. > > Update the comments in iversion.h to describe how a conformant > i_version implementation is expected to behave. This definition > suits the current users of i_version (NFSv4 and IMA), but is > loose enough to allow for a wide range of possible implementations. > > Cc: Colin Walters <walters@...bum.org> > Cc: NeilBrown <neilb@...e.de> > Cc: Trond Myklebust <trondmy@...merspace.com> > Cc: Dave Chinner <david@...morbit.com> > Link: https://lore.kernel.org/linux-xfs/166086932784.5425.17134712694961326033@noble.neil.brown.name/#t > Signed-off-by: Jeff Layton <jlayton@...nel.org> > --- > include/linux/iversion.h | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/include/linux/iversion.h b/include/linux/iversion.h > index 3bfebde5a1a6..45e93e1b4edc 100644 > --- a/include/linux/iversion.h > +++ b/include/linux/iversion.h > @@ -9,8 +9,19 @@ > * --------------------------- > * The change attribute (i_version) is mandated by NFSv4 and is mostly for > * knfsd, but is also used for other purposes (e.g. IMA). The i_version must > - * appear different to observers if there was a change to the inode's data or > - * metadata since it was last queried. > + * appear different to observers if there was an explicit change to the inode's > + * data or metadata since it was last queried. > + * > + * An explicit change is one that would ordinarily result in a change to the > + * inode status change time (aka ctime). The version must appear to change, even > + * if the ctime does not (since the whole point is to avoid missing updates due > + * to timestamp granularity). If POSIX mandates that the ctime must change due > + * to an operation, then the i_version counter must be incremented as well. > + * > + * A conformant implementation is allowed to increment the counter in other > + * cases, but this is not optimal. NFSv4 and IMA both use this value to determine > + * whether caches are up to date. Spurious increments can cause false cache > + * invalidations. "not optimal", but never-the-less allowed - that's "unspecified behaviour" if I've ever seen it. How is userspace supposed to know/deal with this? Indeed, this loophole clause doesn't exist in the man pages that define what statx.stx_ino_version means. The man pages explicitly define that stx_ino_version only ever changes when stx_ctime changes. IOWs, the behaviour userspace developers are going to expect *does not include* stx_ino_version changing it more often than ctime is changed. Hence a kernel iversion implementation that bumps the counter more often than ctime changes *is not conformant with the statx version counter specification*. IOWs, we can't export such behaviour to userspace *ever* - it is a non-conformant implementation. Hence I think anything that bumps iversion outside the bounds of the statx definition should be declared as such: "Non-conformant iversion implementations: - MUST NOT be exported by statx() to userspace - MUST be -tolerated- by kernel internal applications that use iversion for their own purposes." Cheers, Dave. -- Dave Chinner david@...morbit.com
Powered by blists - more mailing lists