Re: [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts

Chuck Lever
2020-03-12T16:24:05Z

> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> This adds the filehandle based functions for the xattr operations
> that call in to the vfs layer to do the actual work.

Before posting again, use "make C=1" to clear up new sparse
errors, and scripts/checkpatch.pl to find and correct whitespace
damage introduced in this series.

> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
> ---
> fs/nfsd/vfs.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/vfs.h | 10 +++++
> 2 files changed, 140 insertions(+)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 0aa02eb18bd3..115449009bc0 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -2058,6 +2058,136 @@ static int exp_rdonly(struct svc_rqst *rqstp, struct svc_export *exp)
> return nfsexp_flags(rqstp, exp) & NFSEXP_READONLY;
> }
>
> +#ifdef CONFIG_NFSD_V4
> +/*
> + * Helper function to translate error numbers. In the case of xattr operations,
> + * some error codes need to be translated outside of the standard translations.
> + *
> + * ENODATA needs to be translated to nfserr_noxattr.
> + * E2BIG to nfserr_xattr2big.
> + *
> + * Additionally, vfs_listxattr can return -ERANGE. This means that the
> + * file has too many extended attributes to retrieve inside an
> + * XATTR_LIST_MAX sized buffer. This is a bug in the xattr implementation:
> + * filesystems will allow the adding of extended attributes until they hit
> + * their own internal limit. This limit may be larger than XATTR_LIST_MAX.
> + * So, at that point, the attributes are present and valid, but can't
> + * be retrieved using listxattr, since the upper level xattr code enforces
> + * the XATTR_LIST_MAX limit.
> + *
> + * This bug means that we need to deal with listxattr returning -ERANGE. The
> + * best mapping is to return TOOSMALL.
> + */
> +static __be32
> +nfsd_xattr_errno(int err)
> +{
> + switch (err) {
> + case -ENODATA:
> + return nfserr_noxattr;
> + case -E2BIG:
> + return nfserr_xattr2big;
> + case -ERANGE:
> + return nfserr_toosmall;
> + }
> + return nfserrno(err);
> +}
> +
> +__be32
> +nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
> + void *buf, int *lenp)
> +{
> + ssize_t lerr;
> + int err;
> +
> + err = fh_verify(rqstp, fhp, 0, NFSD_MAY_READ);
> + if (err)
> + return err;
> +
> + lerr = vfs_getxattr(fhp->fh_dentry, name, buf, *lenp);
> + if (lerr < 0)
> + err = nfsd_xattr_errno(lerr);
> + else
> + *lenp = lerr;
> +
> + return err;
> +}
> +
> +__be32
> +nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, void *buf, int *lenp)
> +{
> + ssize_t lerr;
> + int err;
> +
> + err = fh_verify(rqstp, fhp, 0, NFSD_MAY_READ);
> + if (err)
> + return err;
> +
> + lerr = vfs_listxattr(fhp->fh_dentry, buf, *lenp);
> +
> + if (lerr < 0)
> + err = nfsd_xattr_errno(lerr);
> + else
> + *lenp = lerr;
> +
> + return err;
> +}
> +
> +/*
> + * Removexattr and setxattr need to call fh_lock to both lock the inode
> + * and set the change attribute. Since the top-level vfs_removexattr
> + * and vfs_setxattr calls already do their own inode_lock calls, call
> + * the _locked variant. Pass in a NULL pointer for delegated_inode,
> + * and let the client deal with NFS4ERR_DELAY (same as with e.g.
> + * setattr and remove).
> + */
> +__be32
> +nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name)
> +{
> + int err, ret;
> +
> + err = fh_verify(rqstp, fhp, 0, NFSD_MAY_WRITE);
> + if (err)
> + return err;
> +
> + ret = fh_want_write(fhp);
> + if (ret)
> + return nfserrno(ret);
> +
> + fh_lock(fhp);
> +
> + ret = __vfs_removexattr_locked(fhp->fh_dentry, name, NULL);
> +
> + fh_unlock(fhp);
> + fh_drop_write(fhp);
> +
> + return nfsd_xattr_errno(ret);
> +}
> +
> +__be32
> +nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
> + void *buf, u32 len, u32 flags)
> +{
> + int err, ret;
> +
> + err = fh_verify(rqstp, fhp, 0, NFSD_MAY_WRITE);
> + if (err)
> + return err;
> +
> + ret = fh_want_write(fhp);
> + if (ret)
> + return nfserrno(ret);
> + fh_lock(fhp);
> +
> + ret = __vfs_setxattr_locked(fhp->fh_dentry, name, buf, len, flags,
> + NULL);
> +
> + fh_unlock(fhp);
> + fh_drop_write(fhp);
> +
> + return nfsd_xattr_errno(ret);
> +}
> +#endif
> +
> /*
> * Check for a user's access permissions to this inode.
> */
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index 3eb660ad80d1..2d2cf5b0543b 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -76,6 +76,16 @@ __be32 do_nfsd_create(struct svc_rqst *, struct svc_fh *,
> __be32 nfsd_commit(struct svc_rqst *, struct svc_fh *,
> loff_t, unsigned long, __be32 *verf);
> #endif /* CONFIG_NFSD_V3 */
> +#ifdef CONFIG_NFSD_V4
> +__be32 nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> + char *name, void *buf, int *lenp);
> +__be32 nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> + void *buf, int *lenp);
> +__be32 nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> + char *name);
> +__be32 nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> + char *name, void *buf, u32 len, u32 flags);
> +#endif
> int nfsd_open_break_lease(struct inode *, int);
> __be32 nfsd_open(struct svc_rqst *, struct svc_fh *, umode_t,
> int, struct file **);
> --
> 2.16.6
>

--
Chuck Lever