linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] NFS server user xattr support (RFC8276)
@ 2020-03-27 23:27 Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 01/11] xattr: break delegations in {set,remove}xattr and add _locked versions Frank van der Linden
                   ` (10 more replies)
  0 siblings, 11 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

v1 is here: https://www.spinics.net/lists/linux-nfs/msg76740.html

v2:

* Moved the xattr changes upfront, and Cc-ed them to the appropriate
  recipients.
* Added doxygen comments to the new exported functions (xattr)
* Squashed a number of patches together to avoid unused function
  warnings.
* Renamed nfs4_vbuf_from_stream to nfs4_vbuf_from_vector

 fs/nfsd/nfs4proc.c        | 140 +++++++++-
 fs/nfsd/nfs4xdr.c         | 535 +++++++++++++++++++++++++++++++++++---
 fs/nfsd/nfsd.h            |   5 +-
 fs/nfsd/vfs.c             | 142 ++++++++++
 fs/nfsd/vfs.h             |  10 +
 fs/nfsd/xdr4.h            |  31 +++
 fs/xattr.c                | 111 +++++++-
 include/linux/nfs4.h      |  22 +-
 include/linux/xattr.h     |   4 +
 include/uapi/linux/nfs4.h |   3 +
 10 files changed, 961 insertions(+), 42 deletions(-)

-- 
2.17.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 01/11] xattr: break delegations in {set,remove}xattr and add _locked versions
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 02/11] xattr: add a function to check if a namespace is supported Frank van der Linden
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs
  Cc: Frank van der Linden, linux-fsdevel, Al Viro

set/removexattr on an exported filesystem should break NFS delegations.
This is true in general, but also for the upcoming support for
RFC 8726 (NFSv4 extended attribute support). Make sure that they do.

Additonally, they need to grow a _locked variant, since the NFS server code
will call this with i_rwsem held. It needs to do that in fh_lock to be
able to atomically provide the before and after change attributes.

Cc: stable@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/xattr.c            | 84 +++++++++++++++++++++++++++++++++++++++----
 include/linux/xattr.h |  2 ++
 2 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 90dd78f0eb27..f2854570d411 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -204,10 +204,22 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
 	return error;
 }
 
-
+/**
+ * __vfs_setxattr_locked: set an extended attribute while holding the inode
+ * lock
+ *
+ *  @dentry - object to perform setxattr on
+ *  @name - xattr name to set
+ *  @value - value to set @name to
+ *  @size - size of @value
+ *  @flags - flags to pass into filesystem operations
+ *  @delegated_inode - on return, will contain an inode pointer that
+ *  a delegation was broken on, NULL if none.
+ */
 int
-vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
-		size_t size, int flags)
+__vfs_setxattr_locked(struct dentry *dentry, const char *name,
+		const void *value, size_t size, int flags,
+		struct inode **delegated_inode)
 {
 	struct inode *inode = dentry->d_inode;
 	int error;
@@ -216,15 +228,40 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
 	if (error)
 		return error;
 
-	inode_lock(inode);
 	error = security_inode_setxattr(dentry, name, value, size, flags);
 	if (error)
 		goto out;
 
+	error = try_break_deleg(inode, delegated_inode);
+	if (error)
+		goto out;
+
 	error = __vfs_setxattr_noperm(dentry, name, value, size, flags);
 
 out:
+	return error;
+}
+EXPORT_SYMBOL_GPL(__vfs_setxattr_locked);
+
+int
+vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
+		size_t size, int flags)
+{
+	struct inode *inode = dentry->d_inode;
+	struct inode *delegated_inode = NULL;
+	int error;
+
+retry_deleg:
+	inode_lock(inode);
+	error = __vfs_setxattr_locked(dentry, name, value, size, flags,
+	    &delegated_inode);
 	inode_unlock(inode);
+
+	if (delegated_inode) {
+		error = break_deleg_wait(&delegated_inode);
+		if (!error)
+			goto retry_deleg;
+	}
 	return error;
 }
 EXPORT_SYMBOL_GPL(vfs_setxattr);
@@ -378,8 +415,18 @@ __vfs_removexattr(struct dentry *dentry, const char *name)
 }
 EXPORT_SYMBOL(__vfs_removexattr);
 
+/**
+ * __vfs_removexattr_locked: set an extended attribute while holding the inode
+ * lock
+ *
+ *  @dentry - object to perform setxattr on
+ *  @name - name of xattr to remove
+ *  @delegated_inode - on return, will contain an inode pointer that
+ *  a delegation was broken on, NULL if none.
+ */
 int
-vfs_removexattr(struct dentry *dentry, const char *name)
+__vfs_removexattr_locked(struct dentry *dentry, const char *name,
+		struct inode **delegated_inode)
 {
 	struct inode *inode = dentry->d_inode;
 	int error;
@@ -388,11 +435,14 @@ vfs_removexattr(struct dentry *dentry, const char *name)
 	if (error)
 		return error;
 
-	inode_lock(inode);
 	error = security_inode_removexattr(dentry, name);
 	if (error)
 		goto out;
 
+	error = try_break_deleg(inode, delegated_inode);
+	if (error)
+		goto out;
+
 	error = __vfs_removexattr(dentry, name);
 
 	if (!error) {
@@ -401,12 +451,32 @@ vfs_removexattr(struct dentry *dentry, const char *name)
 	}
 
 out:
+	return error;
+}
+EXPORT_SYMBOL_GPL(__vfs_removexattr_locked);
+
+int
+vfs_removexattr(struct dentry *dentry, const char *name)
+{
+	struct inode *inode = dentry->d_inode;
+	struct inode *delegated_inode = NULL;
+	int error;
+
+retry_deleg:
+	inode_lock(inode);
+	error = __vfs_removexattr_locked(dentry, name, &delegated_inode);
 	inode_unlock(inode);
+
+	if (delegated_inode) {
+		error = break_deleg_wait(&delegated_inode);
+		if (!error)
+			goto retry_deleg;
+	}
+
 	return error;
 }
 EXPORT_SYMBOL_GPL(vfs_removexattr);
 
-
 /*
  * Extended attribute SET operations
  */
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 6dad031be3c2..3a71ad716da5 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -51,8 +51,10 @@ ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
 ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
 int __vfs_setxattr(struct dentry *, struct inode *, const char *, const void *, size_t, int);
 int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int);
+int __vfs_setxattr_locked(struct dentry *, const char *, const void *, size_t, int, struct inode **);
 int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
 int __vfs_removexattr(struct dentry *, const char *);
+int __vfs_removexattr_locked(struct dentry *, const char *, struct inode **);
 int vfs_removexattr(struct dentry *, const char *);
 
 ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size);
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 02/11] xattr: add a function to check if a namespace is supported
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 01/11] xattr: break delegations in {set,remove}xattr and add _locked versions Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 03/11] nfs,nfsd: NFSv4.2 extended attribute protocol definitions Frank van der Linden
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs
  Cc: Frank van der Linden, linux-fsdevel, Al Viro

Add a function that checks is an extended attribute namespace is
supported for an inode.

To be used by the nfs server code when being queried for extended
attributes support.

Cc: linux-fsdevel@vger.kernel.org
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/xattr.c            | 27 +++++++++++++++++++++++++++
 include/linux/xattr.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/fs/xattr.c b/fs/xattr.c
index f2854570d411..1b079550a96d 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -134,6 +134,33 @@ xattr_permission(struct inode *inode, const char *name, int mask)
 	return inode_permission(inode, mask);
 }
 
+/*
+ * Look for any handler that deals with the specified namespace.
+ */
+int
+xattr_supported_namespace(struct inode *inode, const char *prefix)
+{
+	const struct xattr_handler **handlers = inode->i_sb->s_xattr;
+	const struct xattr_handler *handler;
+	size_t preflen;
+
+	if (!(inode->i_opflags & IOP_XATTR)) {
+		if (unlikely(is_bad_inode(inode)))
+			return -EIO;
+		return -EOPNOTSUPP;
+	}
+
+	preflen = strlen(prefix);
+
+	for_each_xattr_handler(handlers, handler) {
+		if (!strncmp(xattr_prefix(handler), prefix, preflen))
+			return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(xattr_supported_namespace);
+
 int
 __vfs_setxattr(struct dentry *dentry, struct inode *inode, const char *name,
 	       const void *value, size_t size, int flags)
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 3a71ad716da5..32e377602068 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -61,6 +61,8 @@ ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_siz
 ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name,
 			   char **xattr_value, size_t size, gfp_t flags);
 
+int xattr_supported_namespace(struct inode *inode, const char *prefix);
+
 static inline const char *xattr_prefix(const struct xattr_handler *handler)
 {
 	return handler->prefix ?: handler->name;
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 03/11] nfs,nfsd: NFSv4.2 extended attribute protocol definitions
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 01/11] xattr: break delegations in {set,remove}xattr and add _locked versions Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 02/11] xattr: add a function to check if a namespace is supported Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 04/11] nfsd: split off the write decode code in to a separate function Frank van der Linden
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

Add definitions for the new operations, errors and flags as defined
in RFC 8276 (File System Extended Attributes in NFSv4).

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 include/linux/nfs4.h      | 20 ++++++++++++++++++++
 include/uapi/linux/nfs4.h |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 82d8fb422092..350aeda0c48c 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -150,6 +150,12 @@ enum nfs_opnum4 {
 	OP_WRITE_SAME = 70,
 	OP_CLONE = 71,
 
+	/* xattr support (RFC8726) */
+	OP_GETXATTR                = 72,
+	OP_SETXATTR                = 73,
+	OP_LISTXATTRS              = 74,
+	OP_REMOVEXATTR             = 75,
+
 	OP_ILLEGAL = 10044,
 };
 
@@ -280,6 +286,10 @@ enum nfsstat4 {
 	NFS4ERR_WRONG_LFS = 10092,
 	NFS4ERR_BADLABEL = 10093,
 	NFS4ERR_OFFLOAD_NO_REQS = 10094,
+
+	/* xattr (RFC8276) */
+	NFS4ERR_NOXATTR        = 10095,
+	NFS4ERR_XATTR2BIG      = 10096,
 };
 
 static inline bool seqid_mutating_err(u32 err)
@@ -452,6 +462,7 @@ enum change_attr_type4 {
 #define FATTR4_WORD2_CHANGE_ATTR_TYPE	(1UL << 15)
 #define FATTR4_WORD2_SECURITY_LABEL     (1UL << 16)
 #define FATTR4_WORD2_MODE_UMASK		(1UL << 17)
+#define FATTR4_WORD2_XATTR_SUPPORT	(1UL << 18)
 
 /* MDS threshold bitmap bits */
 #define THRESHOLD_RD                    (1UL << 0)
@@ -700,4 +711,13 @@ struct nl4_server {
 		struct nfs42_netaddr	nl4_addr; /* NL4_NETADDR */
 	} u;
 };
+
+/*
+ * Options for setxattr. These match the flags for setxattr(2).
+ */
+enum nfs4_setxattr_options {
+	SETXATTR4_EITHER	= 0,
+	SETXATTR4_CREATE	= 1,
+	SETXATTR4_REPLACE	= 2,
+};
 #endif
diff --git a/include/uapi/linux/nfs4.h b/include/uapi/linux/nfs4.h
index 8572930cf5b0..bf197e99b98f 100644
--- a/include/uapi/linux/nfs4.h
+++ b/include/uapi/linux/nfs4.h
@@ -33,6 +33,9 @@
 #define NFS4_ACCESS_EXTEND      0x0008
 #define NFS4_ACCESS_DELETE      0x0010
 #define NFS4_ACCESS_EXECUTE     0x0020
+#define NFS4_ACCESS_XAREAD      0x0040
+#define NFS4_ACCESS_XAWRITE     0x0080
+#define NFS4_ACCESS_XALIST      0x0100
 
 #define NFS4_FH_PERSISTENT		0x0000
 #define NFS4_FH_NOEXPIRE_WITH_OPEN	0x0001
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 04/11] nfsd: split off the write decode code in to a separate function
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
                   ` (2 preceding siblings ...)
  2020-03-27 23:27 ` [PATCH v2 03/11] nfs,nfsd: NFSv4.2 extended attribute protocol definitions Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 05/11] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

nfs4_decode_write has code to parse incoming XDR write data in to
a kvec head, and a list of pages.

Put this code in to a separate function, so that it can be used
later by the xattr code, for setxattr. No functional change.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4xdr.c | 72 +++++++++++++++++++++++++++--------------------
 1 file changed, 42 insertions(+), 30 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9761512674a0..6e7fc6a9931e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -257,6 +257,44 @@ svcxdr_dupstr(struct nfsd4_compoundargs *argp, void *buf, u32 len)
 	return p;
 }
 
+static __be32
+svcxdr_construct_vector(struct nfsd4_compoundargs *argp, struct kvec *head,
+			struct page ***pagelist, u32 buflen)
+{
+	int avail;
+	int len;
+	int pages;
+
+	/* Sorry .. no magic macros for this.. *
+	 * READ_BUF(write->wr_buflen);
+	 * SAVEMEM(write->wr_buf, write->wr_buflen);
+	 */
+	avail = (char *)argp->end - (char *)argp->p;
+	if (avail + argp->pagelen < buflen) {
+		dprintk("NFSD: xdr error (%s:%d)\n",
+			       __FILE__, __LINE__);
+		return nfserr_bad_xdr;
+	}
+	head->iov_base = argp->p;
+	head->iov_len = avail;
+	*pagelist = argp->pagelist;
+
+	len = XDR_QUADLEN(buflen) << 2;
+	if (len >= avail) {
+		len -= avail;
+
+		pages = len >> PAGE_SHIFT;
+		argp->pagelist += pages;
+		argp->pagelen -= pages * PAGE_SIZE;
+		len -= pages * PAGE_SIZE;
+
+		next_decode_page(argp);
+	}
+	argp->p += XDR_QUADLEN(len);
+
+	return 0;
+}
+
 /**
  * savemem - duplicate a chunk of memory for later processing
  * @argp: NFSv4 compound argument structure to be freed with
@@ -1265,8 +1303,6 @@ nfsd4_decode_verify(struct nfsd4_compoundargs *argp, struct nfsd4_verify *verify
 static __be32
 nfsd4_decode_write(struct nfsd4_compoundargs *argp, struct nfsd4_write *write)
 {
-	int avail;
-	int len;
 	DECODE_HEAD;
 
 	status = nfsd4_decode_stateid(argp, &write->wr_stateid);
@@ -1279,34 +1315,10 @@ nfsd4_decode_write(struct nfsd4_compoundargs *argp, struct nfsd4_write *write)
 		goto xdr_error;
 	write->wr_buflen = be32_to_cpup(p++);
 
-	/* Sorry .. no magic macros for this.. *
-	 * READ_BUF(write->wr_buflen);
-	 * SAVEMEM(write->wr_buf, write->wr_buflen);
-	 */
-	avail = (char*)argp->end - (char*)argp->p;
-	if (avail + argp->pagelen < write->wr_buflen) {
-		dprintk("NFSD: xdr error (%s:%d)\n",
-				__FILE__, __LINE__);
-		goto xdr_error;
-	}
-	write->wr_head.iov_base = p;
-	write->wr_head.iov_len = avail;
-	write->wr_pagelist = argp->pagelist;
-
-	len = XDR_QUADLEN(write->wr_buflen) << 2;
-	if (len >= avail) {
-		int pages;
-
-		len -= avail;
-
-		pages = len >> PAGE_SHIFT;
-		argp->pagelist += pages;
-		argp->pagelen -= pages * PAGE_SIZE;
-		len -= pages * PAGE_SIZE;
-
-		next_decode_page(argp);
-	}
-	argp->p += XDR_QUADLEN(len);
+	status = svcxdr_construct_vector(argp, &write->wr_head,
+					 &write->wr_pagelist, write->wr_buflen);
+	if (status)
+		return status;
 
 	DECODE_TAIL;
 }
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 05/11] nfsd: add defines for NFSv4.2 extended attribute support
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
                   ` (3 preceding siblings ...)
  2020-03-27 23:27 ` [PATCH v2 04/11] nfsd: split off the write decode code in to a separate function Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 06/11] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

Add defines for server-side extended attribute support. Most have
already been added as part of client support, but these are
the network order error codes for the noxattr and xattr2big errors,
and the addition of the xattr support to the supported file
attributes (if configured).

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfsd.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 2ab5569126b8..362d481b28c9 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -281,6 +281,8 @@ void		nfsd_lockd_shutdown(void);
 #define nfserr_wrong_lfs		cpu_to_be32(NFS4ERR_WRONG_LFS)
 #define nfserr_badlabel			cpu_to_be32(NFS4ERR_BADLABEL)
 #define nfserr_file_open		cpu_to_be32(NFS4ERR_FILE_OPEN)
+#define nfserr_xattr2big		cpu_to_be32(NFS4ERR_XATTR2BIG)
+#define nfserr_noxattr			cpu_to_be32(NFS4ERR_NOXATTR)
 
 /* error codes for internal use */
 /* if a request fails due to kmalloc failure, it gets dropped.
@@ -382,7 +384,8 @@ void		nfsd_lockd_shutdown(void);
 	(NFSD4_1_SUPPORTED_ATTRS_WORD2 | \
 	FATTR4_WORD2_CHANGE_ATTR_TYPE | \
 	FATTR4_WORD2_MODE_UMASK | \
-	NFSD4_2_SECURITY_ATTRS)
+	NFSD4_2_SECURITY_ATTRS | \
+	FATTR4_WORD2_XATTR_SUPPORT)
 
 extern const u32 nfsd_suppattrs[3][3];
 
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 06/11] nfsd: define xattr functions to call in to their vfs counterparts
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
                   ` (4 preceding siblings ...)
  2020-03-27 23:27 ` [PATCH v2 05/11] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 07/11] nfsd: take xattr bits in to account for permission checks Frank van der Linden
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

This adds the filehandle based functions for the xattr operations
that call in to the vfs layer to do the actual work.

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.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 07/11] nfsd: take xattr bits in to account for permission checks
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
                   ` (5 preceding siblings ...)
  2020-03-27 23:27 ` [PATCH v2 06/11] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 08/11] nfsd: add structure definitions for xattr requests / responses Frank van der Linden
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

Since the NFSv4.2 extended attributes extension defines 3 new access
bits for xattr operations, take them in to account when validating
what the client is asking for, and when checking permissions.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4proc.c |  8 +++++++-
 fs/nfsd/vfs.c      | 12 ++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 0e75f7fb5fec..ee317ae0b609 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -566,8 +566,14 @@ nfsd4_access(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	     union nfsd4_op_u *u)
 {
 	struct nfsd4_access *access = &u->access;
+	u32 access_full;
 
-	if (access->ac_req_access & ~NFS3_ACCESS_FULL)
+	access_full = NFS3_ACCESS_FULL;
+	if (cstate->minorversion >= 2)
+		access_full |= NFS4_ACCESS_XALIST | NFS4_ACCESS_XAREAD |
+			       NFS4_ACCESS_XAWRITE;
+
+	if (access->ac_req_access & ~access_full)
 		return nfserr_inval;
 
 	access->ac_resp_access = access->ac_req_access;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 115449009bc0..19608e690069 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -612,6 +612,12 @@ static struct accessmap	nfs3_regaccess[] = {
     {	NFS3_ACCESS_MODIFY,	NFSD_MAY_WRITE|NFSD_MAY_TRUNC	},
     {	NFS3_ACCESS_EXTEND,	NFSD_MAY_WRITE			},
 
+#ifdef CONFIG_NFSD_V4
+    {	NFS4_ACCESS_XAREAD,	NFSD_MAY_READ			},
+    {	NFS4_ACCESS_XAWRITE,	NFSD_MAY_WRITE			},
+    {	NFS4_ACCESS_XALIST,	NFSD_MAY_READ			},
+#endif
+
     {	0,			0				}
 };
 
@@ -622,6 +628,12 @@ static struct accessmap	nfs3_diraccess[] = {
     {	NFS3_ACCESS_EXTEND,	NFSD_MAY_EXEC|NFSD_MAY_WRITE	},
     {	NFS3_ACCESS_DELETE,	NFSD_MAY_REMOVE			},
 
+#ifdef CONFIG_NFSD_V4
+    {	NFS4_ACCESS_XAREAD,	NFSD_MAY_READ			},
+    {	NFS4_ACCESS_XAWRITE,	NFSD_MAY_WRITE			},
+    {	NFS4_ACCESS_XALIST,	NFSD_MAY_READ			},
+#endif
+
     {	0,			0				}
 };
 
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 08/11] nfsd: add structure definitions for xattr requests / responses
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
                   ` (6 preceding siblings ...)
  2020-03-27 23:27 ` [PATCH v2 07/11] nfsd: take xattr bits in to account for permission checks Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 09/11] nfsd: use kvmalloc in svcxdr_tmpalloc Frank van der Linden
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

Add the structures used in extended attribute request / response handling.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/xdr4.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index db63d39b1507..66499fb6b567 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -224,6 +224,32 @@ struct nfsd4_putfh {
 	bool		no_verify;	    /* represents foreigh fh */
 };
 
+struct nfsd4_getxattr {
+	char		*getxa_name;		/* request */
+	u32		getxa_len;		/* request */
+	void		*getxa_buf;
+};
+
+struct nfsd4_setxattr {
+	u32		setxa_flags;		/* request */
+	char		*setxa_name;		/* request */
+	char		*setxa_buf;		/* request */
+	u32		setxa_len;		/* request */
+	struct nfsd4_change_info  setxa_cinfo;	/* response */
+};
+
+struct nfsd4_removexattr {
+	char		*rmxa_name;		/* request */
+	struct nfsd4_change_info  rmxa_cinfo;	/* response */
+};
+
+struct nfsd4_listxattrs {
+	u64		lsxa_cookie;		/* request */
+	u32		lsxa_maxcount;		/* request */
+	char		*lsxa_buf;		/* unfiltered buffer (reply) */
+	u32		lsxa_len;		/* unfiltered len (reply) */
+};
+
 struct nfsd4_open {
 	u32		op_claim_type;      /* request */
 	struct xdr_netobj op_fname;	    /* request - everything but CLAIM_PREV */
@@ -649,6 +675,11 @@ struct nfsd4_op {
 		struct nfsd4_offload_status	offload_status;
 		struct nfsd4_copy_notify	copy_notify;
 		struct nfsd4_seek		seek;
+
+		struct nfsd4_getxattr		getxattr;
+		struct nfsd4_setxattr		setxattr;
+		struct nfsd4_listxattrs		listxattrs;
+		struct nfsd4_removexattr	removexattr;
 	} u;
 	struct nfs4_replay *			replay;
 };
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 09/11] nfsd: use kvmalloc in svcxdr_tmpalloc
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
                   ` (7 preceding siblings ...)
  2020-03-27 23:27 ` [PATCH v2 08/11] nfsd: add structure definitions for xattr requests / responses Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 10/11] nfsd: implement the xattr functions and en/decode logic Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 11/11] nfsd: add fattr support for user extended attributes Frank van der Linden
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

For user extended attributes, temp allocations larger than one page
might be needed.

Use kvmalloc/kvfree to accommodate these larger allocations, while not
affecting the performance of the smaller ones.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4xdr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 6e7fc6a9931e..f6322add2992 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -230,7 +230,7 @@ svcxdr_tmpalloc(struct nfsd4_compoundargs *argp, u32 len)
 {
 	struct svcxdr_tmpbuf *tb;
 
-	tb = kmalloc(sizeof(*tb) + len, GFP_KERNEL);
+	tb = kvmalloc(sizeof(*tb) + len, GFP_KERNEL);
 	if (!tb)
 		return NULL;
 	tb->next = argp->to_free;
@@ -4691,7 +4691,7 @@ void nfsd4_release_compoundargs(struct svc_rqst *rqstp)
 	while (args->to_free) {
 		struct svcxdr_tmpbuf *tb = args->to_free;
 		args->to_free = tb->next;
-		kfree(tb);
+		kvfree(tb);
 	}
 }
 
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 10/11] nfsd: implement the xattr functions and en/decode logic
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
                   ` (8 preceding siblings ...)
  2020-03-27 23:27 ` [PATCH v2 09/11] nfsd: use kvmalloc in svcxdr_tmpalloc Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  2020-03-29 20:25   ` vfs_listxattr and the NFS server: namespaces Frank van der Linden
  2020-03-27 23:27 ` [PATCH v2 11/11] nfsd: add fattr support for user extended attributes Frank van der Linden
  10 siblings, 1 reply; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden, linux-fsdevel

Implement the main entry points for the *XATTR operations.

Add functions to calculate the reply size for the user extended attribute
operations, and implement the XDR encode / decode logic for these
operations.

Add the user extended attributes operations to nfsd4_ops.

Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4proc.c   | 132 +++++++++++++
 fs/nfsd/nfs4xdr.c    | 450 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs4.h |   2 +-
 3 files changed, 583 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index ee317ae0b609..bb00982f5c6a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2098,6 +2098,80 @@ nfsd4_layoutreturn(struct svc_rqst *rqstp,
 }
 #endif /* CONFIG_NFSD_PNFS */
 
+static __be32
+nfsd4_getxattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	       union nfsd4_op_u *u)
+{
+	struct nfsd4_getxattr *getxattr = &u->getxattr;
+
+	return nfsd_getxattr(rqstp, &cstate->current_fh,
+			     getxattr->getxa_name, getxattr->getxa_buf,
+			     &getxattr->getxa_len);
+}
+
+static __be32
+nfsd4_setxattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	   union nfsd4_op_u *u)
+{
+	struct nfsd4_setxattr *setxattr = &u->setxattr;
+	__be32 ret;
+
+	if (opens_in_grace(SVC_NET(rqstp)))
+		return nfserr_grace;
+
+	ret = nfsd_setxattr(rqstp, &cstate->current_fh, setxattr->setxa_name,
+			    setxattr->setxa_buf, setxattr->setxa_len,
+			    setxattr->setxa_flags);
+
+	if (!ret)
+		set_change_info(&setxattr->setxa_cinfo, &cstate->current_fh);
+
+	return ret;
+}
+
+static __be32
+nfsd4_listxattrs(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	   union nfsd4_op_u *u)
+{
+	int len;
+	__be32 ret;
+
+	/*
+	 * Get the entire list, then copy out only the user attributes
+	 * in the encode function. lsxa_buf was previously allocated as
+	 * tmp svc space, and will be automatically freed later.
+	 */
+	len = XATTR_LIST_MAX;
+
+	ret = nfsd_listxattr(rqstp, &cstate->current_fh, u->listxattrs.lsxa_buf,
+			     &len);
+	if (ret)
+		return ret;
+
+	u->listxattrs.lsxa_len = len;
+
+	return nfs_ok;
+}
+
+static __be32
+nfsd4_removexattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	   union nfsd4_op_u *u)
+{
+	struct nfsd4_removexattr *removexattr = &u->removexattr;
+	__be32 ret;
+
+	if (opens_in_grace(SVC_NET(rqstp)))
+		return nfserr_grace;
+
+	ret = nfsd_removexattr(rqstp, &cstate->current_fh,
+	    removexattr->rmxa_name);
+
+	if (!ret)
+		set_change_info(&removexattr->rmxa_cinfo, &cstate->current_fh);
+
+	return ret;
+}
+
 /*
  * NULL call.
  */
@@ -2705,6 +2779,42 @@ static inline u32 nfsd4_seek_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
 	return (op_encode_hdr_size + 3) * sizeof(__be32);
 }
 
+static inline u32 nfsd4_getxattr_rsize(struct svc_rqst *rqstp,
+				       struct nfsd4_op *op)
+{
+	u32 maxcount, rlen;
+
+	maxcount = svc_max_payload(rqstp);
+	rlen = min_t(u32, XATTR_SIZE_MAX, maxcount);
+
+	return (op_encode_hdr_size + 1 + XDR_QUADLEN(rlen)) * sizeof(__be32);
+}
+
+static inline u32 nfsd4_setxattr_rsize(struct svc_rqst *rqstp,
+				       struct nfsd4_op *op)
+{
+	return (op_encode_hdr_size + op_encode_change_info_maxsz)
+		* sizeof(__be32);
+}
+static inline u32 nfsd4_listxattrs_rsize(struct svc_rqst *rqstp,
+					 struct nfsd4_op *op)
+{
+	u32 maxcount, rlen;
+
+	maxcount = svc_max_payload(rqstp);
+	rlen = min(op->u.listxattrs.lsxa_maxcount, maxcount);
+
+	return (op_encode_hdr_size + 4 + XDR_QUADLEN(rlen)) * sizeof(__be32);
+}
+
+static inline u32 nfsd4_removexattr_rsize(struct svc_rqst *rqstp,
+					  struct nfsd4_op *op)
+{
+	return (op_encode_hdr_size + op_encode_change_info_maxsz)
+		* sizeof(__be32);
+}
+
+
 static const struct nfsd4_operation nfsd4_ops[] = {
 	[OP_ACCESS] = {
 		.op_func = nfsd4_access,
@@ -3086,6 +3196,28 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_name = "OP_COPY_NOTIFY",
 		.op_rsize_bop = nfsd4_copy_notify_rsize,
 	},
+	[OP_GETXATTR] = {
+		.op_func = nfsd4_getxattr,
+		.op_name = "OP_GETXATTR",
+		.op_rsize_bop = nfsd4_getxattr_rsize,
+	},
+	[OP_SETXATTR] = {
+		.op_func = nfsd4_setxattr,
+		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+		.op_name = "OP_SETXATTR",
+		.op_rsize_bop = nfsd4_setxattr_rsize,
+	},
+	[OP_LISTXATTRS] = {
+		.op_func = nfsd4_listxattrs,
+		.op_name = "OP_LISTXATTRS",
+		.op_rsize_bop = nfsd4_listxattrs_rsize,
+	},
+	[OP_REMOVEXATTR] = {
+		.op_func = nfsd4_removexattr,
+		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+		.op_name = "OP_REMOVEXATTR",
+		.op_rsize_bop = nfsd4_removexattr_rsize,
+	},
 };
 
 /**
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f6322add2992..4c4508f1bb8e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -41,6 +41,8 @@
 #include <linux/pagemap.h>
 #include <linux/sunrpc/svcauth_gss.h>
 #include <linux/sunrpc/addr.h>
+#include <linux/xattr.h>
+#include <uapi/linux/xattr.h>
 
 #include "idmap.h"
 #include "acl.h"
@@ -1877,6 +1879,224 @@ nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
 	DECODE_TAIL;
 }
 
+/*
+ * XDR data that is more than PAGE_SIZE in size is normally part of a
+ * read or write. However, the size of extended attributes is limited
+ * by the maximum request size, and then further limited by the underlying
+ * filesystem limits. This can exceed PAGE_SIZE (currently, XATTR_SIZE_MAX
+ * is 64k). Since there is no kvec- or page-based interface to xattrs,
+ * and we're not dealing with contiguous pages, we need to do some copying.
+ */
+
+/*
+ * Decode data into buffer. Uses head and pages constructed by
+ * svcxdr_construct_vector.
+ */
+static __be32
+nfsd4_vbuf_from_vector(struct nfsd4_compoundargs *argp, struct kvec *head,
+		       struct page **pages, char **bufp, u32 buflen)
+{
+	char *tmp, *dp;
+	u32 len;
+
+	if (buflen <= head->iov_len) {
+		/*
+		 * We're in luck, the head has enough space. Just return
+		 * the head, no need for copying.
+		 */
+		*bufp = head->iov_base;
+		return 0;
+	}
+
+	tmp = svcxdr_tmpalloc(argp, buflen);
+	if (tmp == NULL)
+		return nfserr_jukebox;
+
+	dp = tmp;
+	memcpy(dp, head->iov_base, head->iov_len);
+	buflen -= head->iov_len;
+	dp += head->iov_len;
+
+	while (buflen > 0) {
+		len = min_t(u32, buflen, PAGE_SIZE);
+		memcpy(dp, page_address(*pages), len);
+
+		buflen -= len;
+		dp += len;
+		pages++;
+	}
+
+	*bufp = tmp;
+	return 0;
+}
+
+/*
+ * Get a user extended attribute name from the XDR buffer.
+ * It will not have the "user." prefix, so prepend it.
+ * Lastly, check for nul characters in the name.
+ */
+static __be32
+nfsd4_decode_xattr_name(struct nfsd4_compoundargs *argp, char **namep)
+{
+	DECODE_HEAD;
+	char *name, *sp, *dp;
+	u32 namelen, cnt;
+
+	READ_BUF(4);
+	namelen = be32_to_cpup(p++);
+
+	if (namelen > (XATTR_NAME_MAX - XATTR_USER_PREFIX_LEN))
+		return nfserr_nametoolong;
+
+	if (namelen == 0)
+		goto xdr_error;
+
+	READ_BUF(namelen);
+
+	name = svcxdr_tmpalloc(argp, namelen + XATTR_USER_PREFIX_LEN + 1);
+	if (!name)
+		return nfserr_jukebox;
+
+	memcpy(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN);
+
+	/*
+	 * Copy the extended attribute name over while checking for 0
+	 * characters.
+	 */
+	sp = (char *)p;
+	dp = name + XATTR_USER_PREFIX_LEN;
+	cnt = namelen;
+
+	while (cnt-- > 0) {
+		if (*sp == '\0')
+			goto xdr_error;
+		*dp++ = *sp++;
+	}
+	*dp = '\0';
+
+	*namep = name;
+
+	DECODE_TAIL;
+}
+
+/*
+ * A GETXATTR op request comes without a length specifier. We just set the
+ * maximum length for the reply based on XATTR_SIZE_MAX and the maximum
+ * channel reply size, allocate a buffer of that length and pass it to
+ * vfs_getxattr.
+ */
+static __be32
+nfsd4_decode_getxattr(struct nfsd4_compoundargs *argp,
+		      struct nfsd4_getxattr *getxattr)
+{
+	__be32 status;
+	u32 maxcount;
+
+	status = nfsd4_decode_xattr_name(argp, &getxattr->getxa_name);
+	if (status)
+		return status;
+
+	maxcount = svc_max_payload(argp->rqstp);
+	maxcount = min_t(u32, XATTR_SIZE_MAX, maxcount);
+
+	getxattr->getxa_buf = svcxdr_tmpalloc(argp, maxcount);
+	if (!getxattr->getxa_buf)
+		status = nfserr_jukebox;
+	getxattr->getxa_len = maxcount;
+
+	return status;
+}
+
+static __be32
+nfsd4_decode_setxattr(struct nfsd4_compoundargs *argp,
+		      struct nfsd4_setxattr *setxattr)
+{
+	DECODE_HEAD;
+	u32 flags, maxcount, size;
+	struct kvec head;
+	struct page **pagelist;
+
+	READ_BUF(4);
+	flags = be32_to_cpup(p++);
+
+	if (flags > SETXATTR4_REPLACE)
+		return nfserr_inval;
+	setxattr->setxa_flags = flags;
+
+	status = nfsd4_decode_xattr_name(argp, &setxattr->setxa_name);
+	if (status)
+		return status;
+
+	maxcount = svc_max_payload(argp->rqstp);
+	maxcount = min_t(u32, XATTR_SIZE_MAX, maxcount);
+
+	READ_BUF(4);
+	size = be32_to_cpup(p++);
+	if (size > maxcount)
+		return nfserr_xattr2big;
+
+	setxattr->setxa_len = size;
+	if (size > 0) {
+		status = svcxdr_construct_vector(argp, &head, &pagelist, size);
+		if (status)
+			return status;
+
+		status = nfsd4_vbuf_from_vector(argp, &head, pagelist,
+		    &setxattr->setxa_buf, size);
+	}
+
+	DECODE_TAIL;
+}
+
+static __be32
+nfsd4_decode_listxattrs(struct nfsd4_compoundargs *argp,
+			struct nfsd4_listxattrs *listxattrs)
+{
+	DECODE_HEAD;
+	u32 maxcount;
+
+	READ_BUF(12);
+	p = xdr_decode_hyper(p, &listxattrs->lsxa_cookie);
+
+	/*
+	 * If the cookie  is too large to have even one user.x attribute
+	 * plus trailing '\0' left in a maximum size buffer, it's invalid.
+	 */
+	if (listxattrs->lsxa_cookie >=
+	    (XATTR_LIST_MAX / (XATTR_USER_PREFIX_LEN + 2)))
+		return nfserr_badcookie;
+
+	maxcount = be32_to_cpup(p++);
+	if (maxcount < 8)
+		/* Always need at least 2 words (length and one character) */
+		return nfserr_inval;
+
+	maxcount = min(maxcount, svc_max_payload(argp->rqstp));
+	listxattrs->lsxa_maxcount = maxcount;
+
+	/*
+	 * Unfortunately, there is no interface to only list xattrs for
+	 * one prefix. So there is no good way to convert maxcount to
+	 * a maximum value to pass to vfs_listxattr, as we don't know
+	 * how many of the returned attributes will be user attributes.
+	 *
+	 * So, always ask vfs_listxattr for the maximum size, and encode
+	 * as many as possible.
+	 */
+	listxattrs->lsxa_buf = svcxdr_tmpalloc(argp, XATTR_LIST_MAX);
+	if (!listxattrs->lsxa_buf)
+		status = nfserr_jukebox;
+
+	DECODE_TAIL;
+}
+
+static __be32
+nfsd4_decode_removexattr(struct nfsd4_compoundargs *argp,
+			 struct nfsd4_removexattr *removexattr)
+{
+	return nfsd4_decode_xattr_name(argp, &removexattr->rmxa_name);
+}
+
 static __be32
 nfsd4_decode_noop(struct nfsd4_compoundargs *argp, void *p)
 {
@@ -1973,6 +2193,11 @@ static const nfsd4_dec nfsd4_dec_ops[] = {
 	[OP_SEEK]		= (nfsd4_dec)nfsd4_decode_seek,
 	[OP_WRITE_SAME]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_CLONE]		= (nfsd4_dec)nfsd4_decode_clone,
+	/* RFC 8276 extended atributes operations */
+	[OP_GETXATTR]		= (nfsd4_dec)nfsd4_decode_getxattr,
+	[OP_SETXATTR]		= (nfsd4_dec)nfsd4_decode_setxattr,
+	[OP_LISTXATTRS]		= (nfsd4_dec)nfsd4_decode_listxattrs,
+	[OP_REMOVEXATTR]	= (nfsd4_dec)nfsd4_decode_removexattr,
 };
 
 static inline bool
@@ -4456,6 +4681,225 @@ nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
 	return nfserr;
 }
 
+/*
+ * Encode kmalloc-ed buffer in to XDR stream.
+ */
+static int
+nfsd4_vbuf_to_stream(struct xdr_stream *xdr, char *buf, u32 buflen)
+{
+	u32 cplen;
+	__be32 *p;
+
+	cplen = min_t(unsigned long, buflen,
+		      ((void *)xdr->end - (void *)xdr->p));
+	p = xdr_reserve_space(xdr, cplen);
+	if (!p)
+		return nfserr_resource;
+
+	memcpy(p, buf, cplen);
+	buf += cplen;
+	buflen -= cplen;
+
+	while (buflen) {
+		cplen = min_t(u32, buflen, PAGE_SIZE);
+		p = xdr_reserve_space(xdr, cplen);
+		if (!p)
+			return nfserr_resource;
+
+		memcpy(p, buf, cplen);
+
+		if (cplen < PAGE_SIZE) {
+			/*
+			 * We're done, with a length that wasn't page
+			 * aligned, so possibly not word aligned. Pad
+			 * any trailing bytes with 0.
+			 */
+			xdr_encode_opaque_fixed(p, NULL, cplen);
+			break;
+		}
+
+		buflen -= PAGE_SIZE;
+		buf += PAGE_SIZE;
+	}
+
+	return 0;
+}
+
+static __be32
+nfsd4_encode_getxattr(struct nfsd4_compoundres *resp, __be32 nfserr,
+		      struct nfsd4_getxattr *getxattr)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 4);
+	if (!p)
+		return nfserr_resource;
+
+	*p = cpu_to_be32(getxattr->getxa_len);
+
+	if (getxattr->getxa_len == 0)
+		return 0;
+
+	return nfsd4_vbuf_to_stream(xdr, getxattr->getxa_buf,
+				    getxattr->getxa_len);
+}
+
+static __be32
+nfsd4_encode_setxattr(struct nfsd4_compoundres *resp, __be32 nfserr,
+		      struct nfsd4_setxattr *setxattr)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 20);
+	if (!p)
+		return nfserr_resource;
+
+	encode_cinfo(p, &setxattr->setxa_cinfo);
+
+	return 0;
+}
+
+/*
+ * See if there are cookie values that can be rejected outright.
+ */
+static __be32
+nfsd4_listxattr_validate_cookie(struct nfsd4_listxattrs *listxattrs,
+				u32 *offsetp)
+{
+	u64 cookie = listxattrs->lsxa_cookie;
+
+	/*
+	 * If the cookie is larger than the maximum number we can fit
+	 * in either the buffer we just got back from vfs_listxattr, or,
+	 * XDR-encoded, in the return buffer, it's invalid.
+	 */
+	if (cookie > (listxattrs->lsxa_len) / (XATTR_USER_PREFIX_LEN + 2))
+		return nfserr_badcookie;
+
+	if (cookie > (listxattrs->lsxa_maxcount /
+		      (XDR_QUADLEN(XATTR_USER_PREFIX_LEN + 2) + 4)))
+		return nfserr_badcookie;
+
+	*offsetp = (u32)cookie;
+	return 0;
+}
+
+static __be32
+nfsd4_encode_listxattrs(struct nfsd4_compoundres *resp, __be32 nfserr,
+			struct nfsd4_listxattrs *listxattrs)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	u32 cookie_offset, count_offset, eof;
+	u32 left, xdrleft, slen, count;
+	u32 xdrlen, offset;
+	u64 cookie;
+	char *sp;
+	__be32 status;
+	__be32 *p;
+	u32 nuser;
+
+	eof = 1;
+
+	status = nfsd4_listxattr_validate_cookie(listxattrs, &offset);
+	if (status)
+		return status;
+
+	/*
+	 * Reserve space for the cookie and the name array count. Record
+	 * the offsets to save them later.
+	 */
+	cookie_offset = xdr->buf->len;
+	count_offset = cookie_offset + 8;
+	p = xdr_reserve_space(xdr, 12);
+	if (!p)
+		return nfserr_resource;
+
+	count = 0;
+	left = listxattrs->lsxa_len;
+	sp = listxattrs->lsxa_buf;
+	nuser = 0;
+
+	xdrleft = listxattrs->lsxa_maxcount;
+
+	while (left > 0 && xdrleft > 0) {
+		slen = strlen(sp);
+
+		/*
+		 * Check if this a user. attribute, skip it if not.
+		 */
+		if (strncmp(sp, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
+			goto contloop;
+
+		slen -= XATTR_USER_PREFIX_LEN;
+		xdrlen = 4 + ((slen + 3) & ~3);
+		if (xdrlen > xdrleft) {
+			if (count == 0) {
+				/*
+				 * Can't even fit the first attribute name.
+				 */
+				return nfserr_toosmall;
+			}
+			eof = 0;
+			goto wreof;
+		}
+
+		left -= XATTR_USER_PREFIX_LEN;
+		sp += XATTR_USER_PREFIX_LEN;
+		if (nuser++ < offset)
+			goto contloop;
+
+
+		p = xdr_reserve_space(xdr, xdrlen);
+		if (!p)
+			return nfserr_resource;
+
+		p = xdr_encode_opaque(p, sp, slen);
+
+		xdrleft -= xdrlen;
+		count++;
+contloop:
+		sp += slen + 1;
+		left -= slen + 1;
+	}
+
+	/*
+	 * If there were user attributes to copy, but we didn't copy
+	 * any, the offset was too large (e.g. the cookie was invalid).
+	 */
+	if (nuser > 0 && count == 0)
+		return nfserr_badcookie;
+
+wreof:
+	p = xdr_reserve_space(xdr, 4);
+	if (!p)
+		return nfserr_resource;
+	*p = cpu_to_be32(eof);
+
+	cookie = offset + count;
+
+	write_bytes_to_xdr_buf(xdr->buf, cookie_offset, &cookie, 8);
+	count = htonl(count);
+	write_bytes_to_xdr_buf(xdr->buf, count_offset, &count, 4);
+	return 0;
+}
+
+static __be32
+nfsd4_encode_removexattr(struct nfsd4_compoundres *resp, __be32 nfserr,
+			 struct nfsd4_removexattr *removexattr)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 20);
+	if (!p)
+		return nfserr_resource;
+
+	p = encode_cinfo(p, &removexattr->rmxa_cinfo);
+	return 0;
+}
+
 typedef __be32(* nfsd4_enc)(struct nfsd4_compoundres *, __be32, void *);
 
 /*
@@ -4545,6 +4989,12 @@ static const nfsd4_enc nfsd4_enc_ops[] = {
 	[OP_SEEK]		= (nfsd4_enc)nfsd4_encode_seek,
 	[OP_WRITE_SAME]		= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_CLONE]		= (nfsd4_enc)nfsd4_encode_noop,
+
+	/* RFC 8276 extended atributes operations */
+	[OP_GETXATTR]		= (nfsd4_enc)nfsd4_encode_getxattr,
+	[OP_SETXATTR]		= (nfsd4_enc)nfsd4_encode_setxattr,
+	[OP_LISTXATTRS]		= (nfsd4_enc)nfsd4_encode_listxattrs,
+	[OP_REMOVEXATTR]	= (nfsd4_enc)nfsd4_encode_removexattr,
 };
 
 /*
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 350aeda0c48c..cbe50e452f19 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -165,7 +165,7 @@ Needs to be updated if more operations are defined in future.*/
 #define FIRST_NFS4_OP	OP_ACCESS
 #define LAST_NFS40_OP	OP_RELEASE_LOCKOWNER
 #define LAST_NFS41_OP	OP_RECLAIM_COMPLETE
-#define LAST_NFS42_OP	OP_CLONE
+#define LAST_NFS42_OP	OP_REMOVEXATTR
 #define LAST_NFS4_OP	LAST_NFS42_OP
 
 enum nfsstat4 {
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 11/11] nfsd: add fattr support for user extended attributes
  2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
                   ` (9 preceding siblings ...)
  2020-03-27 23:27 ` [PATCH v2 10/11] nfsd: implement the xattr functions and en/decode logic Frank van der Linden
@ 2020-03-27 23:27 ` Frank van der Linden
  10 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-27 23:27 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden

