On Thu, 2022-09-08 at 12:08 +1000, NeilBrown wrote:
> A recent patch moved ACL setting into nfsd_setattr().
> Unfortunately it didn't work as nfsd_setattr() aborts early if
> iap->ia_valid is 0.
>
> Remove this test, and instead avoid calling notify_change() when
> ia_valid is 0.
>
> This means that nfsd_setattr() will now *always* lock the inode.
> Previously it didn't if only a ATTR_MODE change was requested on a
> symlink (see Commit 15b7a1b86d66 ("[PATCH] knfsd: fix setattr-on-symlink
> error return")). I don't think this change really matters.
>
> Fixes: c0cbe70742f4 ("NFSD: add posix ACLs to struct nfsd_attrs")
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfsd/vfs.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 9f486b788ed0..bffb31540df8 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -353,7 +353,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> int accmode = NFSD_MAY_SATTR;
> umode_t ftype = 0;
> __be32 err;
> - int host_err;
> + int host_err = 0;
> bool get_write_count;
> bool size_change = (iap->ia_valid & ATTR_SIZE);
>
> @@ -395,9 +395,6 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> if (S_ISLNK(inode->i_mode))
> iap->ia_valid &= ~ATTR_MODE;
>
> - if (!iap->ia_valid)
> - return 0;
> -
> nfsd_sanitize_attrs(inode, iap);
>
> if (check_guard && guardtime != inode->i_ctime.tv_sec)
> @@ -448,8 +445,10 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> goto out_unlock;
> }
>
> - iap->ia_valid |= ATTR_CTIME;
> - host_err = notify_change(&init_user_ns, dentry, iap, NULL);
> + if (iap->ia_valid) {
> + iap->ia_valid |= ATTR_CTIME;
> + host_err = notify_change(&init_user_ns, dentry, iap, NULL);
> + }
>
> out_unlock:
> if (attr->na_seclabel && attr->na_seclabel->len)
Looks sane.
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
