On Thu, 2022-07-28 at 10:24 -0400, Jeff Layton wrote: > Olga reported a case of data corruption in a situation where two > clients > (v3 and v4) were alternately doing open/write/close the same file. > > Looking at captures, the v3 client failed to perform any of the > GETATTR > calls for CTO during one of the events, leading it to overwrite some > data that had been written by the v4 client. > > We have two calls that work similarly: put_nfs_open_context and > put_nfs_open_context_sync. The only difference is the is_sync > parameter > that gets passed to close_context. The only caller of the _sync > variant > is in the close codepath. > > On a v2/3 mount, if the last reference is not put by > put_nfs_open_context_sync, then CTO revalidation never happens. Fix > this > by adding a new flag to nfs_open_context and set that in > put_nfs_open_context_sync. In nfs_close_context, check for that flag > when we're checking is_sync and treat them as equivalent. > > Cc: Scott Mayhew <smayhew@xxxxxxxxxx> > Cc: Ben Coddington <bcodding@xxxxxxxxxx> > Reported-by: Olga Kornieskaia <kolga@xxxxxxxxxx> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > --- > fs/nfs/inode.c | 3 ++- > include/linux/nfs_fs.h | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > I'm not sure this is the right fix. Maybe we'd be better off just > ignoring the is_sync parameter in nfs_close_context? It's probably > functionally equivalent anyway. > > Thoughts? > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > index b4e46b0ffa2d..58b506a0a2f2 100644 > --- a/fs/nfs/inode.c > +++ b/fs/nfs/inode.c > @@ -1005,7 +1005,7 @@ void nfs_close_context(struct nfs_open_context > *ctx, int is_sync) > > if (!(ctx->mode & FMODE_WRITE)) > return; > - if (!is_sync) > + if (!is_sync && !test_bit(NFS_CONTEXT_DO_CLOSE, &ctx->flags)) > return; > inode = d_inode(ctx->dentry); > if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) NACK. If the 'is_sync' flag is not set, then it is because the function is being called from a context where it is not safe to do a synchronous RPC call. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
