nfscl: Add setting n_localmodtime to the Write RPC code

Similar to commit 2be417843a, I believe there could be a race between
the NFS client VOP_LOOKUP() and file Writing that could result in stale
file attributes being loaded into the NFS vnode by VOP_LOOKUP().

I have not been able to reproduce a failure due to this race, but
I believe that there are two possibilities:

The Lookup RPC happens while VOP_WRITE() is being executed and loads
stale file attributes after VOP_WRITE() returns when it has already
completed the Write/Commit RPC(s).
--> For this case, setting the local modify timestamp at the end of
  VOP_WRITE() should ensure that stale file attributes are not loaded.

The Lookup RPC occurs after VOP_WRITE() has returned, while
asynchronous Write/Commit RPCs are in progress and then is
blocked by the vnode held by VOP_OPEN/VOP_CLOSE/VOP_FSYNC which
will flush writes via ncl_flush() or ncl_vinvalbuf(), clearing the
NMODIFIED flag (which indicates Writes-in-progress). The VOP_LOOKUP()
then acquires the NFS vnode lock and fills in stale file attributes.
 --> Setting the local modify timestamp in ncl_flsuh() and ncl_vinvalbuf()
   when they clear NMODIFIED should ensure that stale file attributes
   are not loaded.

This patch does the above.

PR:	259071
Reviewed by:	asomers
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D32677
This commit is contained in:
Rick Macklem 2021-10-30 17:08:28 -07:00
parent ab87c39c25
commit 50dcff0816
2 changed files with 22 additions and 3 deletions

View File

@ -907,6 +907,7 @@ ncl_write(struct vop_write_args *ap)
int bp_cached, n, on, error = 0, error1, save2, wouldcommit;
size_t orig_resid, local_resid;
off_t orig_size, tmp_off;
struct timespec ts;
KASSERT(uio->uio_rw == UIO_WRITE, ("ncl_write mode"));
KASSERT(uio->uio_segflg != UIO_USERSPACE || uio->uio_td == curthread,
@ -1284,7 +1285,12 @@ ncl_write(struct vop_write_args *ap)
break;
} while (uio->uio_resid > 0 && n > 0);
if (error != 0) {
if (error == 0) {
nanouptime(&ts);
NFSLOCKNODE(np);
np->n_localmodtime = ts;
NFSUNLOCKNODE(np);
} else {
if (ioflag & IO_UNIT) {
VATTR_NULL(&vattr);
vattr.va_size = orig_size;
@ -1356,6 +1362,7 @@ ncl_vinvalbuf(struct vnode *vp, int flags, struct thread *td, int intrflg)
struct nfsmount *nmp = VFSTONFS(vp->v_mount);
int error = 0, slpflag, slptimeo;
bool old_lock;
struct timespec ts;
ASSERT_VOP_LOCKED(vp, "ncl_vinvalbuf");
@ -1400,16 +1407,21 @@ ncl_vinvalbuf(struct vnode *vp, int flags, struct thread *td, int intrflg)
}
if (NFSHASPNFS(nmp)) {
nfscl_layoutcommit(vp, td);
nanouptime(&ts);
/*
* Invalidate the attribute cache, since writes to a DS
* won't update the size attribute.
*/
NFSLOCKNODE(np);
np->n_attrstamp = 0;
} else
} else {
nanouptime(&ts);
NFSLOCKNODE(np);
if (np->n_directio_asyncwr == 0)
}
if (np->n_directio_asyncwr == 0 && (np->n_flag & NMODIFIED) != 0) {
np->n_localmodtime = ts;
np->n_flag &= ~NMODIFIED;
}
NFSUNLOCKNODE(np);
out:
ncl_excl_finish(vp, old_lock);

View File

@ -2914,6 +2914,7 @@ ncl_flush(struct vnode *vp, int waitfor, struct thread *td,
#endif
struct buf *bvec_on_stack[NFS_COMMITBVECSIZ];
u_int bvecsize = 0, bveccount;
struct timespec ts;
if (called_from_renewthread != 0)
slptimeo = hz;
@ -3234,6 +3235,12 @@ ncl_flush(struct vnode *vp, int waitfor, struct thread *td,
vn_printf(vp, "ncl_flush failed");
error = called_from_renewthread != 0 ? EIO : EBUSY;
}
if (error == 0) {
nanouptime(&ts);
NFSLOCKNODE(np);
np->n_localmodtime = ts;
NFSUNLOCKNODE(np);
}
return (error);
}