[PATCH v2 12/14] nfsd: reduce want-write range in nfsd4_create_file()

From: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>

nfsd4_create_file() needs write access to the mount for two purposes:

1/ to create/open the file.
2/ to set attributes on the newly created file.

Normally a file being created would be open for write, and once we have
an active open we have the write access needed for a setattr.
However if a file were created but opened read-only then the setattr
wouldn't necessarily have write access to the mount.

Currently this is all handled by holding the write access across the
open and the setattr.  A subsequent patch will necessarily change how
write access is gained for the open.  So we reduce the range for the
first want_write, and add another one only if setattr is needed on a
read-only open.

Also we need to ensure that we actually do open the file which
previously we didn't if it already existed.  So we add a call to
dentry_open() in that case.

Signed-off-by: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
---
 fs/nfsd/nfs4proc.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 7bb476311195..10323c620b71 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -307,21 +307,23 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	host_err = fh_want_write(fhp);
 	if (host_err) {
 		status = nfserrno(host_err);
-		goto out_free;
+		goto out;
 	}
 
 	child = start_creating(&nop_mnt_idmap, parent,
 			       &QSTR_LEN(open->op_fname, open->op_fnamelen));
 	if (IS_ERR(child)) {
 		status = nfserrno(PTR_ERR(child));
+		fh_drop_write(fhp);
 		goto out;
 	}
 	path.dentry = child;
 
 	if (d_really_is_positive(child)) {
 		/*
-		 * open the file so that we consistently have a valid
-		 * op_filp.
+		 * open the file so that, unless it is O_RDONLY, we
+		 * have write-access to the fs for setattr below.
+		 * Also we can be sure that op_filp->f_path.dentry is valid.
 		 */
 		open->op_filp = dentry_open(&path, oflags, current_cred());
 		if (IS_ERR(open->op_filp)) {
@@ -343,6 +345,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		}
 	}
 	end_creating(child);
+	fh_drop_write(fhp);
 	if (status != nfs_ok)
 		goto out;
 
@@ -386,7 +389,24 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0))
 		iap->ia_valid &= ~ATTR_SIZE;
 
-	status = nfsd_create_setattr(rqstp, fhp, resfhp, &attrs);
+	if (((oflags & O_ACCMODE) == O_RDONLY)) {
+		/*
+		 * We will need write access to set the attrs,
+		 * but a successful open won't have provided
+		 * that.
+		 */
+		int host_err = fh_want_write(fhp);
+		if (host_err) {
+			status = nfserrno(host_err);
+		} else {
+			status = nfsd_create_setattr(rqstp, fhp,
+						     resfhp, &attrs);
+			fh_drop_write(fhp);
+		}
+	} else {
+		status = nfsd_create_setattr(rqstp, fhp,
+					     resfhp, &attrs);
+	}
 
 	if (attrs.na_labelerr)
 		open->op_bmval[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
@@ -397,8 +417,6 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (attrs.na_paclerr)
 		open->op_bmval[2] &= ~FATTR4_WORD2_POSIX_ACCESS_ACL;
 out:
-	fh_drop_write(fhp);
-out_free:
 	nfsd_attrs_free(&attrs);
 	return status;
 }
-- 
2.50.0.107.gf914562f5916.dirty