Check if user extended attributes are supported for an inode,
and return the answer when being queried for file attributes.

An exported filesystem can now signal its RFC8276 user extended
attributes capability.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4xdr.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 4c4508f1bb8e..5acf719c0451 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3229,6 +3229,15 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 	}
 #endif
 
+	if (bmval2 & FATTR4_WORD2_XATTR_SUPPORT) {
+		p = xdr_reserve_space(xdr, 4);
+		if (!p)
+			goto out_resource;
+		err = xattr_supported_namespace(d_inode(dentry),
+						XATTR_USER_PREFIX);
+		*p++ = cpu_to_be32(err == 0);
+	}
+
 	attrlen = htonl(xdr->buf->len - attrlen_offset - 4);
 	write_bytes_to_xdr_buf(xdr->buf, attrlen_offset, &attrlen, 4);
 	status = nfs_ok;
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* vfs_listxattr and the NFS server: namespaces
  2020-03-27 23:27 ` [PATCH v2 10/11] nfsd: implement the xattr functions and en/decode logic Frank van der Linden
@ 2020-03-29 20:25   ` Frank van der Linden
  2020-03-29 21:54     ` Chuck Lever
  0 siblings, 1 reply; 15+ messages in thread
From: Frank van der Linden @ 2020-03-29 20:25 UTC (permalink / raw)
  To: bfields, chuck.lever, linux-nfs; +Cc: linux-fsdevel

On Fri, Mar 27, 2020 at 11:27:16PM +0000, Frank van der Linden wrote:
> Implement the main entry points for the *XATTR operations.
> 
> Add functions to calculate the reply size for the user extended attribute
> operations, and implement the XDR encode / decode logic for these
> operations.
> 
> Add the user extended attributes operations to nfsd4_ops.

The reason I Cc-ed this one to linux-fsdevel, is the following piece of
code:
> +	/*
> +	 * Unfortunately, there is no interface to only list xattrs for
> +	 * one prefix. So there is no good way to convert maxcount to
> +	 * a maximum value to pass to vfs_listxattr, as we don't know
> +	 * how many of the returned attributes will be user attributes.
> +	 *
> +	 * So, always ask vfs_listxattr for the maximum size, and encode
> +	 * as many as possible.
> +	 */
> +	listxattrs->lsxa_buf = svcxdr_tmpalloc(argp, XATTR_LIST_MAX);
> +	if (!listxattrs->lsxa_buf)
> +		status = nfserr_jukebox;
> +
> +	DECODE_TAIL;
> +}

Naturally, it is a waste to always (temporarily) allocate XATTR_LIST_MAX
(currently 64k) bytes for every listxattr request, when in reality you
probably won't need anywhere near that.

The reason for doing that is that, while the NFS request comes with a 
maximum size, you can't translate that in to a maximum size to pass
to vfs_listxattr, since you are specifying the max size for "user."
extended attributes only, since that is the namespace that the NFS
implementation is restricted to. But vfs_listxattr always retrieves
all extended attributes.

The question is then: how to avoid doing that? It might be a good
idea to be able to specify a namespace to vfs_listxattr. This isn't
terribly hard to implement, and can be phased in. E.g:

* Add a "const char *namespace" argument to the listxattr inode op
* For the normal use case it will be NULL, meaning all xattrs. This
  includes use in the system call path (a new system call that
  includes a namespace argument could be added).
* A filesystem that does not support only returning xattrs for one
  namespace will return a specified error if @namespace != NULL, after
  which the fs-indepdent code in fs/xattr.c falls back to pre-allocating
  an XATTR_LIST_MAX buffer and then filtering out all attributes that
  are not in the specified namespace.
* Since the initial use is from the NFS code, converting XFS and ext4
  to support the @namespace argument should catch most use cases of
  the @namespace argument, the rest can be converted over time.

Does this sound reasonable? Or is it overkill?

Another way to do it is to add a vfs_listxattr_alloc variant, and
have the NFS server code filter out the user. xattrs as now. There is
one problem with that: it's possible for the size of listxattr result
to increase between the first null (length probe) call and the second call,
in which case the NFS server would return a spurious error. A small chance,
but it's there nonetheless. And it would be hard to distinguish from
other errors. So I think I still would prefer using an extra namespace
argument.

Alternatively, we could decide that the temporarily allocation of 64k
is no big deal.

Thoughts?

- Frank

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: vfs_listxattr and the NFS server: namespaces
  2020-03-29 20:25   ` vfs_listxattr and the NFS server: namespaces Frank van der Linden
