NFSD: Instantiate a struct file when creating a regular NFSv4 file
authorChuck Lever <chuck.lever@oracle.com>
Wed, 30 Mar 2022 14:30:54 +0000 (10:30 -0400)
committerChuck Lever <chuck.lever@oracle.com>
Mon, 23 May 2022 15:06:29 +0000 (11:06 -0400)
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 provide a status code for this case.

To mitigate some of these problems, reorganize the NFSv4
OPEN(CREATE) logic to allocate resources before the file is actually
created, and open the new file while the parent directory is still
locked.

Two new APIs are added:

+ Add an API that works like nfsd_file_acquire() but does not open
the underlying file. The OPEN(CREATE) path can use this API when it
already has an open file.

+ Add an API that is kin to dentry_open(). NFSD needs to create a
file and grab an open "struct file *" atomically. The
alloc_empty_file() has to be done before the inode create. If it
fails (for example, because the NFS server has exceeded its
max_files limit), we avoid creating the file and can still return
an error to the NFS client.

BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=382
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Tested-by: JianHong Yin <jiyin@redhat.com>
fs/nfsd/filecache.c
fs/nfsd/filecache.h
fs/nfsd/nfs4proc.c
fs/nfsd/nfs4state.c
fs/nfsd/xdr4.h
fs/open.c
include/linux/fs.h

index 781bef7e42d9085e7d0cd3e5a0a449d8f1003848..24772f246461ce0d7dba0b475f3236809df84f8c 100644 (file)
@@ -897,9 +897,9 @@ nfsd_file_is_cached(struct inode *inode)
        return ret;
 }
 
-__be32
-nfsd_file_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
-                 unsigned int may_flags, struct nfsd_file **pnf)
+static __be32
+nfsd_do_file_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
+                    unsigned int may_flags, struct nfsd_file **pnf, bool open)
 {
        __be32  status;
        struct net *net = SVC_NET(rqstp);
@@ -994,10 +994,13 @@ open_file:
                nfsd_file_gc();
 
        nf->nf_mark = nfsd_file_mark_find_or_create(nf);
-       if (nf->nf_mark)
-               status = nfsd_open_verified(rqstp, fhp, may_flags,
-                                           &nf->nf_file);
-       else
+       if (nf->nf_mark) {
+               if (open)
+                       status = nfsd_open_verified(rqstp, fhp, may_flags,
+                                                   &nf->nf_file);
+               else
+                       status = nfs_ok;
+       } else
                status = nfserr_jukebox;
        /*
         * If construction failed, or we raced with a call to unlink()
@@ -1017,6 +1020,40 @@ open_file:
        goto out;
 }
 
+/**
+ * nfsd_file_acquire - Get a struct nfsd_file with an open file
+ * @rqstp: the RPC transaction being executed
+ * @fhp: the NFS filehandle of the file to be opened
+ * @may_flags: NFSD_MAY_ settings for the file
+ * @pnf: OUT: new or found "struct nfsd_file" object
+ *
+ * Returns nfs_ok and sets @pnf on success; otherwise an nfsstat in
+ * network byte order is returned.
+ */
+__be32
+nfsd_file_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
+                 unsigned int may_flags, struct nfsd_file **pnf)
+{
+       return nfsd_do_file_acquire(rqstp, fhp, may_flags, pnf, true);
+}
+
+/**
+ * nfsd_file_create - Get a struct nfsd_file, do not open
+ * @rqstp: the RPC transaction being executed
+ * @fhp: the NFS filehandle of the file just created
+ * @may_flags: NFSD_MAY_ settings for the file
+ * @pnf: OUT: new or found "struct nfsd_file" object
+ *
+ * Returns nfs_ok and sets @pnf on success; otherwise an nfsstat in
+ * network byte order is returned.
+ */
+__be32
+nfsd_file_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
+                unsigned int may_flags, struct nfsd_file **pnf)
+{
+       return nfsd_do_file_acquire(rqstp, fhp, may_flags, pnf, false);
+}
+
 /*
  * Note that fields may be added, removed or reordered in the future. Programs
  * scraping this file for info should test the labels to ensure they're
index 435ceab27897addb7a9cc5202be27ddc2f4ba419..1da0c79a558048efca98716449f6709eaeca2855 100644 (file)
@@ -59,5 +59,7 @@ void nfsd_file_close_inode_sync(struct inode *inode);
 bool nfsd_file_is_cached(struct inode *inode);
 __be32 nfsd_file_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
                  unsigned int may_flags, struct nfsd_file **nfp);
+__be32 nfsd_file_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
+                 unsigned int may_flags, struct nfsd_file **nfp);
 int    nfsd_file_cache_stats_open(struct inode *, struct file *);
 #endif /* _FS_NFSD_FILECACHE_H */
index 99c0485a29e9feef7515bba4046479455c45442a..28bae84d78091455758b74f455cdc6dc66d5e870 100644 (file)
@@ -243,6 +243,37 @@ static inline bool nfsd4_create_is_exclusive(int createmode)
                createmode == NFS4_CREATE_EXCLUSIVE4_1;
 }
 
