On Thu, 2022-03-24 at 18:08 -0400, Chuck Lever wrote: > There have been reports of races that cause NFSv4 OPEN(CREATE) to > return an error even though the requested file was created. NFSv4 > does not seem to provide a status code for this case. > > There are plenty of things that can go wrong between the > vfs_create() call in do_nfsd_create() and nfs4_vfs_get_file(), but > one of them is another client looking up and modifying the file's > mode bits in that window. If that happens, the creator might lose > access to the file before it can be opened. > > Instead of opening the newly created file in nfsd4_process_open2(), > open it as soon as the file is created, and leave it sitting in the > file cache. > > This patch is not optimal, of course - we should replace the > vfs_create() call in do_nfsd_create() with a call that creates the > file and returns a "struct file *" that can be planted immediately > in nf->nf_file. > > But first, I would like to hear opinions about the approach > suggested above. > > BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=382 > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > --- > fs/nfsd/vfs.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index 02a544645b52..80b568fa12f1 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -1410,6 +1410,7 @@ do_nfsd_create(struct svc_rqst *rqstp, struct > svc_fh *fhp, > __be32 err; > int host_err; > __u32 v_mtime=0, v_atime=0; > + struct nfsd_file *nf; > > /* XXX: Shouldn't this be nfserr_inval? */ > err = nfserr_perm; > @@ -1535,6 +1536,14 @@ do_nfsd_create(struct svc_rqst *rqstp, struct > svc_fh *fhp, > iap->ia_atime.tv_nsec = 0; > } > > + /* Attempt to instantiate a filecache entry while we still > hold > + * the parent's inode mutex. */ > + err = nfsd_file_acquire(rqstp, resfhp, NFSD_MAY_WRITE, &nf); > + if (err) > + /* This would be bad */ > + goto out; > + nfsd_file_put(nf); Why rely on the file cache to carry the nfsd_file? Isn't it much easier just to pass it back directly to the caller? There are only 2 callers of do_nfsd_create(), so you'd have nfsd3_proc_create() that will just call nfsd_file_put() as per above, whereas the NFSv4 specific do_open_lookup() could just stash it in the struct nfsd4_open so that it can get passed into do_open_permission() and eventually nfsd4_process_open2(). > + > set_attr: > err = nfsd_create_setattr(rqstp, resfhp, iap); > > > -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
