* [PATCH 01/14] nfs,nfsd: NFSv4.2 extended attribute protocol definitions
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-11 19:59 ` [PATCH 02/14] xattr: modify vfs_{set,remove}xattr for NFS server use Frank van der Linden
` (12 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 02/14] xattr: modify vfs_{set,remove}xattr for NFS server use
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
2020-03-11 19:59 ` [PATCH 01/14] nfs,nfsd: NFSv4.2 extended attribute protocol definitions Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-12 16:23 ` Chuck Lever
2020-03-13 15:35 ` J. Bruce Fields
2020-03-11 19:59 ` [PATCH 03/14] nfsd: split off the write decode code in to a separate function Frank van der Linden
` (11 subsequent siblings)
13 siblings, 2 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 UTC (permalink / raw)
To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden
To be called from the upcoming NFS server xattr code, the vfs_removexattr
and vfs_setxattr need some modifications.
First, 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.
Second, RFC 8276 (NFSv4 extended attribute support) specifies that
delegations should be recalled (8.4.2.4, 8.4.4.4) when a SETXATTR
or REMOVEXATTR operation is performed. So, like with other fs
operations, try to break the delegation. The _locked version of
these operations will not wait for the delegation to be successfully
broken, instead returning an error if it wasn't, so that the NFS
server code can return NFS4ERR_DELAY to the client (similar to
what e.g. vfs_link does).
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
fs/xattr.c | 63 +++++++++++++++++++++++++++++++++++++++++++++------
include/linux/xattr.h | 2 ++
2 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index 90dd78f0eb27..58013bcbc333 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -204,10 +204,10 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
return error;
}
-
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 +216,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);
@@ -379,7 +404,8 @@ __vfs_removexattr(struct dentry *dentry, const char *name)
EXPORT_SYMBOL(__vfs_removexattr);
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 +414,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 +430,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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 02/14] xattr: modify vfs_{set,remove}xattr for NFS server use
2020-03-11 19:59 ` [PATCH 02/14] xattr: modify vfs_{set,remove}xattr for NFS server use Frank van der Linden
@ 2020-03-12 16:23 ` Chuck Lever
2020-03-13 15:35 ` J. Bruce Fields
1 sibling, 0 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-12 16:23 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> To be called from the upcoming NFS server xattr code, the vfs_removexattr
> and vfs_setxattr need some modifications.
>
> First, 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.
>
> Second, RFC 8276 (NFSv4 extended attribute support) specifies that
> delegations should be recalled (8.4.2.4, 8.4.4.4) when a SETXATTR
> or REMOVEXATTR operation is performed. So, like with other fs
> operations, try to break the delegation. The _locked version of
> these operations will not wait for the delegation to be successfully
> broken, instead returning an error if it wasn't, so that the NFS
> server code can return NFS4ERR_DELAY to the client (similar to
> what e.g. vfs_link does).
>
> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
Frank, I appreciate the verbosity of the patch descriptions, and
thanks very much for splitting the client and server work into
separate series.
[cel@klimt linux]$ scripts/get_maintainer.pl fs/xattr.c
Alexander Viro <viro@zeniv.linux.org.uk> (maintainer:FILESYSTEMS (VFS and infrastructure))
linux-fsdevel@vger.kernel.org (open list:FILESYSTEMS (VFS and infrastructure))
linux-kernel@vger.kernel.org (open list)
[cel@klimt linux]$
So patches like this one and 13/14 (or perhaps the whole series)
needs to be cc: linux-fsdevel@vger.kernel.org. At least those
two patches in particular will need an Acked-by: from viro.
> ---
> fs/xattr.c | 63 +++++++++++++++++++++++++++++++++++++++++++++------
> include/linux/xattr.h | 2 ++
> 2 files changed, 58 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 90dd78f0eb27..58013bcbc333 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -204,10 +204,10 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
> return error;
> }
>
> -
> 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)
Since you will need to repost, please consider adding a Doxygen
comment in front of newly introduced global APIs. Such a comment
would be an appropriate place to add non-NFS-related explanatory
text you have provided in the patch description.
Goes for global APIs introduced in other patches too.
> {
> struct inode *inode = dentry->d_inode;
> int error;
> @@ -216,15 +216,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);
> @@ -379,7 +404,8 @@ __vfs_removexattr(struct dentry *dentry, const char *name)
> EXPORT_SYMBOL(__vfs_removexattr);
>
> 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 +414,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 +430,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.16.6
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 02/14] xattr: modify vfs_{set,remove}xattr for NFS server use
2020-03-11 19:59 ` [PATCH 02/14] xattr: modify vfs_{set,remove}xattr for NFS server use Frank van der Linden
2020-03-12 16:23 ` Chuck Lever
@ 2020-03-13 15:35 ` J. Bruce Fields
2020-03-13 16:07 ` [PATCH 02/14] xattr: modify vfs_{set, remove}xattr " Frank van der Linden
1 sibling, 1 reply; 41+ messages in thread
From: J. Bruce Fields @ 2020-03-13 15:35 UTC (permalink / raw)
To: Frank van der Linden; +Cc: chuck.lever, linux-nfs
On Wed, Mar 11, 2020 at 07:59:42PM +0000, Frank van der Linden wrote:
> To be called from the upcoming NFS server xattr code, the vfs_removexattr
> and vfs_setxattr need some modifications.
>
> First, 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.
Unlike NFSv3, NFSv4+ doesn't support atomic before- and after- change
attributes for setattr. Trying to take advantage of that assumption
might result in worse code, though.
> Second, RFC 8276 (NFSv4 extended attribute support) specifies that
> delegations should be recalled (8.4.2.4, 8.4.4.4) when a SETXATTR
> or REMOVEXATTR operation is performed. So, like with other fs
> operations, try to break the delegation. The _locked version of
> these operations will not wait for the delegation to be successfully
> broken, instead returning an error if it wasn't, so that the NFS
> server code can return NFS4ERR_DELAY to the client (similar to
> what e.g. vfs_link does).
Is there a preexisting bug here? Even without NFS support for xattrs, a
local setxattr on the filesystem should still revoke any delegations
held by remote NFS clients. I couldn't tell whether we're getting that
right from a quick look at the current code.
--b.
>
> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
> ---
> fs/xattr.c | 63 +++++++++++++++++++++++++++++++++++++++++++++------
> include/linux/xattr.h | 2 ++
> 2 files changed, 58 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 90dd78f0eb27..58013bcbc333 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -204,10 +204,10 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
> return error;
> }
>
> -
> 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 +216,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);
> @@ -379,7 +404,8 @@ __vfs_removexattr(struct dentry *dentry, const char *name)
> EXPORT_SYMBOL(__vfs_removexattr);
>
> 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 +414,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 +430,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.16.6
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 02/14] xattr: modify vfs_{set, remove}xattr for NFS server use
2020-03-13 15:35 ` J. Bruce Fields
@ 2020-03-13 16:07 ` Frank van der Linden
2020-03-13 21:06 ` J. Bruce Fields
0 siblings, 1 reply; 41+ messages in thread
From: Frank van der Linden @ 2020-03-13 16:07 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: chuck.lever, linux-nfs
On Fri, Mar 13, 2020 at 11:35:49AM -0400, J. Bruce Fields wrote:
>
>
> On Wed, Mar 11, 2020 at 07:59:42PM +0000, Frank van der Linden wrote:
> > Second, RFC 8276 (NFSv4 extended attribute support) specifies that
> > delegations should be recalled (8.4.2.4, 8.4.4.4) when a SETXATTR
> > or REMOVEXATTR operation is performed. So, like with other fs
> > operations, try to break the delegation. The _locked version of
> > these operations will not wait for the delegation to be successfully
> > broken, instead returning an error if it wasn't, so that the NFS
> > server code can return NFS4ERR_DELAY to the client (similar to
> > what e.g. vfs_link does).
>
> Is there a preexisting bug here? Even without NFS support for xattrs, a
> local setxattr on the filesystem should still revoke any delegations
> held by remote NFS clients. I couldn't tell whether we're getting that
> right from a quick look at the current code.
>
> --b.
I think there's currently a bug if that's the expected behavior, yes.
Attribute changes will call notify_change(), and other methods (unlink,
link, rename) call try_break_deleg(). But the xattr entry points
don't do that, which is why I added it.
- Frank
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 02/14] xattr: modify vfs_{set, remove}xattr for NFS server use
2020-03-13 16:07 ` [PATCH 02/14] xattr: modify vfs_{set, remove}xattr " Frank van der Linden
@ 2020-03-13 21:06 ` J. Bruce Fields
0 siblings, 0 replies; 41+ messages in thread
From: J. Bruce Fields @ 2020-03-13 21:06 UTC (permalink / raw)
To: Frank van der Linden; +Cc: chuck.lever, linux-nfs
On Fri, Mar 13, 2020 at 04:07:02PM +0000, Frank van der Linden wrote:
> On Fri, Mar 13, 2020 at 11:35:49AM -0400, J. Bruce Fields wrote:
> >
> >
> > On Wed, Mar 11, 2020 at 07:59:42PM +0000, Frank van der Linden wrote:
> > > Second, RFC 8276 (NFSv4 extended attribute support) specifies that
> > > delegations should be recalled (8.4.2.4, 8.4.4.4) when a SETXATTR
> > > or REMOVEXATTR operation is performed. So, like with other fs
> > > operations, try to break the delegation. The _locked version of
> > > these operations will not wait for the delegation to be successfully
> > > broken, instead returning an error if it wasn't, so that the NFS
> > > server code can return NFS4ERR_DELAY to the client (similar to
> > > what e.g. vfs_link does).
> >
> > Is there a preexisting bug here? Even without NFS support for xattrs, a
> > local setxattr on the filesystem should still revoke any delegations
> > held by remote NFS clients. I couldn't tell whether we're getting that
> > right from a quick look at the current code.
> >
> > --b.
>
> I think there's currently a bug if that's the expected behavior, yes.
> Attribute changes will call notify_change(), and other methods (unlink,
> link, rename) call try_break_deleg(). But the xattr entry points
> don't do that, which is why I added it.
Got it, thanks. In that case I'd move this patch (or the part of it
required to fix that bug) to the front of the series and add a
Cc: stable@vger.kernel.org
--b.
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 03/14] nfsd: split off the write decode code in to a separate function
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
2020-03-11 19:59 ` [PATCH 01/14] nfs,nfsd: NFSv4.2 extended attribute protocol definitions Frank van der Linden
2020-03-11 19:59 ` [PATCH 02/14] xattr: modify vfs_{set,remove}xattr for NFS server use Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-11 19:59 ` [PATCH 04/14] nfsd: make sure the nfsd4_ops array has the right size Frank van der Linden
` (10 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 04/14] nfsd: make sure the nfsd4_ops array has the right size
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (2 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 03/14] nfsd: split off the write decode code in to a separate function Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-11 19:59 ` [PATCH 05/14] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
` (9 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 UTC (permalink / raw)
To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden
The nfsd4_ops was initialized by initializing individual indices (op
numbers). So, the size of the array was determined by the largest
op number.
Some operations are enabled conditionally, based on config options.
If a conditionally enabled operation were to be the highest numbered
operation, the code (through OPDESC) would attempt to access memory
beyond the end of the array. This currently can't happen, since the
highest numbered op is not conditional, but it might in the future.
So, always size the array with LAST_NFS4_OP + 1.
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
fs/nfsd/nfs4proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 0e75f7fb5fec..5de6449e6ff8 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2699,7 +2699,7 @@ static inline u32 nfsd4_seek_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
return (op_encode_hdr_size + 3) * sizeof(__be32);
}
-static const struct nfsd4_operation nfsd4_ops[] = {
+static const struct nfsd4_operation nfsd4_ops[LAST_NFS4_OP + 1] = {
[OP_ACCESS] = {
.op_func = nfsd4_access,
.op_name = "OP_ACCESS",
--
2.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 05/14] nfsd: add defines for NFSv4.2 extended attribute support
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (3 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 04/14] nfsd: make sure the nfsd4_ops array has the right size Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-12 16:23 ` Chuck Lever
2020-03-11 19:59 ` [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
` (8 subsequent siblings)
13 siblings, 1 reply; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 05/14] nfsd: add defines for NFSv4.2 extended attribute support
2020-03-11 19:59 ` [PATCH 05/14] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
@ 2020-03-12 16:23 ` Chuck Lever
0 siblings, 0 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-12 16:23 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> 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)
Not clear to me why these shouldn't go into 01/14
> /* 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)
Should this instead be added, say, in 14/14 ?
> extern const u32 nfsd_suppattrs[3][3];
>
> --
> 2.16.6
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (4 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 05/14] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-12 7:37 ` kbuild test robot
2020-03-12 16:23 ` Chuck Lever
2020-03-11 19:59 ` [PATCH 07/14] nfsd: take xattr bits in to account for permission checks Frank van der Linden
` (7 subsequent siblings)
13 siblings, 2 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts
2020-03-11 19:59 ` [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
@ 2020-03-12 7:37 ` kbuild test robot
2020-03-12 16:23 ` Chuck Lever
1 sibling, 0 replies; 41+ messages in thread
From: kbuild test robot @ 2020-03-12 7:37 UTC (permalink / raw)
To: Frank van der Linden; +Cc: kbuild-all, bfields, chuck.lever, linux-nfs
Hi Frank,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on nfsd/nfsd-next]
[also build test WARNING on nfs/linux-next linus/master v5.6-rc5 next-20200311]
[cannot apply to cel/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Frank-van-der-Linden/server-side-user-xattr-support-RFC-8276/20200312-064928
base: git://linux-nfs.org/~bfields/linux.git nfsd-next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-174-g094d5a94-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> fs/nfsd/vfs.c:2102:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
>> fs/nfsd/vfs.c:2102:13: sparse: expected int err
>> fs/nfsd/vfs.c:2102:13: sparse: got restricted __be32
>> fs/nfsd/vfs.c:2104:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
>> fs/nfsd/vfs.c:2104:24: sparse: expected restricted __be32
>> fs/nfsd/vfs.c:2104:24: sparse: got int err
fs/nfsd/vfs.c:2108:21: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2108:21: sparse: expected int err
fs/nfsd/vfs.c:2108:21: sparse: got restricted __be32
fs/nfsd/vfs.c:2112:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2112:16: sparse: expected restricted __be32
fs/nfsd/vfs.c:2112:16: sparse: got int err
fs/nfsd/vfs.c:2121:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2121:13: sparse: expected int err
fs/nfsd/vfs.c:2121:13: sparse: got restricted __be32
fs/nfsd/vfs.c:2123:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2123:24: sparse: expected restricted __be32
fs/nfsd/vfs.c:2123:24: sparse: got int err
fs/nfsd/vfs.c:2128:21: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2128:21: sparse: expected int err
fs/nfsd/vfs.c:2128:21: sparse: got restricted __be32
fs/nfsd/vfs.c:2132:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2132:16: sparse: expected restricted __be32
fs/nfsd/vfs.c:2132:16: sparse: got int err
fs/nfsd/vfs.c:2148:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2148:13: sparse: expected int err
fs/nfsd/vfs.c:2148:13: sparse: got restricted __be32
fs/nfsd/vfs.c:2150:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2150:24: sparse: expected restricted __be32
fs/nfsd/vfs.c:2150:24: sparse: got int err
fs/nfsd/vfs.c:2172:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2172:13: sparse: expected int err
fs/nfsd/vfs.c:2172:13: sparse: got restricted __be32
fs/nfsd/vfs.c:2174:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2174:24: sparse: expected restricted __be32
fs/nfsd/vfs.c:2174:24: sparse: got int err
vim +2102 fs/nfsd/vfs.c
2094
2095 __be32
2096 nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
2097 void *buf, int *lenp)
2098 {
2099 ssize_t lerr;
2100 int err;
2101
> 2102 err = fh_verify(rqstp, fhp, 0, NFSD_MAY_READ);
2103 if (err)
> 2104 return err;
2105
2106 lerr = vfs_getxattr(fhp->fh_dentry, name, buf, *lenp);
2107 if (lerr < 0)
2108 err = nfsd_xattr_errno(lerr);
2109 else
2110 *lenp = lerr;
2111
2112 return err;
2113 }
2114
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts
@ 2020-03-12 7:37 ` kbuild test robot
0 siblings, 0 replies; 41+ messages in thread
From: kbuild test robot @ 2020-03-12 7:37 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4879 bytes --]
Hi Frank,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on nfsd/nfsd-next]
[also build test WARNING on nfs/linux-next linus/master v5.6-rc5 next-20200311]
[cannot apply to cel/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Frank-van-der-Linden/server-side-user-xattr-support-RFC-8276/20200312-064928
base: git://linux-nfs.org/~bfields/linux.git nfsd-next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-174-g094d5a94-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> fs/nfsd/vfs.c:2102:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
>> fs/nfsd/vfs.c:2102:13: sparse: expected int err
>> fs/nfsd/vfs.c:2102:13: sparse: got restricted __be32
>> fs/nfsd/vfs.c:2104:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
>> fs/nfsd/vfs.c:2104:24: sparse: expected restricted __be32
>> fs/nfsd/vfs.c:2104:24: sparse: got int err
fs/nfsd/vfs.c:2108:21: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2108:21: sparse: expected int err
fs/nfsd/vfs.c:2108:21: sparse: got restricted __be32
fs/nfsd/vfs.c:2112:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2112:16: sparse: expected restricted __be32
fs/nfsd/vfs.c:2112:16: sparse: got int err
fs/nfsd/vfs.c:2121:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2121:13: sparse: expected int err
fs/nfsd/vfs.c:2121:13: sparse: got restricted __be32
fs/nfsd/vfs.c:2123:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2123:24: sparse: expected restricted __be32
fs/nfsd/vfs.c:2123:24: sparse: got int err
fs/nfsd/vfs.c:2128:21: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2128:21: sparse: expected int err
fs/nfsd/vfs.c:2128:21: sparse: got restricted __be32
fs/nfsd/vfs.c:2132:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2132:16: sparse: expected restricted __be32
fs/nfsd/vfs.c:2132:16: sparse: got int err
fs/nfsd/vfs.c:2148:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2148:13: sparse: expected int err
fs/nfsd/vfs.c:2148:13: sparse: got restricted __be32
fs/nfsd/vfs.c:2150:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2150:24: sparse: expected restricted __be32
fs/nfsd/vfs.c:2150:24: sparse: got int err
fs/nfsd/vfs.c:2172:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int err @@ got restricted __int err @@
fs/nfsd/vfs.c:2172:13: sparse: expected int err
fs/nfsd/vfs.c:2172:13: sparse: got restricted __be32
fs/nfsd/vfs.c:2174:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/vfs.c:2174:24: sparse: expected restricted __be32
fs/nfsd/vfs.c:2174:24: sparse: got int err
vim +2102 fs/nfsd/vfs.c
2094
2095 __be32
2096 nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
2097 void *buf, int *lenp)
2098 {
2099 ssize_t lerr;
2100 int err;
2101
> 2102 err = fh_verify(rqstp, fhp, 0, NFSD_MAY_READ);
2103 if (err)
> 2104 return err;
2105
2106 lerr = vfs_getxattr(fhp->fh_dentry, name, buf, *lenp);
2107 if (lerr < 0)
2108 err = nfsd_xattr_errno(lerr);
2109 else
2110 *lenp = lerr;
2111
2112 return err;
2113 }
2114
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts
2020-03-11 19:59 ` [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
2020-03-12 7:37 ` kbuild test robot
@ 2020-03-12 16:23 ` Chuck Lever
2020-03-12 17:16 ` Frank van der Linden
1 sibling, 1 reply; 41+ messages in thread
From: Chuck Lever @ 2020-03-12 16:23 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> 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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts
2020-03-12 16:23 ` Chuck Lever
@ 2020-03-12 17:16 ` Frank van der Linden
2020-03-12 17:57 ` Chuck Lever
0 siblings, 1 reply; 41+ messages in thread
From: Frank van der Linden @ 2020-03-12 17:16 UTC (permalink / raw)
To: Chuck Lever; +Cc: Bruce Fields, Linux NFS Mailing List
On Thu, Mar 12, 2020 at 12:23:57PM -0400, Chuck Lever wrote:
> > 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.
Hi Chuck,
Thanks for this comment (and the other ones).
I forgot to run sparse - I have fixed the __be32/int type mismatches
it flagged in my tree.
I ran checkpath.pl before sending these in. Looks like I missed
one line that is too long. The warnings I didn't fix were:
==
WARNING: function definition argument 'struct dentry *' should also have an identifier name
#156: FILE: include/linux/xattr.h:54:
+int __vfs_setxattr_locked(struct dentry *, const char *, const void *, size_t, int, struct inode **);
==
..changing this would make the prototype declaration of that function not
match with the style of the ones around it (which also don't have argument
names in their declarations) - so I decided not to.
The other one is:
===
WARNING: please, no spaces at the start of a line
#46: FILE: fs/nfsd/vfs.c:616:
+ {^INFS4_ACCESS_XAREAD,^INFSD_MAY_READ^I^I^I},$
===
The warning is correct, but the array that is part of, and the surrounding
accessmap arrays, all have the same spacing. So to both silence checkpacth
and make the code look consistent, I'd have to change the spacing in
all those arrays (nfs3_regaccess, nfs3_diraccess, nfs3_anyaccess).
This didn't seem right, so I didn't do it.
I'll be happy to send a separate whitespace cleanup for that, not sure
if it should be part of this series, though.
Frank
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts
2020-03-12 17:16 ` Frank van der Linden
@ 2020-03-12 17:57 ` Chuck Lever
0 siblings, 0 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-12 17:57 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 12, 2020, at 1:16 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> On Thu, Mar 12, 2020 at 12:23:57PM -0400, Chuck Lever wrote:
>>> 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.
>
> Hi Chuck,
>
> Thanks for this comment (and the other ones).
>
> I forgot to run sparse - I have fixed the __be32/int type mismatches
> it flagged in my tree.
>
> I ran checkpath.pl before sending these in. Looks like I missed
> one line that is too long. The warnings I didn't fix were:
>
> ==
> WARNING: function definition argument 'struct dentry *' should also have an identifier name
> #156: FILE: include/linux/xattr.h:54:
> +int __vfs_setxattr_locked(struct dentry *, const char *, const void *, size_t, int, struct inode **);
> ==
>
> ..changing this would make the prototype declaration of that function not
> match with the style of the ones around it (which also don't have argument
> names in their declarations) - so I decided not to.
>
> The other one is:
>
> ===
> WARNING: please, no spaces at the start of a line
> #46: FILE: fs/nfsd/vfs.c:616:
> + {^INFS4_ACCESS_XAREAD,^INFSD_MAY_READ^I^I^I},$
> ===
>
> The warning is correct, but the array that is part of, and the surrounding
> accessmap arrays, all have the same spacing. So to both silence checkpacth
> and make the code look consistent, I'd have to change the spacing in
> all those arrays (nfs3_regaccess, nfs3_diraccess, nfs3_anyaccess).
>
> This didn't seem right, so I didn't do it.
Fair enough, please add a comment to that effect to the patch description.
> I'll be happy to send a separate whitespace cleanup for that, not sure
> if it should be part of this series, though.
>
> Frank
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 07/14] nfsd: take xattr bits in to account for permission checks
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (5 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-11 19:59 ` [PATCH 08/14] nfsd: add structure definitions for xattr requests / responses Frank van der Linden
` (6 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 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 5de6449e6ff8..b573ae1121af 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 08/14] nfsd: add structure definitions for xattr requests / responses
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (6 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 07/14] nfsd: take xattr bits in to account for permission checks Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-11 19:59 ` [PATCH 09/14] nfsd: use kvmalloc in svcxdr_tmpalloc Frank van der Linden
` (5 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 09/14] nfsd: use kvmalloc in svcxdr_tmpalloc
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (7 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 08/14] nfsd: add structure definitions for xattr requests / responses Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-11 19:59 ` [PATCH 10/14] nfsd: implement the xattr procedure functions Frank van der Linden
` (4 subsequent siblings)
13 siblings, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 10/14] nfsd: implement the xattr procedure functions.
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (8 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 09/14] nfsd: use kvmalloc in svcxdr_tmpalloc Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-12 10:28 ` kbuild test robot
2020-03-12 16:24 ` Chuck Lever
2020-03-11 19:59 ` [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic Frank van der Linden
` (3 subsequent siblings)
13 siblings, 2 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 UTC (permalink / raw)
To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden
Implement the main entry points for the *XATTR operations.
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
fs/nfsd/nfs4proc.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 2 ++
2 files changed, 75 insertions(+)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b573ae1121af..a76b9025a357 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2098,6 +2098,79 @@ 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;
+ int 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 ret, len;
+
+ /*
+ * 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;
+ int 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.
*/
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f6322add2992..b12d7ac6f52c 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"
--
2.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 10/14] nfsd: implement the xattr procedure functions.
2020-03-11 19:59 ` [PATCH 10/14] nfsd: implement the xattr procedure functions Frank van der Linden
@ 2020-03-12 10:28 ` kbuild test robot
2020-03-12 16:24 ` Chuck Lever
1 sibling, 0 replies; 41+ messages in thread
From: kbuild test robot @ 2020-03-12 10:28 UTC (permalink / raw)
To: Frank van der Linden; +Cc: kbuild-all, bfields, chuck.lever, linux-nfs
Hi Frank,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on nfsd/nfsd-next]
[also build test WARNING on nfs/linux-next linus/master v5.6-rc5 next-20200311]
[cannot apply to cel/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Frank-van-der-Linden/server-side-user-xattr-support-RFC-8276/20200312-064928
base: git://linux-nfs.org/~bfields/linux.git nfsd-next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-174-g094d5a94-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
fs/nfsd/nfs4proc.c:1541:24: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [assigned] [usertype] status @@ got e] status @@
fs/nfsd/nfs4proc.c:1541:24: sparse: expected restricted __be32 [assigned] [usertype] status
fs/nfsd/nfs4proc.c:1541:24: sparse: got int
>> fs/nfsd/nfs4proc.c:2122:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int ret @@ got restricted __int ret @@
>> fs/nfsd/nfs4proc.c:2122:13: sparse: expected int ret
>> fs/nfsd/nfs4proc.c:2122:13: sparse: got restricted __be32
>> fs/nfsd/nfs4proc.c:2129:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
>> fs/nfsd/nfs4proc.c:2129:16: sparse: expected restricted __be32
>> fs/nfsd/nfs4proc.c:2129:16: sparse: got int ret
fs/nfsd/nfs4proc.c:2145:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int ret @@ got restricted __int ret @@
fs/nfsd/nfs4proc.c:2145:13: sparse: expected int ret
fs/nfsd/nfs4proc.c:2145:13: sparse: got restricted __be32
fs/nfsd/nfs4proc.c:2148:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/nfs4proc.c:2148:24: sparse: expected restricted __be32
fs/nfsd/nfs4proc.c:2148:24: sparse: got int ret
fs/nfsd/nfs4proc.c:2165:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int ret @@ got restricted __int ret @@
fs/nfsd/nfs4proc.c:2165:13: sparse: expected int ret
fs/nfsd/nfs4proc.c:2165:13: sparse: got restricted __be32
fs/nfsd/nfs4proc.c:2171:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/nfs4proc.c:2171:16: sparse: expected restricted __be32
fs/nfsd/nfs4proc.c:2171:16: sparse: got int ret
vim +2122 fs/nfsd/nfs4proc.c
2111
2112 static __be32
2113 nfsd4_setxattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
2114 union nfsd4_op_u *u)
2115 {
2116 struct nfsd4_setxattr *setxattr = &u->setxattr;
2117 int ret;
2118
2119 if (opens_in_grace(SVC_NET(rqstp)))
2120 return nfserr_grace;
2121
> 2122 ret = nfsd_setxattr(rqstp, &cstate->current_fh, setxattr->setxa_name,
2123 setxattr->setxa_buf, setxattr->setxa_len,
2124 setxattr->setxa_flags);
2125
2126 if (!ret)
2127 set_change_info(&setxattr->setxa_cinfo, &cstate->current_fh);
2128
> 2129 return ret;
2130 }
2131
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 10/14] nfsd: implement the xattr procedure functions.
@ 2020-03-12 10:28 ` kbuild test robot
0 siblings, 0 replies; 41+ messages in thread
From: kbuild test robot @ 2020-03-12 10:28 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3839 bytes --]
Hi Frank,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on nfsd/nfsd-next]
[also build test WARNING on nfs/linux-next linus/master v5.6-rc5 next-20200311]
[cannot apply to cel/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Frank-van-der-Linden/server-side-user-xattr-support-RFC-8276/20200312-064928
base: git://linux-nfs.org/~bfields/linux.git nfsd-next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-174-g094d5a94-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
fs/nfsd/nfs4proc.c:1541:24: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [assigned] [usertype] status @@ got e] status @@
fs/nfsd/nfs4proc.c:1541:24: sparse: expected restricted __be32 [assigned] [usertype] status
fs/nfsd/nfs4proc.c:1541:24: sparse: got int
>> fs/nfsd/nfs4proc.c:2122:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int ret @@ got restricted __int ret @@
>> fs/nfsd/nfs4proc.c:2122:13: sparse: expected int ret
>> fs/nfsd/nfs4proc.c:2122:13: sparse: got restricted __be32
>> fs/nfsd/nfs4proc.c:2129:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
>> fs/nfsd/nfs4proc.c:2129:16: sparse: expected restricted __be32
>> fs/nfsd/nfs4proc.c:2129:16: sparse: got int ret
fs/nfsd/nfs4proc.c:2145:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int ret @@ got restricted __int ret @@
fs/nfsd/nfs4proc.c:2145:13: sparse: expected int ret
fs/nfsd/nfs4proc.c:2145:13: sparse: got restricted __be32
fs/nfsd/nfs4proc.c:2148:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/nfs4proc.c:2148:24: sparse: expected restricted __be32
fs/nfsd/nfs4proc.c:2148:24: sparse: got int ret
fs/nfsd/nfs4proc.c:2165:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected int ret @@ got restricted __int ret @@
fs/nfsd/nfs4proc.c:2165:13: sparse: expected int ret
fs/nfsd/nfs4proc.c:2165:13: sparse: got restricted __be32
fs/nfsd/nfs4proc.c:2171:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/nfs4proc.c:2171:16: sparse: expected restricted __be32
fs/nfsd/nfs4proc.c:2171:16: sparse: got int ret
vim +2122 fs/nfsd/nfs4proc.c
2111
2112 static __be32
2113 nfsd4_setxattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
2114 union nfsd4_op_u *u)
2115 {
2116 struct nfsd4_setxattr *setxattr = &u->setxattr;
2117 int ret;
2118
2119 if (opens_in_grace(SVC_NET(rqstp)))
2120 return nfserr_grace;
2121
> 2122 ret = nfsd_setxattr(rqstp, &cstate->current_fh, setxattr->setxa_name,
2123 setxattr->setxa_buf, setxattr->setxa_len,
2124 setxattr->setxa_flags);
2125
2126 if (!ret)
2127 set_change_info(&setxattr->setxa_cinfo, &cstate->current_fh);
2128
> 2129 return ret;
2130 }
2131
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 10/14] nfsd: implement the xattr procedure functions.
2020-03-11 19:59 ` [PATCH 10/14] nfsd: implement the xattr procedure functions Frank van der Linden
2020-03-12 10:28 ` kbuild test robot
@ 2020-03-12 16:24 ` Chuck Lever
1 sibling, 0 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-12 16:24 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> Implement the main entry points for the *XATTR operations.
This patch triggers "defined but not used" compiler warnings.
These new functions need to be introduced in the same patch
that adds their callsites.
You might consider squashing together all the patches that
only add new NFSD code, for instance.
> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
> ---
> fs/nfsd/nfs4proc.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfs4xdr.c | 2 ++
> 2 files changed, 75 insertions(+)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index b573ae1121af..a76b9025a357 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -2098,6 +2098,79 @@ 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;
> + int 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 ret, len;
> +
> + /*
> + * 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;
> + int 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.
> */
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index f6322add2992..b12d7ac6f52c 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"
> --
> 2.16.6
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (9 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 10/14] nfsd: implement the xattr procedure functions Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-12 13:28 ` kbuild test robot
` (2 more replies)
2020-03-11 19:59 ` [PATCH 12/14] nfsd: add xattr operations to ops array Frank van der Linden
` (2 subsequent siblings)
13 siblings, 3 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 UTC (permalink / raw)
To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden
Add functions to calculate the reply size for the user extended attribute
operations, and implement the XDR encode / decode logic for these
operations.
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
fs/nfsd/nfs4proc.c | 36 +++++
fs/nfsd/nfs4xdr.c | 448 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 484 insertions(+)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a76b9025a357..44d488bdebd9 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2778,6 +2778,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[LAST_NFS4_OP + 1] = {
[OP_ACCESS] = {
.op_func = nfsd4_access,
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index b12d7ac6f52c..41c8b95ca1c5 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1879,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 int to buffer. Uses head and pages constructed by
+ * svcxdr_construct_vector.
+ */
+static int
+nfsd4_vbuf_from_stream(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 int
+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)
+{
+ int 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_stream(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)
{
@@ -1975,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
@@ -4458,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 int
+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;
+ int 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 *);
/*
@@ -4547,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,
};
/*
--
2.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-11 19:59 ` [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic Frank van der Linden
@ 2020-03-12 13:28 ` kbuild test robot
2020-03-12 16:24 ` Chuck Lever
2020-03-12 19:16 ` Chuck Lever
2 siblings, 0 replies; 41+ messages in thread
From: kbuild test robot @ 2020-03-12 13:28 UTC (permalink / raw)
To: Frank van der Linden; +Cc: kbuild-all, bfields, chuck.lever, linux-nfs
Hi Frank,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on nfsd/nfsd-next]
[also build test WARNING on nfs/linux-next linus/master v5.6-rc5 next-20200312]
[cannot apply to cel/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Frank-van-der-Linden/server-side-user-xattr-support-RFC-8276/20200312-064928
base: git://linux-nfs.org/~bfields/linux.git nfsd-next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-174-g094d5a94-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
fs/nfsd/nfs4xdr.c:1860:16: sparse: sparse: incorrect type in assignment (different base types) @@ expected int status @@ got restricted __int status @@
fs/nfsd/nfs4xdr.c:1860:16: sparse: expected int status
fs/nfsd/nfs4xdr.c:1860:16: sparse: got restricted __be32
fs/nfsd/nfs4xdr.c:1862:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/nfs4xdr.c:1862:24: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:1862:24: sparse: got int status
>> fs/nfsd/nfs4xdr.c:1913:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
>> fs/nfsd/nfs4xdr.c:1913:24: sparse: expected int
>> fs/nfsd/nfs4xdr.c:1913:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:1949:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:1949:24: sparse: expected int
fs/nfsd/nfs4xdr.c:1949:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:1958:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:1958:24: sparse: expected int
fs/nfsd/nfs4xdr.c:1958:24: sparse: got restricted __be32 [usertype]
>> fs/nfsd/nfs4xdr.c:1979:9: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [assigned] [usertypint @@
fs/nfsd/nfs4xdr.c:1979:9: sparse: expected int
>> fs/nfsd/nfs4xdr.c:1979:9: sparse: got restricted __be32 [assigned] [usertype] status
>> fs/nfsd/nfs4xdr.c:1997:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got stricted __be32 @@
fs/nfsd/nfs4xdr.c:1997:24: sparse: expected restricted __be32
>> fs/nfsd/nfs4xdr.c:1997:24: sparse: got int [assigned] status
>> fs/nfsd/nfs4xdr.c:2004:24: sparse: sparse: incorrect type in assignment (different base types) @@ expected int [assigned] status @@ got restricted __bint [assigned] status @@
>> fs/nfsd/nfs4xdr.c:2004:24: sparse: expected int [assigned] status
fs/nfsd/nfs4xdr.c:2004:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:2007:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got stricted __be32 @@
fs/nfsd/nfs4xdr.c:2007:16: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:2007:16: sparse: got int [assigned] status
>> fs/nfsd/nfs4xdr.c:2026:16: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [usertype] status @@ got e] status @@
>> fs/nfsd/nfs4xdr.c:2026:16: sparse: expected restricted __be32 [usertype] status
>> fs/nfsd/nfs4xdr.c:2026:16: sparse: got int
>> fs/nfsd/nfs4xdr.c:2044:24: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [assigned] [usertype] status @@ got e] status @@
>> fs/nfsd/nfs4xdr.c:2044:24: sparse: expected restricted __be32 [assigned] [usertype] status
fs/nfsd/nfs4xdr.c:2044:24: sparse: got int
>> fs/nfsd/nfs4xdr.c:2097:39: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got e32 @@
fs/nfsd/nfs4xdr.c:2097:39: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:2097:39: sparse: got int
fs/nfsd/nfs4xdr.c:4698:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:4698:24: sparse: expected int
fs/nfsd/nfs4xdr.c:4698:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:4708:32: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:4708:32: sparse: expected int
fs/nfsd/nfs4xdr.c:4708:32: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:4745:36: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got e32 @@
fs/nfsd/nfs4xdr.c:4745:36: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:4745:36: sparse: got int
fs/nfsd/nfs4xdr.c:4780:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:4780:24: sparse: expected int
fs/nfsd/nfs4xdr.c:4780:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:4784:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:4784:24: sparse: expected int
fs/nfsd/nfs4xdr.c:4784:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:4808:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got stricted __be32 @@
fs/nfsd/nfs4xdr.c:4808:24: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:4808:24: sparse: got int [assigned] status
>> fs/nfsd/nfs4xdr.c:4884:15: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [assigned] [usertype] count @@ got ed int [assigned] [usertype] count @@
>> fs/nfsd/nfs4xdr.c:4884:15: sparse: expected unsigned int [assigned] [usertype] count
fs/nfsd/nfs4xdr.c:4884:15: sparse: got restricted __be32 [usertype]
vim +1913 fs/nfsd/nfs4xdr.c
1881
1882 /*
1883 * XDR data that is more than PAGE_SIZE in size is normally part of a
1884 * read or write. However, the size of extended attributes is limited
1885 * by the maximum request size, and then further limited by the underlying
1886 * filesystem limits. This can exceed PAGE_SIZE (currently, XATTR_SIZE_MAX
1887 * is 64k). Since there is no kvec- or page-based interface to xattrs,
1888 * and we're not dealing with contiguous pages, we need to do some copying.
1889 */
1890
1891 /*
1892 * Decode int to buffer. Uses head and pages constructed by
1893 * svcxdr_construct_vector.
1894 */
1895 static int
1896 nfsd4_vbuf_from_stream(struct nfsd4_compoundargs *argp, struct kvec *head,
1897 struct page **pages, char **bufp, u32 buflen)
1898 {
1899 char *tmp, *dp;
1900 u32 len;
1901
1902 if (buflen <= head->iov_len) {
1903 /*
1904 * We're in luck, the head has enough space. Just return
1905 * the head, no need for copying.
1906 */
1907 *bufp = head->iov_base;
1908 return 0;
1909 }
1910
1911 tmp = svcxdr_tmpalloc(argp, buflen);
1912 if (tmp == NULL)
> 1913 return nfserr_jukebox;
1914
1915 dp = tmp;
1916 memcpy(dp, head->iov_base, head->iov_len);
1917 buflen -= head->iov_len;
1918 dp += head->iov_len;
1919
1920 while (buflen > 0) {
1921 len = min_t(u32, buflen, PAGE_SIZE);
1922 memcpy(dp, page_address(*pages), len);
1923
1924 buflen -= len;
1925 dp += len;
1926 pages++;
1927 }
1928
1929 *bufp = tmp;
1930 return 0;
1931 }
1932
1933 /*
1934 * Get a user extended attribute name from the XDR buffer.
1935 * It will not have the "user." prefix, so prepend it.
1936 * Lastly, check for nul characters in the name.
1937 */
1938 static int
1939 nfsd4_decode_xattr_name(struct nfsd4_compoundargs *argp, char **namep)
1940 {
1941 DECODE_HEAD;
1942 char *name, *sp, *dp;
1943 u32 namelen, cnt;
1944
1945 READ_BUF(4);
1946 namelen = be32_to_cpup(p++);
1947
1948 if (namelen > (XATTR_NAME_MAX - XATTR_USER_PREFIX_LEN))
1949 return nfserr_nametoolong;
1950
1951 if (namelen == 0)
1952 goto xdr_error;
1953
1954 READ_BUF(namelen);
1955
1956 name = svcxdr_tmpalloc(argp, namelen + XATTR_USER_PREFIX_LEN + 1);
1957 if (!name)
1958 return nfserr_jukebox;
1959
1960 memcpy(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN);
1961
1962 /*
1963 * Copy the extended attribute name over while checking for 0
1964 * characters.
1965 */
1966 sp = (char *)p;
1967 dp = name + XATTR_USER_PREFIX_LEN;
1968 cnt = namelen;
1969
1970 while (cnt-- > 0) {
1971 if (*sp == '\0')
1972 goto xdr_error;
1973 *dp++ = *sp++;
1974 }
1975 *dp = '\0';
1976
1977 *namep = name;
1978
> 1979 DECODE_TAIL;
1980 }
1981
1982 /*
1983 * A GETXATTR op request comes without a length specifier. We just set the
1984 * maximum length for the reply based on XATTR_SIZE_MAX and the maximum
1985 * channel reply size, allocate a buffer of that length and pass it to
1986 * vfs_getxattr.
1987 */
1988 static __be32
1989 nfsd4_decode_getxattr(struct nfsd4_compoundargs *argp,
1990 struct nfsd4_getxattr *getxattr)
1991 {
1992 int status;
1993 u32 maxcount;
1994
1995 status = nfsd4_decode_xattr_name(argp, &getxattr->getxa_name);
1996 if (status)
> 1997 return status;
1998
1999 maxcount = svc_max_payload(argp->rqstp);
2000 maxcount = min_t(u32, XATTR_SIZE_MAX, maxcount);
2001
2002 getxattr->getxa_buf = svcxdr_tmpalloc(argp, maxcount);
2003 if (!getxattr->getxa_buf)
> 2004 status = nfserr_jukebox;
2005 getxattr->getxa_len = maxcount;
2006
2007 return status;
2008 }
2009
2010 static __be32
2011 nfsd4_decode_setxattr(struct nfsd4_compoundargs *argp,
2012 struct nfsd4_setxattr *setxattr)
2013 {
2014 DECODE_HEAD;
2015 u32 flags, maxcount, size;
2016 struct kvec head;
2017 struct page **pagelist;
2018
2019 READ_BUF(4);
2020 flags = be32_to_cpup(p++);
2021
2022 if (flags > SETXATTR4_REPLACE)
2023 return nfserr_inval;
2024 setxattr->setxa_flags = flags;
2025
> 2026 status = nfsd4_decode_xattr_name(argp, &setxattr->setxa_name);
2027 if (status)
2028 return status;
2029
2030 maxcount = svc_max_payload(argp->rqstp);
2031 maxcount = min_t(u32, XATTR_SIZE_MAX, maxcount);
2032
2033 READ_BUF(4);
2034 size = be32_to_cpup(p++);
2035 if (size > maxcount)
2036 return nfserr_xattr2big;
2037
2038 setxattr->setxa_len = size;
2039 if (size > 0) {
2040 status = svcxdr_construct_vector(argp, &head, &pagelist, size);
2041 if (status)
2042 return status;
2043
> 2044 status = nfsd4_vbuf_from_stream(argp, &head, pagelist,
2045 &setxattr->setxa_buf, size);
2046 }
2047
2048 DECODE_TAIL;
2049 }
2050
2051 static __be32
2052 nfsd4_decode_listxattrs(struct nfsd4_compoundargs *argp,
2053 struct nfsd4_listxattrs *listxattrs)
2054 {
2055 DECODE_HEAD;
2056 u32 maxcount;
2057
2058 READ_BUF(12);
2059 p = xdr_decode_hyper(p, &listxattrs->lsxa_cookie);
2060
2061 /*
2062 * If the cookie is too large to have even one user.x attribute
2063 * plus trailing '\0' left in a maximum size buffer, it's invalid.
2064 */
2065 if (listxattrs->lsxa_cookie >=
2066 (XATTR_LIST_MAX / (XATTR_USER_PREFIX_LEN + 2)))
2067 return nfserr_badcookie;
2068
2069 maxcount = be32_to_cpup(p++);
2070 if (maxcount < 8)
2071 /* Always need at least 2 words (length and one character) */
2072 return nfserr_inval;
2073
2074 maxcount = min(maxcount, svc_max_payload(argp->rqstp));
2075 listxattrs->lsxa_maxcount = maxcount;
2076
2077 /*
2078 * Unfortunately, there is no interface to only list xattrs for
2079 * one prefix. So there is no good way to convert maxcount to
2080 * a maximum value to pass to vfs_listxattr, as we don't know
2081 * how many of the returned attributes will be user attributes.
2082 *
2083 * So, always ask vfs_listxattr for the maximum size, and encode
2084 * as many as possible.
2085 */
2086 listxattrs->lsxa_buf = svcxdr_tmpalloc(argp, XATTR_LIST_MAX);
2087 if (!listxattrs->lsxa_buf)
2088 status = nfserr_jukebox;
2089
2090 DECODE_TAIL;
2091 }
2092
2093 static __be32
2094 nfsd4_decode_removexattr(struct nfsd4_compoundargs *argp,
2095 struct nfsd4_removexattr *removexattr)
2096 {
> 2097 return nfsd4_decode_xattr_name(argp, &removexattr->rmxa_name);
2098 }
2099
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
@ 2020-03-12 13:28 ` kbuild test robot
0 siblings, 0 replies; 41+ messages in thread
From: kbuild test robot @ 2020-03-12 13:28 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 13885 bytes --]
Hi Frank,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on nfsd/nfsd-next]
[also build test WARNING on nfs/linux-next linus/master v5.6-rc5 next-20200312]
[cannot apply to cel/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Frank-van-der-Linden/server-side-user-xattr-support-RFC-8276/20200312-064928
base: git://linux-nfs.org/~bfields/linux.git nfsd-next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-174-g094d5a94-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
fs/nfsd/nfs4xdr.c:1860:16: sparse: sparse: incorrect type in assignment (different base types) @@ expected int status @@ got restricted __int status @@
fs/nfsd/nfs4xdr.c:1860:16: sparse: expected int status
fs/nfsd/nfs4xdr.c:1860:16: sparse: got restricted __be32
fs/nfsd/nfs4xdr.c:1862:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got be32 @@
fs/nfsd/nfs4xdr.c:1862:24: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:1862:24: sparse: got int status
>> fs/nfsd/nfs4xdr.c:1913:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
>> fs/nfsd/nfs4xdr.c:1913:24: sparse: expected int
>> fs/nfsd/nfs4xdr.c:1913:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:1949:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:1949:24: sparse: expected int
fs/nfsd/nfs4xdr.c:1949:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:1958:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:1958:24: sparse: expected int
fs/nfsd/nfs4xdr.c:1958:24: sparse: got restricted __be32 [usertype]
>> fs/nfsd/nfs4xdr.c:1979:9: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [assigned] [usertypint @@
fs/nfsd/nfs4xdr.c:1979:9: sparse: expected int
>> fs/nfsd/nfs4xdr.c:1979:9: sparse: got restricted __be32 [assigned] [usertype] status
>> fs/nfsd/nfs4xdr.c:1997:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got stricted __be32 @@
fs/nfsd/nfs4xdr.c:1997:24: sparse: expected restricted __be32
>> fs/nfsd/nfs4xdr.c:1997:24: sparse: got int [assigned] status
>> fs/nfsd/nfs4xdr.c:2004:24: sparse: sparse: incorrect type in assignment (different base types) @@ expected int [assigned] status @@ got restricted __bint [assigned] status @@
>> fs/nfsd/nfs4xdr.c:2004:24: sparse: expected int [assigned] status
fs/nfsd/nfs4xdr.c:2004:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:2007:16: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got stricted __be32 @@
fs/nfsd/nfs4xdr.c:2007:16: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:2007:16: sparse: got int [assigned] status
>> fs/nfsd/nfs4xdr.c:2026:16: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [usertype] status @@ got e] status @@
>> fs/nfsd/nfs4xdr.c:2026:16: sparse: expected restricted __be32 [usertype] status
>> fs/nfsd/nfs4xdr.c:2026:16: sparse: got int
>> fs/nfsd/nfs4xdr.c:2044:24: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [assigned] [usertype] status @@ got e] status @@
>> fs/nfsd/nfs4xdr.c:2044:24: sparse: expected restricted __be32 [assigned] [usertype] status
fs/nfsd/nfs4xdr.c:2044:24: sparse: got int
>> fs/nfsd/nfs4xdr.c:2097:39: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got e32 @@
fs/nfsd/nfs4xdr.c:2097:39: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:2097:39: sparse: got int
fs/nfsd/nfs4xdr.c:4698:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:4698:24: sparse: expected int
fs/nfsd/nfs4xdr.c:4698:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:4708:32: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:4708:32: sparse: expected int
fs/nfsd/nfs4xdr.c:4708:32: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:4745:36: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got e32 @@
fs/nfsd/nfs4xdr.c:4745:36: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:4745:36: sparse: got int
fs/nfsd/nfs4xdr.c:4780:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:4780:24: sparse: expected int
fs/nfsd/nfs4xdr.c:4780:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:4784:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected int @@ got restricted __be32 [usertypint @@
fs/nfsd/nfs4xdr.c:4784:24: sparse: expected int
fs/nfsd/nfs4xdr.c:4784:24: sparse: got restricted __be32 [usertype]
fs/nfsd/nfs4xdr.c:4808:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __be32 @@ got stricted __be32 @@
fs/nfsd/nfs4xdr.c:4808:24: sparse: expected restricted __be32
fs/nfsd/nfs4xdr.c:4808:24: sparse: got int [assigned] status
>> fs/nfsd/nfs4xdr.c:4884:15: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [assigned] [usertype] count @@ got ed int [assigned] [usertype] count @@
>> fs/nfsd/nfs4xdr.c:4884:15: sparse: expected unsigned int [assigned] [usertype] count
fs/nfsd/nfs4xdr.c:4884:15: sparse: got restricted __be32 [usertype]
vim +1913 fs/nfsd/nfs4xdr.c
1881
1882 /*
1883 * XDR data that is more than PAGE_SIZE in size is normally part of a
1884 * read or write. However, the size of extended attributes is limited
1885 * by the maximum request size, and then further limited by the underlying
1886 * filesystem limits. This can exceed PAGE_SIZE (currently, XATTR_SIZE_MAX
1887 * is 64k). Since there is no kvec- or page-based interface to xattrs,
1888 * and we're not dealing with contiguous pages, we need to do some copying.
1889 */
1890
1891 /*
1892 * Decode int to buffer. Uses head and pages constructed by
1893 * svcxdr_construct_vector.
1894 */
1895 static int
1896 nfsd4_vbuf_from_stream(struct nfsd4_compoundargs *argp, struct kvec *head,
1897 struct page **pages, char **bufp, u32 buflen)
1898 {
1899 char *tmp, *dp;
1900 u32 len;
1901
1902 if (buflen <= head->iov_len) {
1903 /*
1904 * We're in luck, the head has enough space. Just return
1905 * the head, no need for copying.
1906 */
1907 *bufp = head->iov_base;
1908 return 0;
1909 }
1910
1911 tmp = svcxdr_tmpalloc(argp, buflen);
1912 if (tmp == NULL)
> 1913 return nfserr_jukebox;
1914
1915 dp = tmp;
1916 memcpy(dp, head->iov_base, head->iov_len);
1917 buflen -= head->iov_len;
1918 dp += head->iov_len;
1919
1920 while (buflen > 0) {
1921 len = min_t(u32, buflen, PAGE_SIZE);
1922 memcpy(dp, page_address(*pages), len);
1923
1924 buflen -= len;
1925 dp += len;
1926 pages++;
1927 }
1928
1929 *bufp = tmp;
1930 return 0;
1931 }
1932
1933 /*
1934 * Get a user extended attribute name from the XDR buffer.
1935 * It will not have the "user." prefix, so prepend it.
1936 * Lastly, check for nul characters in the name.
1937 */
1938 static int
1939 nfsd4_decode_xattr_name(struct nfsd4_compoundargs *argp, char **namep)
1940 {
1941 DECODE_HEAD;
1942 char *name, *sp, *dp;
1943 u32 namelen, cnt;
1944
1945 READ_BUF(4);
1946 namelen = be32_to_cpup(p++);
1947
1948 if (namelen > (XATTR_NAME_MAX - XATTR_USER_PREFIX_LEN))
1949 return nfserr_nametoolong;
1950
1951 if (namelen == 0)
1952 goto xdr_error;
1953
1954 READ_BUF(namelen);
1955
1956 name = svcxdr_tmpalloc(argp, namelen + XATTR_USER_PREFIX_LEN + 1);
1957 if (!name)
1958 return nfserr_jukebox;
1959
1960 memcpy(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN);
1961
1962 /*
1963 * Copy the extended attribute name over while checking for 0
1964 * characters.
1965 */
1966 sp = (char *)p;
1967 dp = name + XATTR_USER_PREFIX_LEN;
1968 cnt = namelen;
1969
1970 while (cnt-- > 0) {
1971 if (*sp == '\0')
1972 goto xdr_error;
1973 *dp++ = *sp++;
1974 }
1975 *dp = '\0';
1976
1977 *namep = name;
1978
> 1979 DECODE_TAIL;
1980 }
1981
1982 /*
1983 * A GETXATTR op request comes without a length specifier. We just set the
1984 * maximum length for the reply based on XATTR_SIZE_MAX and the maximum
1985 * channel reply size, allocate a buffer of that length and pass it to
1986 * vfs_getxattr.
1987 */
1988 static __be32
1989 nfsd4_decode_getxattr(struct nfsd4_compoundargs *argp,
1990 struct nfsd4_getxattr *getxattr)
1991 {
1992 int status;
1993 u32 maxcount;
1994
1995 status = nfsd4_decode_xattr_name(argp, &getxattr->getxa_name);
1996 if (status)
> 1997 return status;
1998
1999 maxcount = svc_max_payload(argp->rqstp);
2000 maxcount = min_t(u32, XATTR_SIZE_MAX, maxcount);
2001
2002 getxattr->getxa_buf = svcxdr_tmpalloc(argp, maxcount);
2003 if (!getxattr->getxa_buf)
> 2004 status = nfserr_jukebox;
2005 getxattr->getxa_len = maxcount;
2006
2007 return status;
2008 }
2009
2010 static __be32
2011 nfsd4_decode_setxattr(struct nfsd4_compoundargs *argp,
2012 struct nfsd4_setxattr *setxattr)
2013 {
2014 DECODE_HEAD;
2015 u32 flags, maxcount, size;
2016 struct kvec head;
2017 struct page **pagelist;
2018
2019 READ_BUF(4);
2020 flags = be32_to_cpup(p++);
2021
2022 if (flags > SETXATTR4_REPLACE)
2023 return nfserr_inval;
2024 setxattr->setxa_flags = flags;
2025
> 2026 status = nfsd4_decode_xattr_name(argp, &setxattr->setxa_name);
2027 if (status)
2028 return status;
2029
2030 maxcount = svc_max_payload(argp->rqstp);
2031 maxcount = min_t(u32, XATTR_SIZE_MAX, maxcount);
2032
2033 READ_BUF(4);
2034 size = be32_to_cpup(p++);
2035 if (size > maxcount)
2036 return nfserr_xattr2big;
2037
2038 setxattr->setxa_len = size;
2039 if (size > 0) {
2040 status = svcxdr_construct_vector(argp, &head, &pagelist, size);
2041 if (status)
2042 return status;
2043
> 2044 status = nfsd4_vbuf_from_stream(argp, &head, pagelist,
2045 &setxattr->setxa_buf, size);
2046 }
2047
2048 DECODE_TAIL;
2049 }
2050
2051 static __be32
2052 nfsd4_decode_listxattrs(struct nfsd4_compoundargs *argp,
2053 struct nfsd4_listxattrs *listxattrs)
2054 {
2055 DECODE_HEAD;
2056 u32 maxcount;
2057
2058 READ_BUF(12);
2059 p = xdr_decode_hyper(p, &listxattrs->lsxa_cookie);
2060
2061 /*
2062 * If the cookie is too large to have even one user.x attribute
2063 * plus trailing '\0' left in a maximum size buffer, it's invalid.
2064 */
2065 if (listxattrs->lsxa_cookie >=
2066 (XATTR_LIST_MAX / (XATTR_USER_PREFIX_LEN + 2)))
2067 return nfserr_badcookie;
2068
2069 maxcount = be32_to_cpup(p++);
2070 if (maxcount < 8)
2071 /* Always need@least 2 words (length and one character) */
2072 return nfserr_inval;
2073
2074 maxcount = min(maxcount, svc_max_payload(argp->rqstp));
2075 listxattrs->lsxa_maxcount = maxcount;
2076
2077 /*
2078 * Unfortunately, there is no interface to only list xattrs for
2079 * one prefix. So there is no good way to convert maxcount to
2080 * a maximum value to pass to vfs_listxattr, as we don't know
2081 * how many of the returned attributes will be user attributes.
2082 *
2083 * So, always ask vfs_listxattr for the maximum size, and encode
2084 * as many as possible.
2085 */
2086 listxattrs->lsxa_buf = svcxdr_tmpalloc(argp, XATTR_LIST_MAX);
2087 if (!listxattrs->lsxa_buf)
2088 status = nfserr_jukebox;
2089
2090 DECODE_TAIL;
2091 }
2092
2093 static __be32
2094 nfsd4_decode_removexattr(struct nfsd4_compoundargs *argp,
2095 struct nfsd4_removexattr *removexattr)
2096 {
> 2097 return nfsd4_decode_xattr_name(argp, &removexattr->rmxa_name);
2098 }
2099
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-11 19:59 ` [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic Frank van der Linden
2020-03-12 13:28 ` kbuild test robot
@ 2020-03-12 16:24 ` Chuck Lever
2020-03-19 22:13 ` Frank van der Linden
2020-03-25 23:44 ` Frank van der Linden
2020-03-12 19:16 ` Chuck Lever
2 siblings, 2 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-12 16:24 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> Add functions to calculate the reply size for the user extended attribute
> operations, and implement the XDR encode / decode logic for these
> operations.
>
> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
> ---
> fs/nfsd/nfs4proc.c | 36 +++++
> fs/nfsd/nfs4xdr.c | 448 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 484 insertions(+)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index a76b9025a357..44d488bdebd9 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -2778,6 +2778,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);
These should be added in the same patch that adds OP_GETXATTR and friends.
Also, Trond recently added xdr_align_size which I prefer over the
use of XDR_QUADLEN in new code.
> +}
> +
> +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[LAST_NFS4_OP + 1] = {
> [OP_ACCESS] = {
> .op_func = nfsd4_access,
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index b12d7ac6f52c..41c8b95ca1c5 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1879,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 int to buffer. Uses head and pages constructed by
> + * svcxdr_construct_vector.
> + */
> +static int
> +nfsd4_vbuf_from_stream(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 int
> +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)
> +{
> + int 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_stream(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.
> + */
Well, this approach worries me a little bit. Wouldn't it be better if the
VFS provided the APIs? Review by linux-fsdevel might help here.
> + 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)
> {
> @@ -1975,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
> @@ -4458,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 int
> +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;
> + int 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 *);
>
> /*
> @@ -4547,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,
> };
>
> /*
> --
> 2.16.6
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-12 16:24 ` Chuck Lever
@ 2020-03-19 22:13 ` Frank van der Linden
2020-03-19 22:15 ` Chuck Lever
2020-03-25 23:44 ` Frank van der Linden
1 sibling, 1 reply; 41+ messages in thread
From: Frank van der Linden @ 2020-03-19 22:13 UTC (permalink / raw)
To: Chuck Lever; +Cc: Bruce Fields, Linux NFS Mailing List
On Thu, Mar 12, 2020 at 12:24:18PM -0400, Chuck Lever wrote:
> > +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);
>
> These should be added in the same patch that adds OP_GETXATTR and friends.
>
> Also, Trond recently added xdr_align_size which I prefer over the
> use of XDR_QUADLEN in new code.
Thanks, I've squashed together those patches for this and the other reasons
you pointed out.
As for XDR_QUADLEN: that returns the 32bit-word rounded up lenghth - in words.
xdr_aligned_size returns the 32bit-word rounded up length - in bytes.
So, the result would then look something like:
return xdr_align_size((op_encode_hdr_size * 4) + 4 + rlen);
Is that what you're suggesting?
- Frank
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-19 22:13 ` Frank van der Linden
@ 2020-03-19 22:15 ` Chuck Lever
0 siblings, 0 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-19 22:15 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 19, 2020, at 6:13 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> On Thu, Mar 12, 2020 at 12:24:18PM -0400, Chuck Lever wrote:
>>> +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);
>>
>> These should be added in the same patch that adds OP_GETXATTR and friends.
>>
>> Also, Trond recently added xdr_align_size which I prefer over the
>> use of XDR_QUADLEN in new code.
>
> Thanks, I've squashed together those patches for this and the other reasons
> you pointed out.
>
> As for XDR_QUADLEN: that returns the 32bit-word rounded up lenghth - in words.
> xdr_aligned_size returns the 32bit-word rounded up length - in bytes.
>
> So, the result would then look something like:
>
> return xdr_align_size((op_encode_hdr_size * 4) + 4 + rlen);
>
> Is that what you're suggesting?
Oops, you're right. When you want words, XDR_QUADLEN is correct. Never mind!
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-12 16:24 ` Chuck Lever
2020-03-19 22:13 ` Frank van der Linden
@ 2020-03-25 23:44 ` Frank van der Linden
2020-03-26 14:12 ` Chuck Lever
1 sibling, 1 reply; 41+ messages in thread
From: Frank van der Linden @ 2020-03-25 23:44 UTC (permalink / raw)
To: Chuck Lever; +Cc: Bruce Fields, Linux NFS Mailing List
On Thu, Mar 12, 2020 at 12:24:18PM -0400, Chuck Lever wrote:
> > On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
> > + /*
> > + * 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.
> > + */
>
> Well, this approach worries me a little bit. Wouldn't it be better if the
> VFS provided the APIs? Review by linux-fsdevel might help here.
I missed this comment initially, sorry about the slow reply.
I'll copy this one to -fsdevel for v2.
It would require a modified or new entry point to all filesystems to
support this properly, so I didn't touch it. It's not a complex
task, it just would lead to quite a bit of code churn.
- Frank
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-25 23:44 ` Frank van der Linden
@ 2020-03-26 14:12 ` Chuck Lever
0 siblings, 0 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-26 14:12 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 25, 2020, at 7:44 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> On Thu, Mar 12, 2020 at 12:24:18PM -0400, Chuck Lever wrote:
>>> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>>> + /*
>>> + * 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.
>>> + */
>>
>> Well, this approach worries me a little bit. Wouldn't it be better if the
>> VFS provided the APIs? Review by linux-fsdevel might help here.
>
> I missed this comment initially, sorry about the slow reply.
>
> I'll copy this one to -fsdevel for v2.
>
> It would require a modified or new entry point to all filesystems to
> support this properly, so I didn't touch it. It's not a complex
> task, it just would lead to quite a bit of code churn.
Yep, I recognize it would be a substantial chunk of work. Passing
it by -fsdevel is the right thing to do, IMO. Thanks!
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-11 19:59 ` [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic Frank van der Linden
2020-03-12 13:28 ` kbuild test robot
2020-03-12 16:24 ` Chuck Lever
@ 2020-03-12 19:16 ` Chuck Lever
2020-03-20 16:47 ` Frank van der Linden
2 siblings, 1 reply; 41+ messages in thread
From: Chuck Lever @ 2020-03-12 19:16 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> Add functions to calculate the reply size for the user extended attribute
> operations, and implement the XDR encode / decode logic for these
> operations.
>
> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
> ---
> fs/nfsd/nfs4proc.c | 36 +++++
> fs/nfsd/nfs4xdr.c | 448 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 484 insertions(+)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index a76b9025a357..44d488bdebd9 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -2778,6 +2778,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[LAST_NFS4_OP + 1] = {
> [OP_ACCESS] = {
> .op_func = nfsd4_access,
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index b12d7ac6f52c..41c8b95ca1c5 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1879,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 int to buffer.
"int" is of course a C keyword, so this typo had me confused!
I think you mean "Decode into buffer."
> Uses head and pages constructed by
> + * svcxdr_construct_vector.
> + */
> +static int
> +nfsd4_vbuf_from_stream(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 int
> +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)
> +{
> + int 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_stream(argp, &head, pagelist,
> + &setxattr->setxa_buf, size);
> + }
Now I'm wondering if read_bytes_from_xdr_buf() might be adequate
for this purpose, so you can avoid open-coding all of this logic.
> +
> + 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)
> {
> @@ -1975,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
> @@ -4458,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 int
> +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;
> + int 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 *);
>
> /*
> @@ -4547,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,
> };
>
> /*
> --
> 2.16.6
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-12 19:16 ` Chuck Lever
@ 2020-03-20 16:47 ` Frank van der Linden
2020-03-20 17:34 ` Chuck Lever
0 siblings, 1 reply; 41+ messages in thread
From: Frank van der Linden @ 2020-03-20 16:47 UTC (permalink / raw)
To: Chuck Lever; +Cc: Bruce Fields, Linux NFS Mailing List
Hi Chuck,
On Thu, Mar 12, 2020 at 03:16:37PM -0400, Chuck Lever wrote:
> > +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_stream(argp, &head, pagelist,
> > + &setxattr->setxa_buf, size);
> > + }
>
> Now I'm wondering if read_bytes_from_xdr_buf() might be adequate
> for this purpose, so you can avoid open-coding all of this logic.
This took a little longer, I had to check my notes, but basically the
reasons for doing it this way are:
* The nfsd decode path uses nfsd4_compoundargs, which doesn't have an
xdr_stream (it has a page array). So read_bytes_from_xdr_buf isn't
a natural fit.
* READ_BUF/read_buf don't deal with > PAGE_SIZE chunks, but xattrs may
be larger than that.
The other code that deals with > PAGE_SIZE chunks is the write code. So,
I factored out some code from the write code, used that, and added a function
to process the resulting kvec / pagelist (nfs4_vbuf_from_stream).
There definitely seem to be several copy functions in both the client
and server code that basically do the same, but in slightly different ways,
depending on whether they use an XDR buf or not, whether the pages are
mapped or not, etc. Seems like a good candidate for a cleanup, but I
considered it to be out of scope for these patches.
- Frank
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-20 16:47 ` Frank van der Linden
@ 2020-03-20 17:34 ` Chuck Lever
2020-03-20 17:54 ` J. Bruce Fields
2020-03-20 19:44 ` Frank van der Linden
0 siblings, 2 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-20 17:34 UTC (permalink / raw)
To: Frank van der Linden, Bruce Fields; +Cc: Linux NFS Mailing List
> On Mar 20, 2020, at 12:47 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> Hi Chuck,
>
> On Thu, Mar 12, 2020 at 03:16:37PM -0400, Chuck Lever wrote:
>>> +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_stream(argp, &head, pagelist,
>>> + &setxattr->setxa_buf, size);
>>> + }
>>
>> Now I'm wondering if read_bytes_from_xdr_buf() might be adequate
>> for this purpose, so you can avoid open-coding all of this logic.
>
> This took a little longer, I had to check my notes,
Thanks for checking!
> but basically the reasons for doing it this way are:
>
> * The nfsd decode path uses nfsd4_compoundargs, which doesn't have an
> xdr_stream (it has a page array). So read_bytes_from_xdr_buf isn't
> a natural fit.
> * READ_BUF/read_buf don't deal with > PAGE_SIZE chunks, but xattrs may
> be larger than that.
>
> The other code that deals with > PAGE_SIZE chunks is the write code. So,
> I factored out some code from the write code, used that, and added a function
> to process the resulting kvec / pagelist (nfs4_vbuf_from_stream).
Yes, I only recently discovered that the xdr_stream helpers do not
work for decoding data items that are page size or larger.
For the record, my IMA prototype takes this approach for decode_fattr:
+ if (bmval[2] & FATTR4_WORD2_IMA) {
+ READ_BUF(4);
+ len += 4;
+ dummy32 = be32_to_cpup(p++);
+ READ_BUF(dummy32);
+ if (dummy32 > NFS4_MAXIMALEN)
+ return nfserr_badlabel;
+ *ima = svcxdr_tmpalloc(argp, sizeof(**ima));
+ if (*ima == NULL)
+ return nfserr_jukebox;
+
+ len += (XDR_QUADLEN(dummy32) << 2);
+ READMEM(buf, dummy32);
+ (*ima)->len = dummy32;
+ (*ima)->data = svcxdr_dupstr(argp, buf, dummy32);
+ if ((*ima)->data == NULL)
+ return nfserr_jukebox;
+ } else
+ ima = NULL;
Although, an IMA blob is never larger than a page, IIRC.
> There definitely seem to be several copy functions in both the client
> and server code that basically do the same, but in slightly different ways,
> depending on whether they use an XDR buf or not, whether the pages are
> mapped or not, etc. Seems like a good candidate for a cleanup, but I
> considered it to be out of scope for these patches.
"out of scope" - probably so. Depending on Bruce's thinking, we can
add this to the list of janitorial tasks.
For my peace of mind, "from_stream" implies there _is_ an xdr_stream
in use, even though the function does not have a struct xdr_stream
parameter. Perhaps a different naming scheme would be wise.
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-20 17:34 ` Chuck Lever
@ 2020-03-20 17:54 ` J. Bruce Fields
2020-03-20 19:44 ` Frank van der Linden
1 sibling, 0 replies; 41+ messages in thread
From: J. Bruce Fields @ 2020-03-20 17:54 UTC (permalink / raw)
To: Chuck Lever; +Cc: Frank van der Linden, Linux NFS Mailing List
On Fri, Mar 20, 2020 at 01:34:53PM -0400, Chuck Lever wrote:
>
>
> > On Mar 20, 2020, at 12:47 PM, Frank van der Linden <fllinden@amazon.com> wrote:
> > There definitely seem to be several copy functions in both the client
> > and server code that basically do the same, but in slightly different ways,
> > depending on whether they use an XDR buf or not, whether the pages are
> > mapped or not, etc. Seems like a good candidate for a cleanup, but I
> > considered it to be out of scope for these patches.
>
> "out of scope" - probably so. Depending on Bruce's thinking, we can
> add this to the list of janitorial tasks.
The rewrite of the server xdr encoding was... oh jeez, six years ago
now, and I haven't really thought about it since. I remember thinking
we should do the decode side as well someday, though, and thinking it
should be pretty doable.
--b.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic
2020-03-20 17:34 ` Chuck Lever
2020-03-20 17:54 ` J. Bruce Fields
@ 2020-03-20 19:44 ` Frank van der Linden
1 sibling, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-20 19:44 UTC (permalink / raw)
To: Chuck Lever; +Cc: Bruce Fields, Linux NFS Mailing List
On Fri, Mar 20, 2020 at 01:34:53PM -0400, Chuck Lever wrote:
>
> For my peace of mind, "from_stream" implies there _is_ an xdr_stream
> in use, even though the function does not have a struct xdr_stream
> parameter. Perhaps a different naming scheme would be wise.
Sure. I'll change it to "from_vector", which matches the name of the
"svcxdr_construct_vector" function. Not 100% correct, since it's really
a vector + page array, but it seems better.
- Frank
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 12/14] nfsd: add xattr operations to ops array
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (10 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 11/14] nfsd: add user xattr RPC XDR encoding/decoding logic Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-11 19:59 ` [PATCH 13/14] xattr: add a function to check if a namespace is supported Frank van der Linden
2020-03-11 19:59 ` [PATCH 14/14] nfsd: add fattr support for user extended attributes Frank van der Linden
13 siblings, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 UTC (permalink / raw)
To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden
With all underlying code implemented, let's add the user extended
attributes operations to nfsd4_ops.
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
fs/nfsd/nfs4proc.c | 22 ++++++++++++++++++++++
include/linux/nfs4.h | 2 +-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 44d488bdebd9..889cb6538244 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -3195,6 +3195,28 @@ static const struct nfsd4_operation nfsd4_ops[LAST_NFS4_OP + 1] = {
.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/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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 13/14] xattr: add a function to check if a namespace is supported
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (11 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 12/14] nfsd: add xattr operations to ops array Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
2020-03-12 16:24 ` Chuck Lever
2020-03-11 19:59 ` [PATCH 14/14] nfsd: add fattr support for user extended attributes Frank van der Linden
13 siblings, 1 reply; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 UTC (permalink / raw)
To: bfields, chuck.lever, linux-nfs; +Cc: Frank van der Linden
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.
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 58013bcbc333..7d0ebab1df30 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 13/14] xattr: add a function to check if a namespace is supported
2020-03-11 19:59 ` [PATCH 13/14] xattr: add a function to check if a namespace is supported Frank van der Linden
@ 2020-03-12 16:24 ` Chuck Lever
0 siblings, 0 replies; 41+ messages in thread
From: Chuck Lever @ 2020-03-12 16:24 UTC (permalink / raw)
To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List
> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>
> 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.
Here's a patch that needs to be cc: linux-fsdevel.
Also, perhaps 13/14 and 14/14 could be squashed together.
> 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 58013bcbc333..7d0ebab1df30 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.16.6
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 14/14] nfsd: add fattr support for user extended attributes
2020-03-11 19:59 [PATCH 00/14] server side user xattr support (RFC 8276) Frank van der Linden
` (12 preceding siblings ...)
2020-03-11 19:59 ` [PATCH 13/14] xattr: add a function to check if a namespace is supported Frank van der Linden
@ 2020-03-11 19:59 ` Frank van der Linden
13 siblings, 0 replies; 41+ messages in thread
From: Frank van der Linden @ 2020-03-11 19:59 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 41c8b95ca1c5..d4a45cfefb86 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.16.6
^ permalink raw reply related [flat|nested] 41+ messages in thread