On Sat, 16 Jul 2022, Jeff Layton wrote:
> On Fri, 2022-07-15 at 12:11 -0400, Jeff Layton wrote:
>
>
> [PATCH] SQUASH: nfsd: ensure we fill in pre-op-attrs in
> nfsd4_create_file
>
> In some cases, they're left uninitialized. This also ensures that the
> post_op attrs are properly filled in all cases too.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
Thanks Jeff, but I think this is more noisy than necessary.
The problem is that the d_really_is_positive() doesn't actually change
the directory (obviously) but can succeed - so pre/post attributes are
needed by NFSv4 even though they aren't really relevant.
I would rather use the same approach as in the !open->op_create branch
in d_open_lookup() :
fh_fill_pre_attrs(current_fh);
fh_fill_post_attrs(current_fh);
with a comment explaining that as the directory is locked, and as it
isn't being changed, this makes sense.
I'll fold that in.
Thanks,
NeilBrown
> ---
> fs/nfsd/nfs4proc.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 242f059e6788..05652a7dabe8 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -346,6 +346,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>
> switch (open->op_createmode) {
> case NFS4_CREATE_UNCHECKED:
> + fh_fill_pre_attrs(fhp);
> if (!d_is_reg(child))
> break;
>
> @@ -365,6 +366,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> d_inode(child)->i_atime.tv_sec == v_atime &&
> d_inode(child)->i_size == 0) {
> + fh_fill_pre_attrs(fhp);
> open->op_created = true;
> break; /* subtle */
> }
> @@ -374,6 +376,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> d_inode(child)->i_atime.tv_sec == v_atime &&
> d_inode(child)->i_size == 0) {
> + fh_fill_pre_attrs(fhp);
> open->op_created = true;
> goto set_attr; /* subtle */
> }
> @@ -385,12 +388,10 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> if (!IS_POSIXACL(inode))
> iap->ia_mode &= ~current_umask();
>
> - fh_fill_pre_attrs(fhp);
> status = nfsd4_vfs_create(fhp, child, open);
> if (status != nfs_ok)
> goto out;
> open->op_created = true;
> - fh_fill_post_attrs(fhp);
>
> /* A newly created file already has a file size of zero. */
> if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0))
> @@ -408,6 +409,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> status = nfsd_create_setattr(rqstp, fhp, resfhp, iap);
>
> out:
> + if (status == nfs_ok)
> + fh_fill_post_attrs(fhp);
> inode_unlock(inode);
> if (child && !IS_ERR(child))
> dput(child);
> --
> 2.36.1
>
>
>
