Hi Scott,
On 4/11/25 4:16 PM, Scott Mayhew wrote:
> The nfs inodes for referral anchors that have not yet been followed have
> their filehandles zeroed out.
>
> Attempting to call getxattr() on one of these will cause the nfs client
> to send a GETATTR to the nfs server with the preceding PUTFH sans
> filehandle. The server will reply NFS4ERR_NOFILEHANDLE, leading to -EIO
> being returned to the application.
>
> For example:
>
> $ strace -e trace=getxattr getfattr -n system.nfs4_acl /mnt/t/ref
> getxattr("/mnt/t/ref", "system.nfs4_acl", NULL, 0) = -1 EIO (Input/output error)
> /mnt/t/ref: system.nfs4_acl: Input/output error
> +++ exited with 1 +++
>
> Have the xattr handlers return -ENODATA instead.
This looks like a lot of repeated code. I was wondering if there is a way to
do this with a helper function that returns -ENODATA? Or could it be checked
inside nfs4_proc_set_acl() / nfs4_proc_get_acl() / nfs4_server_supports_acls()
to cut down on duplication?
>
> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 970f28dbf253..a01592930370 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -7933,6 +7933,11 @@ static int nfs4_xattr_set_nfs4_acl(const struct xattr_handler *handler,
> const char *key, const void *buf,
> size_t buflen, int flags)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> return nfs4_proc_set_acl(inode, buf, buflen, NFS4ACL_ACL);
> }
>
> @@ -7940,11 +7945,21 @@ static int nfs4_xattr_get_nfs4_acl(const struct xattr_handler *handler,
> struct dentry *unused, struct inode *inode,
> const char *key, void *buf, size_t buflen)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> return nfs4_proc_get_acl(inode, buf, buflen, NFS4ACL_ACL);
> }
>
> static bool nfs4_xattr_list_nfs4_acl(struct dentry *dentry)
> {
> + struct nfs_fh *fh = NFS_FH(d_inode(dentry));
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
Note that the return value of this function is a boolean, and not an integer,
so callers probably won't care about a specific return value.
> +
> return nfs4_server_supports_acls(NFS_SB(dentry->d_sb), NFS4ACL_ACL);
> }
>
> @@ -7957,6 +7972,11 @@ static int nfs4_xattr_set_nfs4_dacl(const struct xattr_handler *handler,
> const char *key, const void *buf,
> size_t buflen, int flags)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> return nfs4_proc_set_acl(inode, buf, buflen, NFS4ACL_DACL);
> }
>
> @@ -7964,11 +7984,21 @@ static int nfs4_xattr_get_nfs4_dacl(const struct xattr_handler *handler,
> struct dentry *unused, struct inode *inode,
> const char *key, void *buf, size_t buflen)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> return nfs4_proc_get_acl(inode, buf, buflen, NFS4ACL_DACL);
> }
>
> static bool nfs4_xattr_list_nfs4_dacl(struct dentry *dentry)
> {
> + struct nfs_fh *fh = NFS_FH(d_inode(dentry));
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
Here's another boolean return type.
> +
> return nfs4_server_supports_acls(NFS_SB(dentry->d_sb), NFS4ACL_DACL);
> }
>
> @@ -7980,6 +8010,11 @@ static int nfs4_xattr_set_nfs4_sacl(const struct xattr_handler *handler,
> const char *key, const void *buf,
> size_t buflen, int flags)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> return nfs4_proc_set_acl(inode, buf, buflen, NFS4ACL_SACL);
> }
>
> @@ -7987,11 +8022,21 @@ static int nfs4_xattr_get_nfs4_sacl(const struct xattr_handler *handler,
> struct dentry *unused, struct inode *inode,
> const char *key, void *buf, size_t buflen)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> return nfs4_proc_get_acl(inode, buf, buflen, NFS4ACL_SACL);
> }
>
> static bool nfs4_xattr_list_nfs4_sacl(struct dentry *dentry)
> {
> + struct nfs_fh *fh = NFS_FH(d_inode(dentry));
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
Boolean return type.
Thanks,
Anna
> +
> return nfs4_server_supports_acls(NFS_SB(dentry->d_sb), NFS4ACL_SACL);
> }
>
> @@ -8005,6 +8050,11 @@ static int nfs4_xattr_set_nfs4_label(const struct xattr_handler *handler,
> const char *key, const void *buf,
> size_t buflen, int flags)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> if (security_ismaclabel(key))
> return nfs4_set_security_label(inode, buf, buflen);
>
> @@ -8015,6 +8065,11 @@ static int nfs4_xattr_get_nfs4_label(const struct xattr_handler *handler,
> struct dentry *unused, struct inode *inode,
> const char *key, void *buf, size_t buflen)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> +
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> if (security_ismaclabel(key))
> return nfs4_get_security_label(inode, buf, buflen);
> return -EOPNOTSUPP;
> @@ -8023,8 +8078,12 @@ static int nfs4_xattr_get_nfs4_label(const struct xattr_handler *handler,
> static ssize_t
> nfs4_listxattr_nfs4_label(struct inode *inode, char *list, size_t list_len)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> int len = 0;
>
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> if (nfs_server_capable(inode, NFS_CAP_SECURITY_LABEL)) {
> len = security_inode_listsecurity(inode, list, list_len);
> if (len >= 0 && list_len && len > list_len)
> @@ -8056,9 +8115,13 @@ static int nfs4_xattr_set_nfs4_user(const struct xattr_handler *handler,
> const char *key, const void *buf,
> size_t buflen, int flags)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> u32 mask;
> int ret;
>
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> if (!nfs_server_capable(inode, NFS_CAP_XATTR))
> return -EOPNOTSUPP;
>
> @@ -8093,9 +8156,13 @@ static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler,
> struct dentry *unused, struct inode *inode,
> const char *key, void *buf, size_t buflen)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> u32 mask;
> ssize_t ret;
>
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> if (!nfs_server_capable(inode, NFS_CAP_XATTR))
> return -EOPNOTSUPP;
>
> @@ -8120,6 +8187,7 @@ static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler,
> static ssize_t
> nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len)
> {
> + struct nfs_fh *fh = NFS_FH(inode);
> u64 cookie;
> bool eof;
> ssize_t ret, size;
> @@ -8127,6 +8195,9 @@ nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len)
> size_t buflen;
> u32 mask;
>
> + if (unlikely(fh->size == 0))
> + return -ENODATA;
> +
> if (!nfs_server_capable(inode, NFS_CAP_XATTR))
> return 0;
>
