Linux-NFS Archive mirror
 help / color / mirror / Atom feed
From: "NeilBrown" <neilb@suse.de>
To: Chuck Lever <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>, Dai Ngo <Dai.Ngo@oracle.com>,
	Olga Kornievskaia <kolga@netapp.com>, Tom Talpey <tom@talpey.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>,
	linux-nfs@vger.kernel.org
Subject: [PATCH] nfsd: change nfsd_create_setattr() to call nfsd_setattr() unconditionally
Date: Mon, 13 May 2024 15:42:48 +1000	[thread overview]
Message-ID: <171557896893.4857.2572536847924540881@noble.neil.brown.name> (raw)


A recent change improved the guard on calling nfsd_setattr() from
nfsd_create_setattr() so that it could be called even if ia_valid was
zero - if there was a security label that needed to be set.

Unfortunately this is not sufficient as there could be an ACL that needs
to be set.  Most likely in this case there would also be mode bits so
->ia_valid would not be zero, but it isn't safe to depend on that.

Rather than making nfsd_attrs_valid() more complete, this patch removes
it and places the code in-line at the top of nfsd_setattr().  If there
is nothing to be set, that function now short-circuits to the end where
commit_metadata() is called.

With this change it is appropriate to call nfsd_setattr()
unconditionally.

Reported-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/nfsd/vfs.c | 17 +++++++++--------
 fs/nfsd/vfs.h |  8 --------
 2 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 29b1f3613800..e63f870696ad 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -499,6 +499,14 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	bool		size_change = (iap->ia_valid & ATTR_SIZE);
 	int		retries;
 
+	if (!(iap->ia_valid ||
+	      (attr->na_seclabel && attr->na_seclabel->len) ||
+	      (IS_ENABLED(CONFIG_FS_POSIX_ACL) && attr->na_pacl) ||
+	      (IS_ENABLED(CONFIG_FS_POSIX_ACL) &&
+	       !attr->na_aclerr && attr->na_dpacl && S_ISDIR(inode->i_mode))))
+		/* Don't bother with inode_lock() */
+		goto out;
+
 	if (iap->ia_valid & ATTR_SIZE) {
 		accmode |= NFSD_MAY_WRITE|NFSD_MAY_OWNER_OVERRIDE;
 		ftype = S_IFREG;
@@ -1418,14 +1426,7 @@ nfsd_create_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (!uid_eq(current_fsuid(), GLOBAL_ROOT_UID))
 		iap->ia_valid &= ~(ATTR_UID|ATTR_GID);
 
-	/*
-	 * Callers expect new file metadata to be committed even
-	 * if the attributes have not changed.
-	 */
-	if (nfsd_attrs_valid(attrs))
-		status = nfsd_setattr(rqstp, resfhp, attrs, NULL);
-	else
-		status = nfserrno(commit_metadata(resfhp));
+	status = nfsd_setattr(rqstp, resfhp, attrs, NULL);
 
 	/*
 	 * Transactional filesystems had a chance to commit changes
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 57cd70062048..c60fdb6200fd 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -60,14 +60,6 @@ static inline void nfsd_attrs_free(struct nfsd_attrs *attrs)
 	posix_acl_release(attrs->na_dpacl);
 }
 
-static inline bool nfsd_attrs_valid(struct nfsd_attrs *attrs)
-{
-	struct iattr *iap = attrs->na_iattr;
-
-	return (iap->ia_valid || (attrs->na_seclabel &&
-		attrs->na_seclabel->len));
-}
-
 __be32		nfserrno (int errno);
 int		nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
 		                struct svc_export **expp);
-- 
2.44.0


             reply	other threads:[~2024-05-13  5:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-13  5:42 NeilBrown [this message]
2024-05-13  6:31 ` [PATCH] nfsd: change nfsd_create_setattr() to call nfsd_setattr() unconditionally Trond Myklebust
2024-05-13 15:16 ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=171557896893.4857.2572536847924540881@noble.neil.brown.name \
    --to=neilb@suse.de \
    --cc=Dai.Ngo@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=kolga@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).