@ 2020-03-29 21:54     ` Chuck Lever
  2020-03-29 22:57       ` Frank van der Linden
  0 siblings, 1 reply; 15+ messages in thread
From: Chuck Lever @ 2020-03-29 21:54 UTC (permalink / raw)
  To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List, linux-fsdevel



> On Mar 29, 2020, at 4:25 PM, Frank van der Linden <fllinden@amazon.com> wrote:
> 
> On Fri, Mar 27, 2020 at 11:27:16PM +0000, Frank van der Linden wrote:
>> Implement the main entry points for the *XATTR operations.
>> 
>> Add functions to calculate the reply size for the user extended attribute
>> operations, and implement the XDR encode / decode logic for these
>> operations.
>> 
>> Add the user extended attributes operations to nfsd4_ops.
> 
> The reason I Cc-ed this one to linux-fsdevel, is the following piece of
> code:
>> +	/*
>> +	 * Unfortunately, there is no interface to only list xattrs for
>> +	 * one prefix. So there is no good way to convert maxcount to
>> +	 * a maximum value to pass to vfs_listxattr, as we don't know
>> +	 * how many of the returned attributes will be user attributes.
>> +	 *
>> +	 * So, always ask vfs_listxattr for the maximum size, and encode
>> +	 * as many as possible.
>> +	 */
>> +	listxattrs->lsxa_buf = svcxdr_tmpalloc(argp, XATTR_LIST_MAX);
>> +	if (!listxattrs->lsxa_buf)
>> +		status = nfserr_jukebox;
>> +
>> +	DECODE_TAIL;
>> +}
> 
> Naturally, it is a waste to always (temporarily) allocate XATTR_LIST_MAX
> (currently 64k) bytes for every listxattr request, when in reality you
> probably won't need anywhere near that.
> 
> The reason for doing that is that, while the NFS request comes with a 
> maximum size, you can't translate that in to a maximum size to pass
> to vfs_listxattr, since you are specifying the max size for "user."
> extended attributes only, since that is the namespace that the NFS
> implementation is restricted to. But vfs_listxattr always retrieves
> all extended attributes.
> 
> The question is then: how to avoid doing that? It might be a good
> idea to be able to specify a namespace to vfs_listxattr. This isn't
> terribly hard to implement, and can be phased in. E.g:
> 
> * Add a "const char *namespace" argument to the listxattr inode op
> * For the normal use case it will be NULL, meaning all xattrs. This
>  includes use in the system call path (a new system call that
>  includes a namespace argument could be added).
> * A filesystem that does not support only returning xattrs for one
>  namespace will return a specified error if @namespace != NULL, after
>  which the fs-indepdent code in fs/xattr.c falls back to pre-allocating
>  an XATTR_LIST_MAX buffer and then filtering out all attributes that
>  are not in the specified namespace.
> * Since the initial use is from the NFS code, converting XFS and ext4
>  to support the @namespace argument should catch most use cases of
>  the @namespace argument, the rest can be converted over time.
> 
> Does this sound reasonable? Or is it overkill?
> 
> Another way to do it is to add a vfs_listxattr_alloc variant, and
> have the NFS server code filter out the user. xattrs as now. There is
> one problem with that: it's possible for the size of listxattr result
> to increase between the first null (length probe) call and the second call,
> in which case the NFS server would return a spurious error. A small chance,
> but it's there nonetheless. And it would be hard to distinguish from
> other errors. So I think I still would prefer using an extra namespace
> argument.
> 
> Alternatively, we could decide that the temporarily allocation of 64k
> is no big deal.