+static __be32
+nfsd4_vfs_create(struct svc_fh *fhp, struct dentry *child,
+                struct nfsd4_open *open)
+{
+       struct file *filp;
+       struct path path;
+       int oflags;
+
+       oflags = O_CREAT | O_LARGEFILE;
+       switch (open->op_share_access & NFS4_SHARE_ACCESS_BOTH) {
+       case NFS4_SHARE_ACCESS_WRITE:
+               oflags |= O_WRONLY;
+               break;
+       case NFS4_SHARE_ACCESS_BOTH:
+               oflags |= O_RDWR;
+               break;
+       default:
+               oflags |= O_RDONLY;
+       }
+
+       path.mnt = fhp->fh_export->ex_path.mnt;
+       path.dentry = child;
+       filp = dentry_create(&path, oflags, open->op_iattr.ia_mode,
+                            current_cred());
+       if (IS_ERR(filp))
+               return nfserrno(PTR_ERR(filp));
+
+       open->op_filp = filp;
+       return nfs_ok;
+}
+
 /*
  * Implement NFSv4's unchecked, guarded, and exclusive create
  * semantics for regular files. Open state for this new file is
@@ -355,11 +386,9 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (!IS_POSIXACL(inode))
                iap->ia_mode &= ~current_umask();
 
-       host_err = vfs_create(&init_user_ns, inode, child, iap->ia_mode, true);
-       if (host_err < 0) {
-               status = nfserrno(host_err);
+       status = nfsd4_vfs_create(fhp, child, open);
+       if (status != nfs_ok)
                goto out;
-       }
        open->op_created = true;
 
        /* A newly created file already has a file size of zero. */
@@ -517,6 +546,8 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
                (int)open->op_fnamelen, open->op_fname,
                open->op_openowner);
 
+       open->op_filp = NULL;
+
        /* This check required by spec. */
        if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
                return nfserr_inval;
@@ -613,6 +644,10 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
        if (reclaim && !status)
                nn->somebody_reclaimed = true;
 out:
+       if (open->op_filp) {
+               fput(open->op_filp);
+               open->op_filp = NULL;
+       }
        if (resfh && resfh != &cstate->current_fh) {
                fh_dup2(&cstate->current_fh, resfh);
                fh_put(resfh);
index 231e5c19cdb729551b4d98ec2d0908dc8defd307..131102cba06be13ad72892273a638d8195630433 100644 (file)
@@ -5093,9 +5093,19 @@ static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
 
        if (!fp->fi_fds[oflag]) {
                spin_unlock(&fp->fi_lock);
-               status = nfsd_file_acquire(rqstp, cur_fh, access, &nf);
-               if (status)
-                       goto out_put_access;
+
+               if (!open->op_filp) {
+                       status = nfsd_file_acquire(rqstp, cur_fh, access, &nf);
+                       if (status != nfs_ok)
+                               goto out_put_access;
+               } else {
+                       status = nfsd_file_create(rqstp, cur_fh, access, &nf);
+                       if (status != nfs_ok)
+                               goto out_put_access;
+                       nf->nf_file = open->op_filp;
+                       open->op_filp = NULL;
+               }
+
                spin_lock(&fp->fi_lock);
                if (!fp->fi_fds[oflag]) {
                        fp->fi_fds[oflag] = nf;
index 846ab6df9d48f270ad3bf70bb2dbdb9017bc8cad..7b744011f2d3d8402d24cb6ddb8103055b51bd4e 100644 (file)
@@ -273,6 +273,7 @@ struct nfsd4_open {
        bool            op_truncate;        /* used during processing */
        bool            op_created;         /* used during processing */
        struct nfs4_openowner *op_openowner; /* used during processing */
+       struct file     *op_filp;           /* used during processing */
        struct nfs4_file *op_file;          /* used during processing */
        struct nfs4_ol_stateid *op_stp;     /* used during processing */
        struct nfs4_clnt_odstate *op_odstate; /* used during processing */
index 1315253e024739de4b1c592dabfcd04224a427c9..117355ae6bd5f75bd7625b74971081307cfee0c7 100644 (file)
--- a/fs/open.c
+++ b/fs/open.c
@@ -981,6 +981,48 @@ struct file *dentry_open(const struct path *path, int flags,
 }
 EXPORT_SYMBOL(dentry_open);
 
+/**
+ * dentry_create - Create and open a file
+ * @path: path to create
+ * @flags: O_ flags
+ * @mode: mode bits for new file
+ * @cred: credentials to use
+ *
+ * Caller must hold the parent directory's lock, and have prepared
+ * a negative dentry, placed in @path->dentry, for the new file.
+ *
+ * Caller sets @path->mnt to the vfsmount of the filesystem where
+ * the new file is to be created. The parent directory and the
+ * negative dentry must reside on the same filesystem instance.
+ *
+ * On success, returns a "struct file *". Otherwise a ERR_PTR
+ * is returned.
+ */
+struct file *dentry_create(const struct path *path, int flags, umode_t mode,
+                          const struct cred *cred)
+{
+       struct file *f;
+       int error;
+
+       validate_creds(cred);
+       f = alloc_empty_file(flags, cred);
+       if (IS_ERR(f))
+               return f;
+
+       error = vfs_create(mnt_user_ns(path->mnt),
+                          d_inode(path->dentry->d_parent),
+                          path->dentry, mode, true);
+       if (!error)
+               error = vfs_open(path, f);
+
+       if (unlikely(error)) {
+               fput(f);
+               return ERR_PTR(error);
+       }
+       return f;
+}
+EXPORT_SYMBOL(dentry_create);
+
 struct file *open_with_fake_path(const struct path *path, int flags,
                                struct inode *inode, const struct cred *cred)
 {
index aa6c1bbdb8c439975afa31f016751af76c9953d3..b848355b5e6c6c56ddbdfa486ebcd8f65f4121de 100644 (file)
@@ -2640,6 +2640,8 @@ static inline struct file *file_open_root_mnt(struct vfsmount *mnt,
                              name, flags, mode);
 }
 extern struct file * dentry_open(const struct path *, int, const struct cred *);
+extern struct file *dentry_create(const struct path *path, int flags,
+                                 umode_t mode, const struct cred *cred);
 extern struct file * open_with_fake_path(const struct path *, int,
                                         struct inode*, const struct cred *);
 static inline struct file *file_clone_open(struct file *file)