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@xxxxxxxxx>
Signed-off-by: NeilBrown <neilb@xxxxxxx>
---
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