An order-4 allocation is not something that can be relied upon. I would
rather find a way to avoid the need for it if at all possible.


--
Chuck Lever




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: vfs_listxattr and the NFS server: namespaces
  2020-03-29 21:54     ` Chuck Lever
@ 2020-03-29 22:57       ` Frank van der Linden
  0 siblings, 0 replies; 15+ messages in thread
From: Frank van der Linden @ 2020-03-29 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Bruce Fields, Linux NFS Mailing List, linux-fsdevel

On Sun, Mar 29, 2020 at 05:54:38PM -0400, Chuck Lever wrote:
> > Alternatively, we could decide that the temporarily allocation of 64k
> > is no big deal.
> 
> An order-4 allocation is not something that can be relied upon. I would
> rather find a way to avoid the need for it if at all possible.

It's not strictly an order-4 allocation anymore, as I changed
svcxdr_tmpalloc to use kvmalloc.

But sure, it's best to avoid it, it's a waste.

- Frank

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-03-29 22:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-27 23:27 [PATCH v2 00/11] NFS server user xattr support (RFC8276) Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 01/11] xattr: break delegations in {set,remove}xattr and add _locked versions Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 02/11] xattr: add a function to check if a namespace is supported Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 03/11] nfs,nfsd: NFSv4.2 extended attribute protocol definitions Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 04/11] nfsd: split off the write decode code in to a separate function Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 05/11] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 06/11] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 07/11] nfsd: take xattr bits in to account for permission checks Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 08/11] nfsd: add structure definitions for xattr requests / responses Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 09/11] nfsd: use kvmalloc in svcxdr_tmpalloc Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 10/11] nfsd: implement the xattr functions and en/decode logic Frank van der Linden
2020-03-29 20:25   ` vfs_listxattr and the NFS server: namespaces Frank van der Linden
2020-03-29 21:54     ` Chuck Lever
2020-03-29 22:57       ` Frank van der Linden
2020-03-27 23:27 ` [PATCH v2 11/11] nfsd: add fattr support for user extended attributes Frank van der Linden

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).