All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR)
@ 2010-06-21 11:25 James Morris
  2010-06-21 11:27 ` [PATCH 1/8][RFC v05] NFSv3: convert client to generic xattr API James Morris
                   ` (7 more replies)
  0 siblings, 8 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:25 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

This is version 5 of the NFSv3 XATTR protocol extension patches, which 
I've previously posted as:
 
v1: http://thread.gmane.org/gmane.linux.file-systems/35475
v2: http://thread.gmane.org/gmane.linux.nfs/30539
v3: http://thread.gmane.org/gmane.linux.nfs/30971
v4: http://thread.gmane.org/gmane.linux.kernel.lsm/10562

In the previous version, I implemented a new top-level xattr namespace on 
the server, which is used to store client-supplied xattrs, e.g.:

client: user.a        ->       server: nfsd.user.a

In this version, I've enhanced support for security xattrs, and updated 
SELinux so that it can utilize the XATTR protocol for security labeling.

I added a new NFS error code, NFSERR_NODATA, so that we can cleanly handle 
cases where the xattr system calls on the server return -ENODATA to 
indicate a non-existent xattr (this is often not an error condition).

Also new are the xattr and xattrsec mount options, which are used to 
control the use of the XATTR protocol and XATTR security labeling 
respectively (see patch #7).

The userspace patch for the mount utility is available at:
http://namei.org/nfsv3xattr/v05/userspace/

The XATTR code also now calls back into the LSM during file creation so 
that an appropriate security label may be installed at the same time 
(atomically from the client pov).  This follows the behavior of the ACL 
code (see nfs3_init_xattr() in patch #6).

For SELinux, the approach is to allow both genfs (the current labeling 
behavior) and xattr labeling.  To support the latter, an fs_use_xattr 
statement needs to be added to policy for NFS:

http://namei.org/nfsv3xattr/v05/policy/

By default, mounts will still use genfs, unless the admin also supplies 
the new 'xattrsec' mount option, to indicate to the security module that 
it should use the XATTR protocol for labeling.  If XATTR is unavailable, 
the mount will fail (and not fall back to genfs).


This code still has several major todo items (mostly marked in the code), 
and needs much more testing, although I'd like to get feedback from the 
NFS and security folk on the current approach.

Comments welcome.


- James
-- 
James Morris
<jmorris@namei.org>

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

* [PATCH 1/8][RFC v05] NFSv3: convert client to generic xattr API
  2010-06-21 11:25 [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR) James Morris
@ 2010-06-21 11:27 ` James Morris
  2010-06-21 11:28 ` [PATCH 2/8][RFC v05] NFSv3: add xattr API config option for client James Morris
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:27 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

Convert existing NFSv3 client use (i.e. ACLs) of the xattrs to
the kernel's generic xattr API.

This helps simplify the code, and prepare for the subsequent
NFSv3 xattr protocol patches, which will also utilize the
generic xattr API.

Signed-off-by: James Morris <jmorris@namei.org>
---
 fs/nfs/dir.c           |    8 ++--
 fs/nfs/file.c          |    8 ++--
 fs/nfs/internal.h      |    4 ++
 fs/nfs/nfs3acl.c       |  122 ++++++++++++++++++++---------------------------
 fs/nfs/super.c         |    3 +
 include/linux/nfs_fs.h |   16 ------
 6 files changed, 67 insertions(+), 94 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 782b431..5fce66f 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -94,10 +94,10 @@ const struct inode_operations nfs3_dir_inode_operations = {
 	.permission	= nfs_permission,
 	.getattr	= nfs_getattr,
 	.setattr	= nfs_setattr,
-	.listxattr	= nfs3_listxattr,
-	.getxattr	= nfs3_getxattr,
-	.setxattr	= nfs3_setxattr,
-	.removexattr	= nfs3_removexattr,
+	.listxattr	= generic_listxattr,
+	.getxattr	= generic_getxattr,
+	.setxattr	= generic_setxattr,
+	.removexattr	= generic_removexattr,
 };
 #endif  /* CONFIG_NFS_V3 */
 
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 36a5e74..6f6349f 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -91,10 +91,10 @@ const struct inode_operations nfs3_file_inode_operations = {
 	.permission	= nfs_permission,
 	.getattr	= nfs_getattr,
 	.setattr	= nfs_setattr,
-	.listxattr	= nfs3_listxattr,
-	.getxattr	= nfs3_getxattr,
-	.setxattr	= nfs3_setxattr,
-	.removexattr	= nfs3_removexattr,
+	.listxattr	= generic_listxattr,
+	.getxattr	= generic_getxattr,
+	.setxattr	= generic_setxattr,
+	.removexattr	= generic_removexattr,
 };
 #endif  /* CONFIG_NFS_v3 */
 
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index d8bd619..733269c 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -4,6 +4,7 @@
 
 #include "nfs4_fs.h"
 #include <linux/mount.h>
+#include <linux/xattr.h>
 #include <linux/security.h>
 
 #define NFS_MS_MASK (MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_SYNCHRONOUS)
@@ -282,6 +283,9 @@ static inline char *nfs_devname(const struct vfsmount *mnt_parent,
 			dentry, buffer, buflen);
 }
 
+/* nfs3acl.c */
+extern const struct xattr_handler *nfs3_xattr_handlers[];
+
 /*
  * Determine the actual block size (and log2 thereof)
  */
diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 9f88c5f..27d1a44 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -10,64 +10,41 @@
 
 #define NFSDBG_FACILITY	NFSDBG_PROC
 
-ssize_t nfs3_listxattr(struct dentry *dentry, char *buffer, size_t size)
+static size_t nfs3_acl_xattr_list(struct dentry *dentry,
+				  char *list, size_t list_len,
+				  const char *name, size_t name_len,
+				  int acl_type)
 {
-	struct inode *inode = dentry->d_inode;
 	struct posix_acl *acl;
-	int pos=0, len=0;
+	char *acl_name = (acl_type == ACL_TYPE_ACCESS) ?
+		POSIX_ACL_XATTR_ACCESS : POSIX_ACL_XATTR_DEFAULT;
+	size_t size = strlen(acl_name) + 1;
 
-#	define output(s) do {						\
-			if (pos + sizeof(s) <= size) {			\
-				memcpy(buffer + pos, s, sizeof(s));	\
-				pos += sizeof(s);			\
-			}						\
-			len += sizeof(s);				\
-		} while(0)
+	acl = nfs3_proc_getacl(dentry->d_inode, acl_type);
+	if (!acl)
+		return 0;
 
-	acl = nfs3_proc_getacl(inode, ACL_TYPE_ACCESS);
 	if (IS_ERR(acl))
 		return PTR_ERR(acl);
-	if (acl) {
-		output("system.posix_acl_access");
-		posix_acl_release(acl);
-	}
 
-	if (S_ISDIR(inode->i_mode)) {
-		acl = nfs3_proc_getacl(inode, ACL_TYPE_DEFAULT);
-		if (IS_ERR(acl))
-			return PTR_ERR(acl);
-		if (acl) {
-			output("system.posix_acl_default");
-			posix_acl_release(acl);
-		}
-	}
-
-#	undef output
+	if (list && size <= list_len)
+		memcpy(list, acl_name, size);
 
-	if (!buffer || len <= size)
-		return len;
-	return -ERANGE;
+	posix_acl_release(acl);
+	return size;
 }
 
-ssize_t nfs3_getxattr(struct dentry *dentry, const char *name,
-		void *buffer, size_t size)
+static int nfs3_acl_xattr_get(struct dentry *dentry, const char *name,
+			      void *buffer, size_t size, int acl_type)
 {
-	struct inode *inode = dentry->d_inode;
 	struct posix_acl *acl;
-	int type, error = 0;
-
-	if (strcmp(name, POSIX_ACL_XATTR_ACCESS) == 0)
-		type = ACL_TYPE_ACCESS;
-	else if (strcmp(name, POSIX_ACL_XATTR_DEFAULT) == 0)
-		type = ACL_TYPE_DEFAULT;
-	else
-		return -EOPNOTSUPP;
+	int error = 0;
 
-	acl = nfs3_proc_getacl(inode, type);
+	acl = nfs3_proc_getacl(dentry->d_inode, acl_type);
 	if (IS_ERR(acl))
 		return PTR_ERR(acl);
 	else if (acl) {
-		if (type == ACL_TYPE_ACCESS && acl->a_count == 0)
+		if (acl_type == ACL_TYPE_ACCESS && acl->a_count == 0)
 			error = -ENODATA;
 		else
 			error = posix_acl_to_xattr(acl, buffer, size);
@@ -78,43 +55,48 @@ ssize_t nfs3_getxattr(struct dentry *dentry, const char *name,
 	return error;
 }
 
-int nfs3_setxattr(struct dentry *dentry, const char *name,
-	     const void *value, size_t size, int flags)
+static int nfs3_acl_xattr_set(struct dentry *dentry, const char *name,
+			      const void *value, size_t size, int flags,
+			      int acl_type)
 {
-	struct inode *inode = dentry->d_inode;
 	struct posix_acl *acl;
-	int type, error;
+	int error;
 
-	if (strcmp(name, POSIX_ACL_XATTR_ACCESS) == 0)
-		type = ACL_TYPE_ACCESS;
-	else if (strcmp(name, POSIX_ACL_XATTR_DEFAULT) == 0)
-		type = ACL_TYPE_DEFAULT;
-	else
-		return -EOPNOTSUPP;
+	if (value == NULL && (flags & XATTR_REPLACE))
+		acl = NULL;	/* remove xattr */
+	else {
+		acl = posix_acl_from_xattr(value, size);
+		if (IS_ERR(acl))
+			return PTR_ERR(acl);
+	}
 
-	acl = posix_acl_from_xattr(value, size);
-	if (IS_ERR(acl))
-		return PTR_ERR(acl);
-	error = nfs3_proc_setacl(inode, type, acl);
+	error = nfs3_proc_setacl(dentry->d_inode, acl_type, acl);
 	posix_acl_release(acl);
 
 	return error;
 }
 
-int nfs3_removexattr(struct dentry *dentry, const char *name)
-{
-	struct inode *inode = dentry->d_inode;
-	int type;
-
-	if (strcmp(name, POSIX_ACL_XATTR_ACCESS) == 0)
-		type = ACL_TYPE_ACCESS;
-	else if (strcmp(name, POSIX_ACL_XATTR_DEFAULT) == 0)
-		type = ACL_TYPE_DEFAULT;
-	else
-		return -EOPNOTSUPP;
-
-	return nfs3_proc_setacl(inode, type, NULL);
-}
+static struct xattr_handler nfs3_xattr_acl_access_handler = {
+	.prefix = POSIX_ACL_XATTR_ACCESS,
+	.flags	= ACL_TYPE_ACCESS,
+	.list   = nfs3_acl_xattr_list,
+	.get    = nfs3_acl_xattr_get,
+	.set    = nfs3_acl_xattr_set,
+};
+
+static struct xattr_handler nfs3_xattr_acl_default_handler = {
+	.prefix = POSIX_ACL_XATTR_DEFAULT,
+	.flags	= ACL_TYPE_DEFAULT,
+	.list   = nfs3_acl_xattr_list,
+	.get    = nfs3_acl_xattr_get,
+	.set    = nfs3_acl_xattr_set,
+};
+
+const struct xattr_handler *nfs3_xattr_handlers[] = {
+	&nfs3_xattr_acl_access_handler,
+	&nfs3_xattr_acl_default_handler,
+	NULL
+};
 
 static void __nfs3_forget_cached_acls(struct nfs_inode *nfsi)
 {
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 04214fc..69b55c0 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2023,6 +2023,9 @@ static void nfs_fill_super(struct super_block *sb,
 		 */
 		sb->s_flags |= MS_POSIXACL;
 		sb->s_time_gran = 1;
+#ifdef CONFIG_NFS_V3_ACL
+		sb->s_xattr = nfs3_xattr_handlers;
+#endif
 	}
 
 	sb->s_op = &nfs_sops;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 77c2ae5..9b0d2de 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -401,22 +401,6 @@ static inline struct rpc_cred *nfs_file_cred(struct file *file)
 }
 
 /*
- * linux/fs/nfs/xattr.c
- */
-#ifdef CONFIG_NFS_V3_ACL
-extern ssize_t nfs3_listxattr(struct dentry *, char *, size_t);
-extern ssize_t nfs3_getxattr(struct dentry *, const char *, void *, size_t);
-extern int nfs3_setxattr(struct dentry *, const char *,
-			const void *, size_t, int);
-extern int nfs3_removexattr (struct dentry *, const char *name);
-#else
-# define nfs3_listxattr NULL
-# define nfs3_getxattr NULL
-# define nfs3_setxattr NULL
-# define nfs3_removexattr NULL
-#endif
-
-/*
  * linux/fs/nfs/direct.c
  */
 extern ssize_t nfs_direct_IO(int, struct kiocb *, const struct iovec *, loff_t,
-- 
1.7.0.1


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

* [PATCH 2/8][RFC v05] NFSv3: add xattr API config option for client
  2010-06-21 11:25 [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR) James Morris
  2010-06-21 11:27 ` [PATCH 1/8][RFC v05] NFSv3: convert client to generic xattr API James Morris
@ 2010-06-21 11:28 ` James Morris
  2010-06-21 11:29 ` [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol James Morris
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:28 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

Add a separate configuration option for xattr API use by NFSv3 client
code, and make it independent of ACLs, so other NFSv3 client code (e.g.
the subsequent XATTR side-protocol) can use it.

Move the ACL handlers into the new xattr API file, where all such
handlers in the NFSv3 client code will live.

Signed-off-by: James Morris <jmorris@namei.org>
---
 fs/nfs/Kconfig     |    4 ++++
 fs/nfs/Makefile    |    1 +
 fs/nfs/internal.h  |    6 +++++-
 fs/nfs/nfs3acl.c   |   10 ++--------
 fs/nfs/nfs3xattr.c |   25 +++++++++++++++++++++++++
 5 files changed, 37 insertions(+), 9 deletions(-)
 create mode 100644 fs/nfs/nfs3xattr.c

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index a43d07e..e6c1e49 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -38,9 +38,13 @@ config NFS_V3
 
 	  If unsure, say Y.
 
+config NFS_V3_XATTR_API
+	def_bool n
+
 config NFS_V3_ACL
 	bool "NFS client support for the NFSv3 ACL protocol extension"
 	depends on NFS_V3
+	select NFS_V3_XATTR_API
 	help
 	  Some NFS servers support an auxiliary NFSv3 ACL protocol that
 	  Sun added to Solaris but never became an official part of the
diff --git a/fs/nfs/Makefile b/fs/nfs/Makefile
index da7fda6..1e2743e 100644
--- a/fs/nfs/Makefile
+++ b/fs/nfs/Makefile
@@ -11,6 +11,7 @@ nfs-y 			:= client.o dir.o file.o getroot.o inode.o super.o nfs2xdr.o \
 nfs-$(CONFIG_ROOT_NFS)	+= nfsroot.o
 nfs-$(CONFIG_NFS_V3)	+= nfs3proc.o nfs3xdr.o
 nfs-$(CONFIG_NFS_V3_ACL)	+= nfs3acl.o
+nfs-$(CONFIG_NFS_V3_XATTR_API)	+= nfs3xattr.o
 nfs-$(CONFIG_NFS_V4)	+= nfs4proc.o nfs4xdr.o nfs4state.o nfs4renewd.o \
 			   delegation.o idmap.o \
 			   callback.o callback_xdr.o callback_proc.o \
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 733269c..6c3c498 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -283,9 +283,13 @@ static inline char *nfs_devname(const struct vfsmount *mnt_parent,
 			dentry, buffer, buflen);
 }
 
-/* nfs3acl.c */
+/* nfsxattr.c */
 extern const struct xattr_handler *nfs3_xattr_handlers[];
 
+/* nfs3acl.c */
+extern struct xattr_handler nfs3_xattr_acl_access_handler;
+extern struct xattr_handler nfs3_xattr_acl_default_handler;
+
 /*
  * Determine the actual block size (and log2 thereof)
  */
diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 27d1a44..8aa0929 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -76,7 +76,7 @@ static int nfs3_acl_xattr_set(struct dentry *dentry, const char *name,
 	return error;
 }
 
-static struct xattr_handler nfs3_xattr_acl_access_handler = {
+struct xattr_handler nfs3_xattr_acl_access_handler = {
 	.prefix = POSIX_ACL_XATTR_ACCESS,
 	.flags	= ACL_TYPE_ACCESS,
 	.list   = nfs3_acl_xattr_list,
@@ -84,7 +84,7 @@ static struct xattr_handler nfs3_xattr_acl_access_handler = {
 	.set    = nfs3_acl_xattr_set,
 };
 
-static struct xattr_handler nfs3_xattr_acl_default_handler = {
+struct xattr_handler nfs3_xattr_acl_default_handler = {
 	.prefix = POSIX_ACL_XATTR_DEFAULT,
 	.flags	= ACL_TYPE_DEFAULT,
 	.list   = nfs3_acl_xattr_list,
@@ -92,12 +92,6 @@ static struct xattr_handler nfs3_xattr_acl_default_handler = {
 	.set    = nfs3_acl_xattr_set,
 };
 
-const struct xattr_handler *nfs3_xattr_handlers[] = {
-	&nfs3_xattr_acl_access_handler,
-	&nfs3_xattr_acl_default_handler,
-	NULL
-};
-
 static void __nfs3_forget_cached_acls(struct nfs_inode *nfsi)
 {
 	if (!IS_ERR(nfsi->acl_access)) {
diff --git a/fs/nfs/nfs3xattr.c b/fs/nfs/nfs3xattr.c
new file mode 100644
index 0000000..de69f1e
--- /dev/null
+++ b/fs/nfs/nfs3xattr.c
@@ -0,0 +1,25 @@
+/*
+ * Extended attribute (xattr) API and protocol for NFSv3.
+ *
+ * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/fs.h>
+#include <linux/nfs.h>
+#include <linux/nfs3.h>
+#include <linux/nfs_fs.h>
+
+#include "internal.h"
+
+#define NFSDBG_FACILITY	NFSDBG_PROC
+
+struct xattr_handler *nfs3_xattr_handlers[] = {
+#ifdef CONFIG_NFS_V3_ACL
+	&nfs3_xattr_acl_access_handler,
+	&nfs3_xattr_acl_default_handler,
+#endif
+	NULL
+};
-- 
1.7.0.1


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

* [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-21 11:25 [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR) James Morris
  2010-06-21 11:27 ` [PATCH 1/8][RFC v05] NFSv3: convert client to generic xattr API James Morris
  2010-06-21 11:28 ` [PATCH 2/8][RFC v05] NFSv3: add xattr API config option for client James Morris
@ 2010-06-21 11:29 ` James Morris
  2010-06-21 20:02   ` Chuck Lever
       [not found]   ` <alpine.LRH.2.00.1006212128160.13583-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
  2010-06-21 11:30 ` [PATCH 4/8][RFC v05] NFSv3: add server " James Morris
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:29 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

Add client support for the Linux NFSv3 extended attribute
side protocol (XATTR).

This extends Linux extended attributes over the network to
servers which support the protocol.

Operation is currently limited to the user.* namespace.

Signed-off-by: James Morris <jmorris@namei.org>
---
 fs/nfs/Kconfig            |   32 ++++++
 fs/nfs/Makefile           |    1 +
 fs/nfs/client.c           |   51 +++++++++-
 fs/nfs/internal.h         |   11 ++
 fs/nfs/nfs3xattr.c        |  241 ++++++++++++++++++++++++++++++++++++++++++++-
 fs/nfs/nfs3xattr_user.c   |   53 ++++++++++
 fs/nfs/nfs3xdr.c          |  187 +++++++++++++++++++++++++++++++++++
 fs/nfs/super.c            |    2 +-
 include/linux/nfs_fs_sb.h |    3 +-
 include/linux/nfs_mount.h |    3 +
 include/linux/nfs_xattr.h |   21 ++++
 include/linux/nfs_xdr.h   |   45 +++++++++
 12 files changed, 646 insertions(+), 4 deletions(-)
 create mode 100644 fs/nfs/nfs3xattr_user.c
 create mode 100644 include/linux/nfs_xattr.h

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index e6c1e49..b74faec 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -64,6 +64,38 @@ config NFS_V3_ACL
 
 	  If unsure, say N.
 
+config NFS_V3_XATTR
+	bool "NFS client support for the NFSv3 XATTR protocol extension (EXPERIMENTAL)"
+	depends on NFS_V3 && EXPERIMENTAL
+	select NFS_V3_XATTR_API
+	help
+	  This option selects client suport for the Linux NFSv3 extended
+	  attribute protocol extension (XATTR).
+
+	  This is a side-protocol which extends general support for Linux
+	  extended attributes over the network, and is based on the GPLd
+	  IRIX implmentation (although not wire-compatible with it).
+
+	  Only the user.* namespace is currently supported.  When connected
+	  to a server which also supports XATTR, the full range of extended
+	  attribute system calls:
+
+	    getxattr(2), listxattr(2), setxattr(2) and removexattr(2)
+
+	  should work as expected.
+
+	  If unsure, say N.
+
+config NFS_V3_XATTR_USER
+	bool "Extended attributes in the user namespace (EXPERIMENTAL)"
+	depends on NFS_V3_XATTR
+	help
+	  This option selects extended attributes in the user.* namespace,
+	  which are arbitrarily named and managed by users, and conveyed
+	  via the XATTR protocol extension for NFS version 3.
+
+	  If unsure, say N.
+
 config NFS_V4
 	bool "NFS client support for NFS version 4 (EXPERIMENTAL)"
 	depends on NFS_FS && EXPERIMENTAL
diff --git a/fs/nfs/Makefile b/fs/nfs/Makefile
index 1e2743e..54018ee 100644
--- a/fs/nfs/Makefile
+++ b/fs/nfs/Makefile
@@ -12,6 +12,7 @@ nfs-$(CONFIG_ROOT_NFS)	+= nfsroot.o
 nfs-$(CONFIG_NFS_V3)	+= nfs3proc.o nfs3xdr.o
 nfs-$(CONFIG_NFS_V3_ACL)	+= nfs3acl.o
 nfs-$(CONFIG_NFS_V3_XATTR_API)	+= nfs3xattr.o
+nfs-$(CONFIG_NFS_V3_XATTR_USER)	+= nfs3xattr_user.o
 nfs-$(CONFIG_NFS_V4)	+= nfs4proc.o nfs4xdr.o nfs4state.o nfs4renewd.o \
 			   delegation.o idmap.o \
 			   callback.o callback_xdr.o callback_proc.o \
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 7ec9b34..b5adf31 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -98,6 +98,21 @@ struct rpc_program		nfsacl_program = {
 };
 #endif  /* CONFIG_NFS_V3_ACL */
 
+#ifdef CONFIG_NFS_V3_XATTR
+static struct rpc_stat		nfs_xattr_rpcstat = { &nfs_xattr_program };
+static struct rpc_version *	nfs_xattr_version[] = {
+	[3]			= &nfs_xattr_version3,
+};
+
+struct rpc_program		nfs_xattr_program = {
+	.name			= "nfsxattr",
+	.number			= NFS_XATTR_PROGRAM,
+	.nrvers			= ARRAY_SIZE(nfs_xattr_version),
+	.version		= nfs_xattr_version,
+	.stats			= &nfs_xattr_rpcstat,
+};
+#endif  /* CONFIG_NFS_V3_XATTR */
+
 struct nfs_client_initdata {
 	const char *hostname;
 	const struct sockaddr *addr;
@@ -707,6 +722,36 @@ static inline void nfs_init_server_aclclient(struct nfs_server *server)
 #endif
 
 /*
+ * Initialise an NFSv3 XATTR client connection
+ */
+#ifdef CONFIG_NFS_V3_XATTR
+static void nfs_init_server_xattrclient(struct nfs_server *server)
+{
+	if (server->nfs_client->rpc_ops->version != 3)
+		goto out_no_xattr;
+	if (server->flags & NFS_MOUNT_NOXATTR)
+		goto out_no_xattr;
+
+	server->client_xattr = rpc_bind_new_program(server->client, &nfs_xattr_program, 3);
+	if (IS_ERR(server->client_xattr))
+		goto out_no_xattr;
+
+	/* No errors! Assume that XATTR is supported */
+	server->caps |= NFS_CAP_XATTR;
+	return;
+
+out_no_xattr:
+	server->caps &= ~NFS_CAP_XATTR;
+}
+#else
+static inline void nfs_init_server_xattrclient(struct nfs_server *server)
+{
+	server->flags &= ~NFS_MOUNT_NOXATTR;
+	server->caps &= ~NFS_CAP_XATTR;
+}
+#endif
+
+/*
  * Create a general RPC client
  */
 static int nfs_init_server_rpcclient(struct nfs_server *server,
@@ -852,8 +897,12 @@ static int nfs_init_server(struct nfs_server *server,
 	server->mountd_protocol = data->mount_server.protocol;
 
 	server->namelen  = data->namlen;
-	/* Create a client RPC handle for the NFSv3 ACL management interface */
+	/*
+	 * Create client RPC handles for the NFSv3 ACL and XATTR management
+	 * interfaces
+	 */
 	nfs_init_server_aclclient(server);
+	nfs_init_server_xattrclient(server);
 	dprintk("<-- nfs_init_server() = 0 [new %p]\n", clp);
 	return 0;
 
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 6c3c498..1a4f777 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -286,6 +286,17 @@ static inline char *nfs_devname(const struct vfsmount *mnt_parent,
 /* nfsxattr.c */
 extern const struct xattr_handler *nfs3_xattr_handlers[];
 
+extern int nfs3_proc_getxattr(struct inode *inode, const char *namespace,
+			      const char *name, void *value, size_t size);
+extern int nfs3_proc_setxattr(struct inode *inode, const char *namespace,
+			      const char *name, const void *value,
+			      size_t size, int flags);
+extern int nfs3_proc_listxattr(struct inode *inode, char *list,
+			       size_t list_len);
+
+/* nfs3xattr_user.c */
+extern struct xattr_handler nfs3_xattr_user_handler;
+
 /* nfs3acl.c */
 extern struct xattr_handler nfs3_xattr_acl_access_handler;
 extern struct xattr_handler nfs3_xattr_acl_default_handler;
diff --git a/fs/nfs/nfs3xattr.c b/fs/nfs/nfs3xattr.c
index de69f1e..f34b9a0 100644
--- a/fs/nfs/nfs3xattr.c
+++ b/fs/nfs/nfs3xattr.c
@@ -1,6 +1,8 @@
 /*
  * Extended attribute (xattr) API and protocol for NFSv3.
  *
+ * Based on the ACL code.
+ *
  * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris@redhat.com>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -16,10 +18,247 @@
 
 #define NFSDBG_FACILITY	NFSDBG_PROC
 
-struct xattr_handler *nfs3_xattr_handlers[] = {
+const struct xattr_handler *nfs3_xattr_handlers[] = {
+#ifdef CONFIG_NFS_V3_XATTR_USER
+	&nfs3_xattr_user_handler,
+#endif
 #ifdef CONFIG_NFS_V3_ACL
 	&nfs3_xattr_acl_access_handler,
 	&nfs3_xattr_acl_default_handler,
 #endif
 	NULL
 };
+
+#ifdef CONFIG_NFS_V3_XATTR
+/*
+ * XATTR protocol
+ */
+
+/*
+ * Call GETXATTR
+ *
+ * FIXME:
+ * - Cache xattrs
+ * - Handle size probing
+ */
+int nfs3_proc_getxattr(struct inode *inode, const char *namespace,
+		       const char *name, void *value, size_t size)
+{
+	int status;
+	struct nfs_fattr fattr;
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct nfs3_getxattrargs args = {
+		.fh		= NFS_FH(inode),
+	};
+	struct nfs3_getxattrres res = {
+		.fattr		= &fattr,
+	};
+	struct rpc_message msg = {
+		.rpc_argp	= &args,
+		.rpc_resp	= &res,
+	};
+
+	if (!name || !*name)
+		return -EINVAL;
+
+	if (size > XATTR_SIZE_MAX)
+		return -EINVAL;
+
+	if (!nfs_server_capable(inode, NFS_CAP_XATTR))
+		return -EOPNOTSUPP;
+
+	status = nfs_revalidate_inode(server, inode);
+	if (status < 0)
+		return status;
+
+	/*
+	 * Applications usually first probe the xattr value size, then
+	 * perform a full call.  For now, just return a dummy value.
+	 */
+	if (!size || !value)
+		return 4096;
+
+	args.xattr_namespace = namespace;
+	args.xattr_name = name;
+	args.xattr_size_max = size;
+
+	/*
+	 * FIXME
+	 *
+	 * This is ugly.  We pre-allocate a buffer for the XDR layer to use,
+	 * passing the size of the buffer via xattr_val_len, which is
+	 * updated with the actual length decoded.  We should investigate
+	 * using the page-based interface used by ACLs and others, or some
+	 * other better way.
+	 */
+	res.xattr_val_len = size;
+	res.xattr_val = kmalloc(size, GFP_KERNEL);
+	if (!res.xattr_val)
+		return -ENOMEM;
+
+	dprintk("NFS call getxattr %s%s %zd\n", namespace, name, size);
+
+	msg.rpc_proc = &server->client_xattr->cl_procinfo[XATTRPROC3_GETXATTR];
+	nfs_fattr_init(&fattr);
+	status = rpc_call_sync(server->client_xattr, &msg, 0);
+
+	dprintk("NFS reply getxattr: status=%d len=%d\n",
+		status, res.xattr_val_len);
+
+	switch (status) {
+	case 0:
+		status = nfs_refresh_inode(inode, &fattr);
+		break;
+	case -EPFNOSUPPORT:
+	case -EPROTONOSUPPORT:
+		dprintk("NFS_V3_XATTR extension not supported; disabling\n");
+		server->caps &= ~NFS_CAP_XATTR;
+	case -ENOTSUPP:
+		status = -EOPNOTSUPP;
+	default:
+		goto cleanup;
+	}
+
+	status = res.xattr_val_len;
+	if (status <= size)
+		memcpy(value, res.xattr_val, status);
+
+cleanup:
+	kfree(res.xattr_val);
+	return status;
+}
+
+/*
+ * Call SETXATTR or RMXATTR
+ *
+ * RMXATTR is invoked with a NULL buffer and XATTR_REPLACE.
+ *
+ */
+int nfs3_proc_setxattr(struct inode *inode, const char *namespace,
+		       const char *name, const void *value,
+		       size_t size, int flags)
+
+{
+	int status;
+	struct nfs_fattr fattr;
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct nfs3_setxattrargs args = {
+		.fh		= NFS_FH(inode),
+	};
+	struct nfs3_setxattrres res = {
+		.fattr		= &fattr,
+	};
+	struct rpc_message msg = {
+		.rpc_argp	= &args,
+		.rpc_resp	= &res,
+	};
+
+	if (!name || !*name)
+		return -EINVAL;
+
+	if (!nfs_server_capable(inode, NFS_CAP_XATTR))
+		return -EOPNOTSUPP;
+
+	status = nfs_revalidate_inode(server, inode);
+	if (status < 0)
+		return status;
+
+	args.xattr_namespace = namespace;
+	args.xattr_name = name;
+	args.xattr_flags = flags;
+	args.xattr_val = value;
+	args.xattr_val_len = size;
+
+	dprintk("NFS call setxattr %s%s %zd 0x%08x\n",
+		namespace, name, size, flags);
+
+	msg.rpc_proc = &server->client_xattr->cl_procinfo[XATTRPROC3_SETXATTR];
+	nfs_fattr_init(&fattr);
+	status = rpc_call_sync(server->client_xattr, &msg, 0);
+
+	dprintk("NFS reply setxattr: status=%d\n", status);
+
+	switch (status) {
+	case 0:
+		status = nfs_refresh_inode(inode, &fattr);
+		break;
+	case -EPFNOSUPPORT:
+	case -EPROTONOSUPPORT:
+		dprintk("NFS_V3_XATTR extension not supported; disabling\n");
+		server->caps &= ~NFS_CAP_XATTR;
+	case -ENOTSUPP:
+		status = -EOPNOTSUPP;
+	default:
+		break;
+	}
+	return status;
+}
+
+/*
+ * Call LISTXATTR
+ */
+int nfs3_proc_listxattr(struct inode *inode, char *list, size_t list_len)
+{
+	int status;
+	struct nfs_fattr fattr;
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct nfs3_listxattrargs args = {
+		.fh		= NFS_FH(inode),
+	};
+	struct nfs3_listxattrres res = {
+		.fattr		= &fattr,
+	};
+	struct rpc_message msg = {
+		.rpc_argp	= &args,
+		.rpc_resp	= &res,
+	};
+
+	if (list_len > XATTR_LIST_MAX)
+		return -EINVAL;
+
+	if (!nfs_server_capable(inode, NFS_CAP_XATTR))
+		return -EOPNOTSUPP;
+
+	dprintk("NFS call listxattr %zd\n", list_len);
+
+	/* FIXME: handle probes */
+	if (!list || !list_len)
+		return 1024;
+
+	args.xattr_list_max = list_len;
+
+	/* FIXME (see comments for getxattr) */
+	res.xattr_list_len = list_len;
+	res.xattr_list = kmalloc(list_len, GFP_KERNEL);
+	if (!res.xattr_list)
+		return -ENOMEM;
+
+	msg.rpc_proc = &server->client_xattr->cl_procinfo[XATTRPROC3_LISTXATTR];
+	nfs_fattr_init(&fattr);
+	status = rpc_call_sync(server->client_xattr, &msg, 0);
+
+	dprintk("NFS reply listxattr: status=%d\n", status);
+
+	switch (status) {
+	case 0:
+		status = nfs_refresh_inode(inode, &fattr);
+		break;
+	case -EPFNOSUPPORT:
+	case -EPROTONOSUPPORT:
+		dprintk("NFS_V3_XATTR extension not supported; disabling\n");
+		server->caps &= ~NFS_CAP_XATTR;
+	case -ENOTSUPP:
+		status = -EOPNOTSUPP;
+	default:
+		goto cleanup;
+	}
+
+	status = res.xattr_list_len;
+	if (status <= list_len)
+		memcpy(list, res.xattr_list, status);
+
+cleanup:
+	kfree(res.xattr_list);
+	return status;
+}
+#endif	/* CONFIG_NFS_V3_XATTR */
diff --git a/fs/nfs/nfs3xattr_user.c b/fs/nfs/nfs3xattr_user.c
new file mode 100644
index 0000000..c544fcb
--- /dev/null
+++ b/fs/nfs/nfs3xattr_user.c
@@ -0,0 +1,53 @@
+/*
+ * Support for extended attributes in the the user.* namespace, which are
+ * arbitrarily named and managed by users and conveyed via the XATTR
+ * protocol extension.
+ *
+ * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/fs.h>
+#include <linux/nfs.h>
+#include <linux/nfs3.h>
+#include <linux/nfs_fs.h>
+
+#include "internal.h"
+
+#define NFSDBG_FACILITY	NFSDBG_PROC
+
+/*
+ * The generic xattr code will call this for each helper, which is ok for
+ * now, because we only support this single namespace.  If support is
+ * expanded to more namespaces, we we'll need a custom listxattr operation.
+ */
+static size_t nfs3_user_xattr_list(struct dentry *dentry, char *list,
+				   size_t list_len, const char *name,
+				   size_t name_len, int hflags)
+{
+	return nfs3_proc_listxattr(dentry->d_inode, list, list_len);
+}
+
+static int nfs3_user_xattr_get(struct dentry *dentry, const char *name,
+			       void *buffer, size_t size, int hflags)
+{
+	return nfs3_proc_getxattr(dentry->d_inode, XATTR_USER_PREFIX,
+				  name, buffer, size);
+}
+
+static int nfs3_user_xattr_set(struct dentry *dentry, const char *name,
+			       const void *value, size_t size,
+			       int flags, int hflags)
+{
+	return nfs3_proc_setxattr(dentry->d_inode, XATTR_USER_PREFIX,
+				  name, value, size, flags);
+}
+
+struct xattr_handler nfs3_xattr_user_handler = {
+	.prefix = XATTR_USER_PREFIX,
+	.list   = nfs3_user_xattr_list,
+	.get    = nfs3_user_xattr_get,
+	.set    = nfs3_user_xattr_set,
+};
diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
index 75dcfc7..08d5ec9 100644
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -87,6 +87,26 @@
 #define ACL3_setaclres_sz	(1+NFS3_post_op_attr_sz)
 
 /*
+ * FIXME: currently, the RPC layer will allocate the maximum buffer size
+ * here for each call (which can be ~ 64k).  The Labeled NFS prototype code
+ * uses 4k, although we should not impose limits for NFS which don't exist
+ * in the OS unless absolutely necsssary.  We likely need a dynamic scheme
+ * here, possibly using pages.
+ */
+#define XATTR3_xattrname_sz	(1+(XATTR_NAME_MAX>>2))
+#define XATTR3_xattrval_sz	(1+(XATTR_SIZE_MAX>>2))
+#define XATTR3_xattrlist_sz	(1+(XATTR_LIST_MAX>>2))
+
+#define XATTR3_getxattrargs_sz	(NFS3_fh_sz+XATTR3_xattrname_sz+1)
+#define XATTR3_getxattrres_sz	(1+NFS3_post_op_attr_sz+XATTR3_xattrval_sz)
+
+#define XATTR3_setxattrargs_sz	(NFS3_fh_sz+XATTR3_xattrname_sz+XATTR3_xattrval_sz+1)
+#define XATTR3_setxattrres_sz	(1+NFS3_post_op_attr_sz)
+
+#define XATTR3_listxattrargs_sz	(NFS3_fh_sz+1)
+#define XATTR3_listxattrres_sz	(1+NFS3_post_op_attr_sz+XATTR3_xattrlist_sz)
+
+/*
  * Map file type to S_IFMT bits
  */
 static const umode_t nfs_type2fmt[] = {
@@ -726,6 +746,72 @@ nfs3_xdr_setaclargs(struct rpc_rqst *req, __be32 *p,
 }
 #endif  /* CONFIG_NFS_V3_ACL */
 
+#ifdef CONFIG_NFS_V3_XATTR
+/*
+ * Special case of xdr_encode_opaque, where the xattr helpers hand us
+ * separate namespace and name buffers, which we encode as a single XDR
+ * string over the wire.  Neither namespace nor name may be empty or null.
+ */
+static __be32 *xattr_encode_name(__be32 *p, const char *namespace, const char *name)
+{
+	unsigned int nslen, namelen, totlen, quadlen, padding;
+
+	nslen = strlen(namespace);
+	namelen = strlen(name);
+	totlen = nslen + namelen;
+	quadlen = XDR_QUADLEN(totlen);
+	padding = (quadlen << 2) - totlen;
+
+	*p++ = cpu_to_be32(totlen);
+	memcpy(p, namespace, nslen);
+	memcpy((char *)p + nslen, name, namelen);
+
+	if (padding != 0)
+		memset((char *)p + totlen, 0, padding);
+	p += quadlen;
+	return p;
+}
+
+/*
+ * Encode GETXATTR arguments
+ */
+static int nfs3_xdr_getxattrargs(struct rpc_rqst *req, __be32 *p,
+				 struct nfs3_getxattrargs *args)
+{
+	p = xdr_encode_fhandle(p, args->fh);
+	p = xattr_encode_name(p, args->xattr_namespace, args->xattr_name);
+	*p++ = htonl(args->xattr_size_max);
+	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
+	return 0;
+}
+
+/*
+ * Encode SETXATTR arguments
+ */
+static int nfs3_xdr_setxattrargs(struct rpc_rqst *req, __be32 *p,
+				 struct nfs3_setxattrargs *args)
+{
+	p = xdr_encode_fhandle(p, args->fh);
+	p = xattr_encode_name(p, args->xattr_namespace, args->xattr_name);
+	p = xdr_encode_array(p, args->xattr_val, args->xattr_val_len);
+	*p++ = htonl(args->xattr_flags);
+	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
+	return 0;
+}
+
+/*
+ * Encode LISTXATTR arguments
+ */
+static int nfs3_xdr_listxattrargs(struct rpc_rqst *req, __be32 *p,
+				  struct nfs3_listxattrargs *args)
+{
+	p = xdr_encode_fhandle(p, args->fh);
+	*p++ = htonl(args->xattr_list_max);
+	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
+	return 0;
+}
+#endif	/* CONFIG_NFS_V3_XATTR */
+
 /*
  * NFS XDR decode functions
  */
@@ -1135,6 +1221,69 @@ nfs3_xdr_setaclres(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 }
 #endif  /* CONFIG_NFS_V3_ACL */
 
+#ifdef CONFIG_NFS_V3_XATTR
+/*
+ * Decode GETXATTR reply
+ *
+ * FIXME: determine appropriate error returns
+ */
+static int nfs3_xdr_getxattrres(struct rpc_rqst *req, __be32 *p,
+				struct nfs3_getxattrres *res)
+{
+	char *xattr_val;
+	unsigned int xattr_max_size = res->xattr_val_len;
+	int status = ntohl(*p++);
+
+	if (status != 0)
+		return nfs_stat_to_errno(status);
+
+	p = xdr_decode_post_op_attr(p, res->fattr);
+	p = xdr_decode_string_inplace(p, &xattr_val,
+	                              &res->xattr_val_len,
+	                              xattr_max_size);
+	if (p == NULL)
+		return -EINVAL;
+	memcpy(res->xattr_val, xattr_val, res->xattr_val_len);
+	return 0;
+}
+
+/*
+ * Decode SETXATTR reply
+ */
+static int nfs3_xdr_setxattrres(struct rpc_rqst *req, __be32 *p,
+				struct nfs3_setxattrres *res)
+{
+	int status = ntohl(*p++);
+
+	if (status)
+		return nfs_stat_to_errno(status);
+	xdr_decode_post_op_attr(p, res->fattr);
+	return 0;
+}
+
+/*
+ * Decode LISTXATTR reply
+ */
+static int nfs3_xdr_listxattrres(struct rpc_rqst *req, __be32 *p,
+				 struct nfs3_listxattrres *res)
+{
+	char *xattr_list;
+	unsigned int size = res->xattr_list_len;
+	int status = ntohl(*p++);
+
+	if (status != 0)
+		return nfs_stat_to_errno(status);
+
+	p = xdr_decode_post_op_attr(p, res->fattr);
+	p = xdr_decode_string_inplace(p, &xattr_list,
+	                              &res->xattr_list_len, size);
+	if (p == NULL)
+		return -EINVAL;
+	memcpy(res->xattr_list, xattr_list, res->xattr_list_len);
+	return 0;
+}
+#endif	/* CONFIG_NFS_V3_XATTR */
+
 #define PROC(proc, argtype, restype, timer)				\
 [NFS3PROC_##proc] = {							\
 	.p_proc      = NFS3PROC_##proc,					\
@@ -1206,3 +1355,41 @@ struct rpc_version		nfsacl_version3 = {
 	.procs			= nfs3_acl_procedures,
 };
 #endif  /* CONFIG_NFS_V3_ACL */
+
+#ifdef CONFIG_NFS_V3_XATTR
+static struct rpc_procinfo	nfs3_xattr_procedures[] = {
+	[XATTRPROC3_GETXATTR] = {
+		.p_proc		= XATTRPROC3_GETXATTR,
+		.p_encode	= (kxdrproc_t) nfs3_xdr_getxattrargs,
+		.p_decode	= (kxdrproc_t) nfs3_xdr_getxattrres,
+		.p_arglen	= XATTR3_getxattrargs_sz,
+		.p_replen	= XATTR3_getxattrres_sz,
+		.p_timer	= 1,
+		.p_name		= "GETXATTR",
+	},
+	[XATTRPROC3_SETXATTR] = {
+		.p_proc		= XATTRPROC3_SETXATTR,
+		.p_encode	= (kxdrproc_t) nfs3_xdr_setxattrargs,
+		.p_decode	= (kxdrproc_t) nfs3_xdr_setxattrres,
+		.p_arglen	= XATTR3_setxattrargs_sz,
+		.p_replen	= XATTR3_setxattrres_sz,
+		.p_timer	= 1,
+		.p_name		= "SETXATTR",
+	},
+	[XATTRPROC3_LISTXATTR] = {
+		.p_proc		= XATTRPROC3_LISTXATTR,
+		.p_encode	= (kxdrproc_t) nfs3_xdr_listxattrargs,
+		.p_decode	= (kxdrproc_t) nfs3_xdr_listxattrres,
+		.p_arglen	= XATTR3_listxattrargs_sz,
+		.p_replen	= XATTR3_listxattrres_sz,
+		.p_timer	= 1,
+		.p_name		= "LISTXATTR",
+	},
+};
+
+struct rpc_version		nfs_xattr_version3 = {
+	.number			= 3,
+	.nrprocs		= ARRAY_SIZE(nfs3_xattr_procedures),
+	.procs			= nfs3_xattr_procedures,
+};
+#endif	/* CONFIG_NFS_V3_XATTR */
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 69b55c0..62f8db8 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2023,7 +2023,7 @@ static void nfs_fill_super(struct super_block *sb,
 		 */
 		sb->s_flags |= MS_POSIXACL;
 		sb->s_time_gran = 1;
-#ifdef CONFIG_NFS_V3_ACL
+#if defined(CONFIG_NFS_V3_ACL) || defined (CONFIG_NFS_V3_XATTR_USER)
 		sb->s_xattr = nfs3_xattr_handlers;
 #endif
 	}
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index d6e10a4..dde0d66 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -103,6 +103,7 @@ struct nfs_server {
 	struct list_head	master_link;	/* link in master servers list */
 	struct rpc_clnt *	client;		/* RPC client handle */
 	struct rpc_clnt *	client_acl;	/* ACL RPC client handle */
+	struct rpc_clnt *	client_xattr;	/* XATTR RPC client handle */
 	struct nlm_host		*nlm_host;	/* NLM client handle */
 	struct nfs_iostats __percpu *io_stats;	/* I/O statistics */
 	struct backing_dev_info	backing_dev_info;
@@ -176,7 +177,7 @@ struct nfs_server {
 #define NFS_CAP_CTIME		(1U << 12)
 #define NFS_CAP_MTIME		(1U << 13)
 #define NFS_CAP_POSIX_LOCK	(1U << 14)
-
+#define NFS_CAP_XATTR		(1U << 15)
 
 /* maximum number of slots to use */
 #define NFS4_MAX_SLOT_TABLE RPC_MAX_SLOT_TABLE
diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
index 4499016..04bb4ee 100644
--- a/include/linux/nfs_mount.h
+++ b/include/linux/nfs_mount.h
@@ -70,4 +70,7 @@ struct nfs_mount_data {
 #define NFS_MOUNT_LOOKUP_CACHE_NONE	0x20000
 #define NFS_MOUNT_NORESVPORT		0x40000
 
+/* FIXME: determine semantics and modify flagmask if exposed to userland */
+#define NFS_MOUNT_NOXATTR		0x80000
+
 #endif
diff --git a/include/linux/nfs_xattr.h b/include/linux/nfs_xattr.h
new file mode 100644
index 0000000..98fdbed
--- /dev/null
+++ b/include/linux/nfs_xattr.h
@@ -0,0 +1,21 @@
+/*
+ * Extended attribute protocol for NFSv3 (XATTR)
+ *
+ *
+ * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris@redhat.com>
+ *
+ */
+#ifndef __LINUX_NFS_XATTR_H
+#define __LINUX_NFS_XATTR_H
+
+#include <linux/xattr.h>
+
+#define NFS_XATTR_PROGRAM	391063	/* TODO: find another value */
+
+/* xattr procedure numbers */
+#define XATTRPROC3_GETXATTR	1
+#define XATTRPROC3_SETXATTR	2
+#define XATTRPROC3_LISTXATTR	3
+#define XATTRPROC3_RMXATTR	4
+
+#endif  /* __LINUX_NFS_XATTR_H */
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 51914d7..b632fe6 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -2,6 +2,7 @@
 #define _LINUX_NFS_XDR_H
 
 #include <linux/nfsacl.h>
+#include <linux/nfs_xattr.h>
 #include <linux/nfs3.h>
 
 /*
@@ -514,6 +515,27 @@ struct nfs3_setaclargs {
 	struct page **		pages;
 };
 
+struct nfs3_getxattrargs {
+	struct nfs_fh *		fh;
+	const char *		xattr_namespace;
+	const char *		xattr_name;
+	unsigned int		xattr_size_max;
+};
+
+struct nfs3_setxattrargs {
+	struct nfs_fh *		fh;
+	unsigned int		xattr_flags;
+	const char *		xattr_namespace;
+	const char *		xattr_name;
+	const char *		xattr_val;
+	int			xattr_val_len;
+};
+
+struct nfs3_listxattrargs {
+	struct nfs_fh *		fh;
+	unsigned int		xattr_list_max;
+};
+
 struct nfs_diropok {
 	struct nfs_fh *		fh;
 	struct nfs_fattr *	fattr;
@@ -646,6 +668,26 @@ struct nfs3_getaclres {
 	struct posix_acl *	acl_default;
 };
 
+struct nfs3_getxattrres {
+	struct nfs_fattr *	fattr;
+	char *			xattr_val;
+	int			xattr_val_len;
+};
+
+/*
+ * Note: if we don't add any more fields, we can get rid of this struct and
+ * just use fattr in the calling code.
+ */
+struct nfs3_setxattrres {
+	struct nfs_fattr *	fattr;
+};
+
+struct nfs3_listxattrres {
+	struct nfs_fattr *	fattr;
+	char *			xattr_list;
+	int			xattr_list_len;
+};
+
 #ifdef CONFIG_NFS_V4
 
 typedef u64 clientid4;
@@ -1079,4 +1121,7 @@ extern struct rpc_version	nfs_version4;
 extern struct rpc_version	nfsacl_version3;
 extern struct rpc_program	nfsacl_program;
 
+extern struct rpc_version	nfs_xattr_version3;
+extern struct rpc_program	nfs_xattr_program;
+
 #endif
-- 
1.7.0.1


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

* [PATCH 4/8][RFC v05] NFSv3: add server implementation of XATTR protocol
  2010-06-21 11:25 [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR) James Morris
                   ` (2 preceding siblings ...)
  2010-06-21 11:29 ` [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol James Morris
@ 2010-06-21 11:30 ` James Morris
  2010-06-21 11:30 ` [PATCH 5/8][RFC v05] XATTR: add new top level nfsd namespace and implement ext3 support James Morris
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:30 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

Add server support for the Linux NFSv3 extended attribute
side protocol (XATTR).

Signed-off-by: James Morris <jmorris@namei.org>
---
 fs/nfsd/Kconfig            |    8 +
 fs/nfsd/Makefile           |    1 +
 fs/nfsd/nfs3xattr.c        |  354 ++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfsctl.c           |    3 +
 fs/nfsd/nfssvc.c           |   60 +++++++-
 fs/nfsd/vfs.c              |    5 +-
 fs/nfsd/vfs.h              |   13 ++
 fs/nfsd/xdr3.h             |   46 ++++++
 include/linux/sunrpc/svc.h |    2 +-
 9 files changed, 486 insertions(+), 6 deletions(-)
 create mode 100644 fs/nfsd/nfs3xattr.c

diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 503b9da..4252d16 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -64,6 +64,14 @@ config NFSD_V3_ACL
 
 	  If unsure, say N.
 
+config NFSD_V3_XATTR
+	bool "NFS server support for the NFSv3 XATTR protocol extension (EXPERIMENTAL)"
+	depends on NFSD_V3 && EXPERIMENTAL
+	help
+	  NFS server support for the NFSv3 XATTR protocol.
+
+	  If unsure, say N.
+
 config NFSD_V4
 	bool "NFS server support for NFS version 4 (EXPERIMENTAL)"
 	depends on NFSD && PROC_FS && EXPERIMENTAL
diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
index 9b118ee..e206b52 100644
--- a/fs/nfsd/Makefile
+++ b/fs/nfsd/Makefile
@@ -9,5 +9,6 @@ nfsd-y 			:= nfssvc.o nfsctl.o nfsproc.o nfsfh.o vfs.o \
 nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
 nfsd-$(CONFIG_NFSD_V3)	+= nfs3proc.o nfs3xdr.o
 nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
+nfsd-$(CONFIG_NFSD_V3_XATTR) += nfs3xattr.o
 nfsd-$(CONFIG_NFSD_V4)	+= nfs4proc.o nfs4xdr.o nfs4state.o nfs4idmap.o \
 			   nfs4acl.o nfs4callback.o nfs4recover.o
diff --git a/fs/nfsd/nfs3xattr.c b/fs/nfsd/nfs3xattr.c
new file mode 100644
index 0000000..b5a5faa
--- /dev/null
+++ b/fs/nfsd/nfs3xattr.c
@@ -0,0 +1,354 @@
+/*
+ * Process version 3 NFSXATTR requests.
+ *
+ * Based on the NFSACL code by:
+ * Copyright (C) 2002-2003 Andreas Gruenbacher <agruen@suse.de>
+ *
+ * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/sunrpc/svc.h>
+#include <linux/nfs3.h>
+#include <linux/xattr.h>
+#include <linux/nfs_xattr.h>
+
+#include "nfsd.h"
+#include "xdr3.h"
+#include "vfs.h"
+#include "cache.h"
+
+#define NFSDDBG_FACILITY	NFSDDBG_PROC
+#define RETURN_STATUS(st)	{ resp->status = (st); return (st); }
+
+/* NULL call */
+static __be32 nfsd3_proc_null(struct svc_rqst *rqstp, void *argp, void *resp)
+{
+	return nfs_ok;
+}
+
+/*
+ * GETXATTR
+ *
+ * FIXME:
+ *  - Implement shared xattr cache
+ *  - Audit nfs error returns
+ */
+static __be32 nfsd3_proc_getxattr(struct svc_rqst * rqstp,
+				  struct nfsd3_getxattrargs *argp,
+				  struct nfsd3_getxattrres *resp)
+{
+	__be32 nfserr = nfserrno(-EINVAL);
+	svc_fh *fh;
+	void *value;
+	int ret;
+	char *name, *xattr_name = argp->xattr_name;
+	unsigned int size_max = argp->xattr_size_max;
+	unsigned int name_len = argp->xattr_name_len;
+
+	dprintk("nfsd: GETXATTR(3)  %s %.*s %u\n", SVCFH_fmt(&argp->fh),
+	        name_len, xattr_name, size_max);
+
+	if (name_len > XATTR_NAME_MAX)
+		RETURN_STATUS(nfserr);
+
+	if (size_max > XATTR_SIZE_MAX)
+		RETURN_STATUS(nfserr);
+
+	/* Probes must be handled by the client */
+	if (size_max == 0)
+		RETURN_STATUS(nfserr);
+
+	fh = fh_copy(&resp->fh, &argp->fh);
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_READ);
+	if (nfserr)
+		RETURN_STATUS(nfserr);
+
+	/* Convert xdr string to real string */
+	name = kmalloc(name_len + 1, GFP_KERNEL);
+	if (name == NULL)
+		RETURN_STATUS(nfserrno(-ENOMEM));
+
+	ret = snprintf(name, name_len + 1, "%.*s", name_len, xattr_name);
+	if (ret > name_len) {
+		nfserr = nfserrno(-EINVAL);
+		goto cleanup;
+	}
+
+	/* Only the user namespace is currently supported by the server */
+	if (strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
+		nfserr = nfserrno(-EINVAL);
+		goto cleanup;
+	}
+
+	ret = nfsd_getxattr(fh->fh_dentry, name, &value);
+	if (ret <= 0) {
+		if (ret == 0)
+			ret = -ENODATA;
+		nfserr = nfserrno(ret);
+		goto cleanup;
+	}
+
+	nfserr = 0;
+	resp->xattr_val = value;
+	resp->xattr_val_len = ret;
+
+cleanup:
+	kfree(name);
+	RETURN_STATUS(nfserr);
+}
+
+/* cribbed from decode pathname */
+static __be32 *decode_xattrname(__be32 *p, char **namp, unsigned int *lenp)
+{
+	char *name;
+	unsigned int i;
+
+	p = xdr_decode_string_inplace(p, namp, lenp, XATTR_NAME_MAX);
+	if (p != NULL)
+		for (i = 0, name = *namp; i < *lenp; i++, name++)
+			if (*name == '\0')
+				return NULL;
+	return p;
+}
+
+static int nfs3svc_decode_getxattrargs(struct svc_rqst *rqstp, __be32 *p,
+				       struct nfsd3_getxattrargs *argp)
+{
+	if (!(p = nfs3svc_decode_fh(p, &argp->fh)))
+		return 0;
+	if (!(p = decode_xattrname(p, &argp->xattr_name, &argp->xattr_name_len)))
+		return 0;
+	argp->xattr_size_max = ntohl(*p++);
+	return xdr_argsize_check(rqstp, p);
+}
+
+static int nfs3svc_encode_getxattrres(struct svc_rqst *rqstp, __be32 *p,
+				      struct nfsd3_getxattrres *resp)
+{
+	p = nfs3svc_encode_post_op_attr(rqstp, p, &resp->fh);
+	if (resp->status == 0)
+		p = xdr_encode_array(p, resp->xattr_val, resp->xattr_val_len);
+	return xdr_ressize_check(rqstp, p);
+}
+
+static int nfs3svc_release_getxattr(struct svc_rqst *rqstp, __be32 *p,
+				    struct nfsd3_getxattrres *resp)
+{
+	fh_put(&resp->fh);
+	kfree(resp->xattr_val);
+	return 1;
+}
+
+/*
+ * SETXATTR and RMXATTR
+ *
+ * RMXATTR is detected with zero buffer len and XATTR_REPLACE.
+ *
+ */
+static __be32 nfsd3_proc_setxattr(struct svc_rqst * rqstp,
+				  struct nfsd3_setxattrargs *argp,
+				  struct nfsd3_setxattrres *resp)
+{
+	__be32 nfserr = nfserrno(-EINVAL);
+	svc_fh *fh;
+	int ret;
+	char *name, *xattr_name = argp->xattr_name;
+	unsigned int name_len = argp->xattr_name_len;
+	unsigned int val_len = argp->xattr_val_len;
+	unsigned int flags = argp->xattr_flags;
+
+	dprintk("nfsd: SETXATTR(3)  %s %.*s %u %#x\n", SVCFH_fmt(&argp->fh),
+		name_len, xattr_name, val_len, flags);
+
+	if (name_len > XATTR_NAME_MAX)
+		RETURN_STATUS(nfserr);
+
+	if (val_len > XATTR_SIZE_MAX)
+		RETURN_STATUS(nfserr);
+
+	if (flags & ~(XATTR_CREATE|XATTR_REPLACE))
+		RETURN_STATUS(nfserr);
+
+	fh = fh_copy(&resp->fh, &argp->fh);
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_SATTR);
+	if (nfserr)
+		RETURN_STATUS(nfserr);
+
+	/* Convert xdr string to real string */
+	name = kmalloc(name_len + 1, GFP_KERNEL);
+	if (name == NULL)
+		RETURN_STATUS(nfserrno(-ENOMEM));
+
+	ret = snprintf(name, name_len + 1, "%.*s", name_len, xattr_name);
+	if (ret > name_len) {
+		nfserr = nfserrno(-EINVAL);
+		goto cleanup;
+	}
+
+	/* Only the user namespace is currently supported by the server */
+	if (strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
+		nfserr = nfserrno(-EINVAL);
+		goto cleanup;
+	}
+
+	if (!val_len) {
+		if (flags & ~XATTR_REPLACE) {
+			nfserr = nfserrno(-EINVAL);
+			goto cleanup;
+		}
+		ret = vfs_removexattr(fh->fh_dentry, name);
+	} else
+		ret = vfs_setxattr(fh->fh_dentry, name,
+				   argp->xattr_val, val_len, flags);
+
+	nfserr = nfserrno(ret);
+
+cleanup:
+	kfree(name);
+	RETURN_STATUS(nfserr);
+}
+
+static int nfs3svc_decode_setxattrargs(struct svc_rqst *rqstp, __be32 *p,
+				       struct nfsd3_setxattrargs *argp)
+{
+	if (!(p = nfs3svc_decode_fh(p, &argp->fh)))
+		return 0;
+	if (!(p = decode_xattrname(p, &argp->xattr_name, &argp->xattr_name_len)))
+		return 0;
+	if (!(p = xdr_decode_string_inplace(p, &argp->xattr_val,
+					&argp->xattr_val_len, XATTR_SIZE_MAX)))
+		return 0;
+	argp->xattr_flags = ntohl(*p++);
+	return xdr_argsize_check(rqstp, p);
+}
+
+static int nfs3svc_encode_setxattrres(struct svc_rqst *rqstp, __be32 *p,
+				      struct nfsd3_setxattrres *resp)
+{
+	p = nfs3svc_encode_post_op_attr(rqstp, p, &resp->fh);
+	return xdr_ressize_check(rqstp, p);
+}
+
+static int nfs3svc_release_setxattr(struct svc_rqst *rqstp, __be32 *p,
+				    struct nfsd3_setxattrres *resp)
+{
+	fh_put(&resp->fh);
+	return 1;
+}
+
+/*
+ * LISTXATTR
+ *
+ * TODO: namespace filtering?
+ */
+static __be32 nfsd3_proc_listxattr(struct svc_rqst * rqstp,
+				   struct nfsd3_listxattrargs *argp,
+				   struct nfsd3_listxattrres *resp)
+{
+	__be32 nfserr = nfserrno(-EINVAL);
+	svc_fh *fh;
+	char *list;
+	int ret;
+	unsigned int list_max = argp->xattr_list_max;
+
+	dprintk("nfsd: LISTXATTR(3)  %s %u\n", SVCFH_fmt(&argp->fh), list_max);
+
+	if (list_max > XATTR_LIST_MAX)
+		RETURN_STATUS(nfserr);
+
+	/* Probes must be handled by the client */
+	if (list_max == 0)
+		RETURN_STATUS(nfserr);
+
+	fh = fh_copy(&resp->fh, &argp->fh);
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_READ);
+	if (nfserr)
+		RETURN_STATUS(nfserr);
+
+	list = kmalloc(list_max, GFP_ATOMIC);
+	if (list == NULL)
+		RETURN_STATUS(nfserrno(-ENOMEM));
+
+	ret = vfs_listxattr(fh->fh_dentry, list, list_max);
+	if (ret <= 0) {
+		if (ret == 0)
+			ret = -ENODATA;
+		RETURN_STATUS(nfserrno(ret));
+	}
+
+	nfserr = 0;
+	resp->xattr_list = list;
+	resp->xattr_list_len = ret;
+
+	RETURN_STATUS(nfserr);
+}
+
+static int nfs3svc_decode_listxattrargs(struct svc_rqst *rqstp, __be32 *p,
+				        struct nfsd3_listxattrargs *argp)
+{
+	if (!(p = nfs3svc_decode_fh(p, &argp->fh)))
+		return 0;
+	argp->xattr_list_max = ntohl(*p++);
+	return xdr_argsize_check(rqstp, p);
+}
+
+static int nfs3svc_encode_listxattrres(struct svc_rqst *rqstp, __be32 *p,
+				       struct nfsd3_listxattrres *resp)
+{
+	p = nfs3svc_encode_post_op_attr(rqstp, p, &resp->fh);
+	if (resp->status == 0)
+		p = xdr_encode_array(p, resp->xattr_list, resp->xattr_list_len);
+	return xdr_ressize_check(rqstp, p);
+}
+
+static int nfs3svc_release_listxattr(struct svc_rqst *rqstp, __be32 *p,
+				     struct nfsd3_listxattrres *resp)
+{
+	fh_put(&resp->fh);
+	kfree(resp->xattr_list);
+	return 1;
+}
+
+#define ST 1		/* status */
+#define AT 21           /* attributes */
+#define pAT (1+AT)      /* post attributes - conditional */
+
+#define nfs3svc_decode_voidargs		NULL
+#define nfs3svc_release_void		NULL
+#define nfsd3_voidres			nfsd3_voidargs
+struct nfsd3_voidargs { int dummy; };
+
+#define PROC(name, argt, rest, relt, cache, respsize)	\
+ { (svc_procfunc) nfsd3_proc_##name,		\
+   (kxdrproc_t) nfs3svc_decode_##argt##args,	\
+   (kxdrproc_t) nfs3svc_encode_##rest##res,	\
+   (kxdrproc_t) nfs3svc_release_##relt,		\
+   sizeof(struct nfsd3_##argt##args),		\
+   sizeof(struct nfsd3_##rest##res),		\
+   0,						\
+   cache,					\
+   respsize,					\
+ }
+
+#define G_RSZ	(ST+pAT+1+(XATTR_SIZE_MAX>>2))
+#define S_RSZ	(ST+pAT)
+#define L_RSZ	(ST+pAT+1+(XATTR_LIST_MAX>>2))
+
+static struct svc_procedure nfsd_xattr_procedures3[] = {
+  PROC(null,       void,       void,       void,       RC_NOCACHE, ST),
+  PROC(getxattr,   getxattr,   getxattr,   getxattr,   RC_NOCACHE, G_RSZ),
+  PROC(setxattr,   setxattr,   setxattr,   setxattr,   RC_NOCACHE, S_RSZ),
+  PROC(listxattr,  listxattr,  listxattr,  listxattr,  RC_NOCACHE, L_RSZ),
+};
+
+struct svc_version	nfsd_xattr_version3 = {
+	.vs_vers	= 3,
+	.vs_nproc	= 4,
+	.vs_proc	= nfsd_xattr_procedures3,
+	.vs_dispatch	= nfsd_dispatch,
+	.vs_xdrsize	= NFS3_SVC_XDRSIZE,
+	.vs_hidden	= 1,
+};
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 508941c..64945b5 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1421,6 +1421,8 @@ static int create_proc_exports_entry(void)
 }
 #endif
 
+extern void __init nfsd_prog_init(void);
+
 static int __init init_nfsd(void)
 {
 	int retval;
@@ -1429,6 +1431,7 @@ static int __init init_nfsd(void)
 	retval = nfs4_state_init(); /* nfs4 locking state */
 	if (retval)
 		return retval;
+	nfsd_prog_init();
 	nfsd_stat_init();	/* Statistics */
 	retval = nfsd_reply_cache_init();
 	if (retval)
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 06b2a26..29096ae 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -15,6 +15,7 @@
 #include <linux/sunrpc/svcsock.h>
 #include <linux/lockd/bind.h>
 #include <linux/nfsacl.h>
+#include <linux/nfs_xattr.h>
 #include <linux/seq_file.h>
 #include "nfsd.h"
 #include "cache.h"
@@ -87,6 +88,27 @@ static struct svc_stat	nfsd_acl_svcstats = {
 };
 #endif /* defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL) */
 
+#ifdef CONFIG_NFSD_V3_XATTR
+static struct svc_stat	nfsd_xattr_svcstats;
+static struct svc_version *	nfsd_xattr_version[] = {
+	[3] = &nfsd_xattr_version3,
+};
+
+#define NFSD_XATTR_MINVERS	3
+#define NFSD_XATTR_NRVERS	ARRAY_SIZE(nfsd_xattr_version)
+static struct svc_version *nfsd_xattr_versions[NFSD_XATTR_NRVERS];
+
+static struct svc_program	nfsd_xattr_program = {
+	.pg_prog		= NFS_XATTR_PROGRAM,
+	.pg_nvers		= NFSD_XATTR_NRVERS,
+	.pg_vers		= nfsd_xattr_versions,
+	.pg_name		= "nfsxattr",
+	.pg_class		= "nfsd",		/* share nfsd auth */
+	.pg_stats		= &nfsd_xattr_svcstats,
+	.pg_authenticate	= &svc_set_client,
+};
+#endif	/* CONFIG_NFSD_V3_XATTR */
+
 static struct svc_version *	nfsd_version[] = {
 	[2] = &nfsd_version2,
 #if defined(CONFIG_NFSD_V3)
@@ -102,9 +124,6 @@ static struct svc_version *	nfsd_version[] = {
 static struct svc_version *nfsd_versions[NFSD_NRVERS];
 
 struct svc_program		nfsd_program = {
-#if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
-	.pg_next		= &nfsd_acl_program,
-#endif
 	.pg_prog		= NFS_PROGRAM,		/* program number */
 	.pg_nvers		= NFSD_NRVERS,		/* nr of entries in nfsd_version */
 	.pg_vers		= nfsd_versions,	/* version table */
@@ -115,6 +134,28 @@ struct svc_program		nfsd_program = {
 
 };
 
+static void __init nfsd_prog_add(struct svc_program *new)
+{
+	struct svc_program *p = &nfsd_program;
+
+	while (p->pg_next)
+		p = p->pg_next;
+
+	p->pg_next = new;
+}
+
+/* Dynamically initialize list of service programs */
+void __init nfsd_prog_init(void)
+{
+#if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
+	nfsd_prog_add(&nfsd_acl_program);
+#endif
+
+#ifdef CONFIG_NFSD_V3_XATTR
+	nfsd_prog_add(&nfsd_xattr_program);
+#endif
+}
+
 u32 nfsd_supported_minorversion;
 
 int nfsd_vers(int vers, enum vers_op change)
@@ -128,6 +169,10 @@ int nfsd_vers(int vers, enum vers_op change)
 		if (vers < NFSD_ACL_NRVERS)
 			nfsd_acl_versions[vers] = nfsd_acl_version[vers];
 #endif
+#ifdef CONFIG_NFSD_V3_XATTR
+		if (vers < NFSD_XATTR_NRVERS)
+			nfsd_xattr_versions[vers] = nfsd_xattr_version[vers];
+#endif
 		break;
 	case NFSD_CLEAR:
 		nfsd_versions[vers] = NULL;
@@ -135,6 +180,10 @@ int nfsd_vers(int vers, enum vers_op change)
 		if (vers < NFSD_ACL_NRVERS)
 			nfsd_acl_versions[vers] = NULL;
 #endif
+#ifdef CONFIG_NFSD_V3_XATTR
+		if (vers < NFSD_XATTR_NRVERS)
+			nfsd_xattr_versions[vers] = NULL;
+#endif
 		break;
 	case NFSD_TEST:
 		return nfsd_versions[vers] != NULL;
@@ -213,6 +262,11 @@ void nfsd_reset_versions(void)
 			nfsd_acl_program.pg_vers[i] =
 				nfsd_acl_version[i];
 #endif
+#ifdef CONFIG_NFSD_V3_XATTR
+		for (i = NFSD_XATTR_MINVERS; i < NFSD_XATTR_NRVERS; i++)
+			nfsd_xattr_program.pg_vers[i] =
+				nfsd_xattr_version[i];
+#endif
 	}
 }
 
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 3c11112..86309d2 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -454,8 +454,9 @@ out_nfserr:
 
 #if defined(CONFIG_NFSD_V2_ACL) || \
     defined(CONFIG_NFSD_V3_ACL) || \
-    defined(CONFIG_NFSD_V4)
-static ssize_t nfsd_getxattr(struct dentry *dentry, char *key, void **buf)
+    defined(CONFIG_NFSD_V4) || \
+    defined(CONFIG_NFSD_V3_XATTR)
+ssize_t nfsd_getxattr(struct dentry *dentry, char *key, void **buf)
 {
 	ssize_t buflen;
 	ssize_t ret;
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 217a62c..4a4d6ec 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -99,4 +99,17 @@ struct posix_acl *nfsd_get_posix_acl(struct svc_fh *, int);
 int nfsd_set_posix_acl(struct svc_fh *, int, struct posix_acl *);
 #endif
 
+#if defined(CONFIG_NFSD_V2_ACL) || \
+    defined(CONFIG_NFSD_V3_ACL) || \
+    defined(CONFIG_NFSD_V4) || \
+    defined(CONFIG_NFSD_V3_XATTR)
+ssize_t nfsd_getxattr(struct dentry *dentry, char *key, void **buf);
+#endif
+
+#ifdef CONFIG_NFSD_V3_XATTR
+extern struct svc_version nfsd_xattr_version3;
+#else
+#define nfsd_xattr_version3 NULL
+#endif
+
 #endif /* LINUX_NFSD_VFS_H */
diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
index 7df980e..e6ccc60 100644
--- a/fs/nfsd/xdr3.h
+++ b/fs/nfsd/xdr3.h
@@ -119,6 +119,27 @@ struct nfsd3_setaclargs {
 	struct posix_acl	*acl_default;
 };
 
+struct nfsd3_getxattrargs {
+	struct svc_fh           fh;
+	char *                  xattr_name;
+	unsigned int            xattr_name_len;
+	unsigned int            xattr_size_max;
+};
+
+struct nfsd3_setxattrargs {
+	struct svc_fh           fh;
+	unsigned int            xattr_flags;
+	char *                  xattr_name;
+	unsigned int            xattr_name_len;
+	char *                  xattr_val;
+	int                     xattr_val_len;
+};
+
+struct nfsd3_listxattrargs {
+	struct svc_fh           fh;
+	unsigned int            xattr_list_max;
+};
+
 struct nfsd3_attrstat {
 	__be32			status;
 	struct svc_fh		fh;
@@ -227,6 +248,25 @@ struct nfsd3_getaclres {
 	struct posix_acl	*acl_default;
 };
 
+struct nfsd3_getxattrres {
+	__be32                  status;
+	struct svc_fh           fh;
+	char *                  xattr_val;
+	unsigned int            xattr_val_len;
+};
+
+struct nfsd3_setxattrres {
+	__be32                  status;
+	struct svc_fh           fh;
+};
+
+struct nfsd3_listxattrres {
+	__be32                  status;
+	struct svc_fh           fh;
+	char *                  xattr_list;
+	unsigned int            xattr_list_len;
+};
+
 /* dummy type for release */
 struct nfsd3_fhandle_pair {
 	__u32			dummy;
@@ -247,6 +287,9 @@ union nfsd3_xdrstore {
 	struct nfsd3_linkargs		linkargs;
 	struct nfsd3_symlinkargs	symlinkargs;
 	struct nfsd3_readdirargs	readdirargs;
+	struct nfsd3_getxattrargs	getxattrargs;
+	struct nfsd3_setxattrargs	setxattrargs;
+	struct nfsd3_listxattrargs	listxattrargs;
 	struct nfsd3_diropres 		diropres;
 	struct nfsd3_accessres		accessres;
 	struct nfsd3_readlinkres	readlinkres;
@@ -260,6 +303,9 @@ union nfsd3_xdrstore {
 	struct nfsd3_pathconfres	pathconfres;
 	struct nfsd3_commitres		commitres;
 	struct nfsd3_getaclres		getaclres;
+	struct nfsd3_getxattrres	getxattrres;
+	struct nfsd3_setxattrres	setxattrres;
+	struct nfsd3_listxattrres	listxattrres;
 };
 
 #define NFS3_SVC_XDRSIZE		sizeof(union nfsd3_xdrstore)
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 5a3085b..8bde5c1 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -371,7 +371,7 @@ struct svc_version {
 	u32			vs_xdrsize;	/* xdrsize needed for this version */
 
 	unsigned int		vs_hidden : 1;	/* Don't register with portmapper.
-						 * Only used for nfsacl so far. */
+						 * Used for nfsacl and nfsxattr. */
 
 	/* Override dispatch function (e.g. when caching replies).
 	 * A return value of 0 means drop the request. 
-- 
1.7.0.1


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

* [PATCH 5/8][RFC v05] XATTR: add new top level nfsd namespace and implement ext3 support
  2010-06-21 11:25 [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR) James Morris
                   ` (3 preceding siblings ...)
  2010-06-21 11:30 ` [PATCH 4/8][RFC v05] NFSv3: add server " James Morris
@ 2010-06-21 11:30 ` James Morris
       [not found] ` <alpine.LRH.2.00.1006212051530.13583-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:30 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

Add a new top-level 'nfsd' xattr namespace for use by the NFSv3 server
when storing xattrs provided by clients using the XATTR
protocol.

Also implement filesystem-level support for the new namespace for the
ext3 filesystem for testing.

Signed-off-by: James Morris <jmorris@namei.org>
---
 fs/ext3/Makefile      |    2 +-
 fs/ext3/xattr.c       |    2 +
 fs/ext3/xattr.h       |    2 +
 fs/ext3/xattr_nfsd.c  |   58 +++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xattr.c            |    6 +++-
 include/linux/xattr.h |    3 ++
 6 files changed, 70 insertions(+), 3 deletions(-)
 create mode 100644 fs/ext3/xattr_nfsd.c

diff --git a/fs/ext3/Makefile b/fs/ext3/Makefile
index e77766a..216ca8a 100644
--- a/fs/ext3/Makefile
+++ b/fs/ext3/Makefile
@@ -7,6 +7,6 @@ obj-$(CONFIG_EXT3_FS) += ext3.o
 ext3-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \
 	   ioctl.o namei.o super.o symlink.o hash.o resize.o ext3_jbd.o
 
-ext3-$(CONFIG_EXT3_FS_XATTR)	 += xattr.o xattr_user.o xattr_trusted.o
+ext3-$(CONFIG_EXT3_FS_XATTR)	 += xattr.o xattr_user.o xattr_trusted.o xattr_nfsd.o
 ext3-$(CONFIG_EXT3_FS_POSIX_ACL) += acl.o
 ext3-$(CONFIG_EXT3_FS_SECURITY)	 += xattr_security.o
diff --git a/fs/ext3/xattr.c b/fs/ext3/xattr.c
index 71fb8d6..e264320 100644
--- a/fs/ext3/xattr.c
+++ b/fs/ext3/xattr.c
@@ -114,6 +114,7 @@ static const struct xattr_handler *ext3_xattr_handler_map[] = {
 #ifdef CONFIG_EXT3_FS_SECURITY
 	[EXT3_XATTR_INDEX_SECURITY]	     = &ext3_xattr_security_handler,
 #endif
+	[EXT3_XATTR_INDEX_NFSD]		     = &ext3_xattr_nfsd_handler,
 };
 
 const struct xattr_handler *ext3_xattr_handlers[] = {
@@ -126,6 +127,7 @@ const struct xattr_handler *ext3_xattr_handlers[] = {
 #ifdef CONFIG_EXT3_FS_SECURITY
 	&ext3_xattr_security_handler,
 #endif
+	&ext3_xattr_nfsd_handler,
 	NULL
 };
 
diff --git a/fs/ext3/xattr.h b/fs/ext3/xattr.h
index 377fe72..92eac0c 100644
--- a/fs/ext3/xattr.h
+++ b/fs/ext3/xattr.h
@@ -21,6 +21,7 @@
 #define EXT3_XATTR_INDEX_TRUSTED		4
 #define	EXT3_XATTR_INDEX_LUSTRE			5
 #define EXT3_XATTR_INDEX_SECURITY	        6
+#define EXT3_XATTR_INDEX_NFSD			7
 
 struct ext3_xattr_header {
 	__le32	h_magic;	/* magic number for identification */
@@ -63,6 +64,7 @@ extern const struct xattr_handler ext3_xattr_trusted_handler;
 extern const struct xattr_handler ext3_xattr_acl_access_handler;
 extern const struct xattr_handler ext3_xattr_acl_default_handler;
 extern const struct xattr_handler ext3_xattr_security_handler;
+extern const struct xattr_handler ext3_xattr_nfsd_handler;
 
 extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
 
diff --git a/fs/ext3/xattr_nfsd.c b/fs/ext3/xattr_nfsd.c
new file mode 100644
index 0000000..7e38c35
--- /dev/null
+++ b/fs/ext3/xattr_nfsd.c
@@ -0,0 +1,58 @@
+/*
+ * linux/fs/ext3/xattr_nfsd.c
+ * Handler for nfsd extended attributes.
+ *
+ * Copyright (C) 2003 by Andreas Gruenbacher, <a.gruenbacher@computer.org>
+ * Copyright (C) 2010 Red Hat, Inc., James Morris <jmorris@redhat.com>
+ */
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/capability.h>
+#include <linux/fs.h>
+#include <linux/ext3_jbd.h>
+#include <linux/ext3_fs.h>
+#include "xattr.h"
+
+static size_t ext3_xattr_nfsd_list(struct dentry *dentry, char *list,
+				   size_t list_size, const char *name,
+				   size_t name_len, int type)
+{
+	const size_t prefix_len = XATTR_NFSD_PREFIX_LEN;
+	const size_t total_len = prefix_len + name_len + 1;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return 0;
+
+	if (list && total_len <= list_size) {
+		memcpy(list, XATTR_NFSD_PREFIX, prefix_len);
+		memcpy(list+prefix_len, name, name_len);
+		list[prefix_len + name_len] = '\0';
+	}
+	return total_len;
+}
+
+static int ext3_xattr_nfsd_get(struct dentry *dentry, const char *name,
+			       void *buffer, size_t size, int type)
+{
+	if (strcmp(name, "") == 0)
+		return -EINVAL;
+	return ext3_xattr_get(dentry->d_inode, EXT3_XATTR_INDEX_NFSD,
+			      name, buffer, size);
+}
+
+static int ext3_xattr_nfsd_set(struct dentry *dentry, const char *name,
+			       const void *value, size_t size, int flags,
+			       int type)
+{
+	if (strcmp(name, "") == 0)
+		return -EINVAL;
+	return ext3_xattr_set(dentry->d_inode, EXT3_XATTR_INDEX_NFSD, name,
+			      value, size, flags);
+}
+
+const struct xattr_handler ext3_xattr_nfsd_handler = {
+	.prefix	= XATTR_NFSD_PREFIX,
+	.list	= ext3_xattr_nfsd_list,
+	.get	= ext3_xattr_nfsd_get,
+	.set	= ext3_xattr_nfsd_set,
+};
diff --git a/fs/xattr.c b/fs/xattr.c
index 01bb813..ec4129b 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -46,9 +46,11 @@ xattr_permission(struct inode *inode, const char *name, int mask)
 		return 0;
 
 	/*
-	 * The trusted.* namespace can only be accessed by a privileged user.
+	 * The trusted.* and nfsd.* namespaces can only be accessed by a
+	 * privileged user.
 	 */
-	if (!strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN))
+	if (!strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN) ||
+	    !strncmp(name, XATTR_NFSD_PREFIX, XATTR_NFSD_PREFIX_LEN))
 		return (capable(CAP_SYS_ADMIN) ? 0 : -EPERM);
 
 	/* In user.* namespace, only regular files and directories can have
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 0cfa1e9..da362a7 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -33,6 +33,9 @@
 #define XATTR_USER_PREFIX "user."
 #define XATTR_USER_PREFIX_LEN (sizeof (XATTR_USER_PREFIX) - 1)
 
+#define XATTR_NFSD_PREFIX "nfsd."
+#define XATTR_NFSD_PREFIX_LEN (sizeof (XATTR_NFSD_PREFIX) - 1)
+
 struct inode;
 struct dentry;
 
-- 
1.7.0.1


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

* [PATCH 6/8][RFC v05] NFSv3: Add server namespace support for XATTR protocol implementation
       [not found] ` <alpine.LRH.2.00.1006212051530.13583-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
@ 2010-06-21 11:31     ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:31 UTC (permalink / raw)
  To: linux-nfs-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	J. Bruce Fields, Neil Brown,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Stephen Smalley

Add support for an NFS server namespace for storing and retrieving
client-supplied xattrs.  All xattrs are now stored on the server under
the 'nfsd' namespace, and are not interpreted by the server at all.
This allows clients to utilize arbitrary xattr namespaces and for the
server to act only as a storage mechanism for the xattrs.

Currently, only the user, trusted and system namespaces are
supported by the client.

A new NFS error code has been implemented, so that -ENODATA
may be propagated cleanly from the server to the client when
accessing xattrs on the server.

For files newly created in the security namespace, a call
is made back into the security subsystem to allow setting
of security xattrs before the file is instantiated on the
client.

NFS ACLs continue to work as expected, because they implement a
hard-coded system xattr namespace which is invoked before the
NFS layer.  e.g. any access to a system.posix_acl_access xattr
is invokes the NFS_ACL protocol.

Signed-off-by: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
---
 fs/nfs/Kconfig              |   10 ----
 fs/nfs/Makefile             |    2 +-
 fs/nfs/internal.h           |    5 ++-
 fs/nfs/nfs2xdr.c            |    3 +
 fs/nfs/nfs3proc.c           |    9 +++
 fs/nfs/nfs3xattr.c          |   28 ++++++++++-
 fs/nfs/nfs3xattr_handlers.c |  121 +++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/nfs3xattr_user.c     |   53 -------------------
 fs/nfs/super.c              |    2 +-
 fs/nfsd/nfs3xattr.c         |   87 ++++++++++++++++++++----------
 fs/nfsd/nfsd.h              |    1 +
 fs/nfsd/nfsproc.c           |    3 +
 fs/nfsd/vfs.h               |   10 ++++
 include/linux/nfs.h         |    1 +
 14 files changed, 239 insertions(+), 96 deletions(-)
 create mode 100644 fs/nfs/nfs3xattr_handlers.c
 delete mode 100644 fs/nfs/nfs3xattr_user.c

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index b74faec..d9d2c53 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -86,16 +86,6 @@ config NFS_V3_XATTR
 
 	  If unsure, say N.
 
-config NFS_V3_XATTR_USER
-	bool "Extended attributes in the user namespace (EXPERIMENTAL)"
-	depends on NFS_V3_XATTR
-	help
-	  This option selects extended attributes in the user.* namespace,
-	  which are arbitrarily named and managed by users, and conveyed
-	  via the XATTR protocol extension for NFS version 3.
-
-	  If unsure, say N.
-
 config NFS_V4
 	bool "NFS client support for NFS version 4 (EXPERIMENTAL)"
 	depends on NFS_FS && EXPERIMENTAL
diff --git a/fs/nfs/Makefile b/fs/nfs/Makefile
index 54018ee..b289d7e 100644
--- a/fs/nfs/Makefile
+++ b/fs/nfs/Makefile
@@ -12,7 +12,7 @@ nfs-$(CONFIG_ROOT_NFS)	+= nfsroot.o
 nfs-$(CONFIG_NFS_V3)	+= nfs3proc.o nfs3xdr.o
 nfs-$(CONFIG_NFS_V3_ACL)	+= nfs3acl.o
 nfs-$(CONFIG_NFS_V3_XATTR_API)	+= nfs3xattr.o
-nfs-$(CONFIG_NFS_V3_XATTR_USER)	+= nfs3xattr_user.o
+nfs-$(CONFIG_NFS_V3_XATTR)	+= nfs3xattr_handlers.o
 nfs-$(CONFIG_NFS_V4)	+= nfs4proc.o nfs4xdr.o nfs4state.o nfs4renewd.o \
 			   delegation.o idmap.o \
 			   callback.o callback_xdr.o callback_proc.o \
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 1a4f777..6aff3a9 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -293,9 +293,12 @@ extern int nfs3_proc_setxattr(struct inode *inode, const char *namespace,
 			      size_t size, int flags);
 extern int nfs3_proc_listxattr(struct inode *inode, char *list,
 			       size_t list_len);
+extern int nfs3_init_xattr(struct inode *dir, struct dentry *dentry);
 
-/* nfs3xattr_user.c */
+/* nfs3xattr_handers.c */
 extern struct xattr_handler nfs3_xattr_user_handler;
+extern struct xattr_handler nfs3_xattr_trusted_handler;
+extern struct xattr_handler nfs3_xattr_security_handler;
 
 /* nfs3acl.c */
 extern struct xattr_handler nfs3_xattr_acl_access_handler;
diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 81cf142..f72880b 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -701,6 +701,9 @@ static struct {
 	{ NFSERR_SERVERFAULT,	-EREMOTEIO	},
 	{ NFSERR_BADTYPE,	-EBADTYPE	},
 	{ NFSERR_JUKEBOX,	-EJUKEBOX	},
+#ifdef CONFIG_NFS_V3_XATTR
+	{ NFSERR_NODATA,	-ENODATA	},
+#endif
 	{ -1,			-EIO		}
 };
 
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index fabb4f2..6d71153 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -386,6 +386,9 @@ nfs3_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 			goto out;
 	}
 	status = nfs3_proc_set_default_acl(dir, dentry->d_inode, mode);
+	if (status != 0)
+		goto out;
+	status = nfs3_init_xattr(dir, dentry);
 out:
 	nfs3_free_createdata(data);
 	dprintk("NFS reply create: %d\n", status);
@@ -565,6 +568,9 @@ nfs3_proc_mkdir(struct inode *dir, struct dentry *dentry, struct iattr *sattr)
 		goto out;
 
 	status = nfs3_proc_set_default_acl(dir, dentry->d_inode, mode);
+	if (status != 0)
+		goto out;
+	status = nfs3_init_xattr(dir, dentry);
 out:
 	nfs3_free_createdata(data);
 	dprintk("NFS reply mkdir: %d\n", status);
@@ -702,6 +708,9 @@ nfs3_proc_mknod(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 	if (status != 0)
 		goto out;
 	status = nfs3_proc_set_default_acl(dir, dentry->d_inode, mode);
+	if (status != 0)
+		goto out;
+	status = nfs3_init_xattr(dir, dentry);
 out:
 	nfs3_free_createdata(data);
 	dprintk("NFS reply mknod: %d\n", status);
diff --git a/fs/nfs/nfs3xattr.c b/fs/nfs/nfs3xattr.c
index f34b9a0..3295b17 100644
--- a/fs/nfs/nfs3xattr.c
+++ b/fs/nfs/nfs3xattr.c
@@ -19,8 +19,10 @@
 #define NFSDBG_FACILITY	NFSDBG_PROC
 
 const struct xattr_handler *nfs3_xattr_handlers[] = {
-#ifdef CONFIG_NFS_V3_XATTR_USER
+#ifdef CONFIG_NFS_V3_XATTR
 	&nfs3_xattr_user_handler,
+	&nfs3_xattr_trusted_handler,
+	&nfs3_xattr_security_handler,
 #endif
 #ifdef CONFIG_NFS_V3_ACL
 	&nfs3_xattr_acl_access_handler,
@@ -261,4 +263,28 @@ cleanup:
 	kfree(res.xattr_list);
 	return status;
 }
+
+/*
+ * Create an xattr for a newly created file, if required by the security
+ * subsystem.
+ */
+int nfs3_init_xattr(struct inode *dir, struct dentry *dentry)
+{
+	int ret;
+	size_t len;
+	void *val;
+	struct inode *inode = dentry->d_inode;
+
+	ret = security_inode_init_security(inode, dir, NULL, &val, &len);
+	if (ret) {
+		if (ret == -EOPNOTSUPP)
+			return 0;
+		return ret;
+	}
+
+	ret = security_inode_setsecctx(dentry, val, len);
+	kfree(val);
+
+	return ret;
+}
 #endif	/* CONFIG_NFS_V3_XATTR */
diff --git a/fs/nfs/nfs3xattr_handlers.c b/fs/nfs/nfs3xattr_handlers.c
new file mode 100644
index 0000000..f7eb561
--- /dev/null
+++ b/fs/nfs/nfs3xattr_handlers.c
@@ -0,0 +1,121 @@
+/*
+ * Client support for the NFS_XATTR protocol.
+ *
+ * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/fs.h>
+#include <linux/nfs.h>
+#include <linux/nfs3.h>
+#include <linux/nfs_fs.h>
+
+#include "internal.h"
+
+#define NFSDBG_FACILITY	NFSDBG_PROC
+
+/*
+ * 'user' namespace
+ */
+
+/*
+ * Call the LISTXATTR procedure only once per syscall, when the user
+ * handler is invoked.
+ */
+static size_t nfs3_user_xattr_list(struct dentry *dentry, char *list,
+				   size_t list_len, const char *name,
+				   size_t name_len, int hflags)
+{
+
+	return nfs3_proc_listxattr(dentry->d_inode, list, list_len);
+}
+
+static size_t nfs3_noop_xattr_list(struct dentry *dentry, char *list,
+				   size_t list_len, const char *name,
+				   size_t name_len, int hflags)
+{
+	return 0;
+}
+
+static int nfs3_user_xattr_get(struct dentry *dentry, const char *name,
+			       void *buffer, size_t size, int hflags)
+{
+	return nfs3_proc_getxattr(dentry->d_inode, XATTR_USER_PREFIX,
+				  name, buffer, size);
+}
+
+static int nfs3_user_xattr_set(struct dentry *dentry, const char *name,
+			       const void *value, size_t size,
+			       int flags, int hflags)
+{
+	return nfs3_proc_setxattr(dentry->d_inode, XATTR_USER_PREFIX,
+				  name, value, size, flags);
+}
+
+/*
+ * 'trusted' namespace
+ */
+struct xattr_handler nfs3_xattr_user_handler = {
+	.prefix = XATTR_USER_PREFIX,
+	.list	= nfs3_user_xattr_list,
+	.get    = nfs3_user_xattr_get,
+	.set    = nfs3_user_xattr_set,
+};
+
+static int nfs3_trusted_xattr_get(struct dentry *dentry, const char *name,
+				  void *buffer, size_t size, int hflags)
+{
+	return nfs3_proc_getxattr(dentry->d_inode, XATTR_TRUSTED_PREFIX,
+				  name, buffer, size);
+}
+
+static int nfs3_trusted_xattr_set(struct dentry *dentry, const char *name,
+				  const void *value, size_t size,
+				  int flags, int hflags)
+{
+	return nfs3_proc_setxattr(dentry->d_inode, XATTR_TRUSTED_PREFIX,
+				  name, value, size, flags);
+}
+
+struct xattr_handler nfs3_xattr_trusted_handler = {
+	.prefix = XATTR_TRUSTED_PREFIX,
+	.list	= nfs3_noop_xattr_list,
+	.get    = nfs3_trusted_xattr_get,
+	.set    = nfs3_trusted_xattr_set,
+};
+
+/*
+ * 'security' namespace
+ */
+static int nfs3_security_xattr_get(struct dentry *dentry, const char *name,
+				   void *buffer, size_t size, int hflags)
+{
+	return nfs3_proc_getxattr(dentry->d_inode, XATTR_SECURITY_PREFIX,
+				  name, buffer, size);
+}
+
+static int nfs3_security_xattr_set(struct dentry *dentry, const char *name,
+				   const void *value, size_t size,
+				   int flags, int hflags)
+{
+	int ret;
+
+	ret = nfs3_proc_setxattr(dentry->d_inode, XATTR_SECURITY_PREFIX,
+				  name, value, size, flags);
+
+	/* FIXME: once size probing is fixed, don't translate to zero */
+	if ((flags & XATTR_REPLACE) && ret == -ENODATA) {
+		printk(KERN_DEBUG "%s: ignoring -ENODATA (fixme)\n", __func__);
+		ret = 0;
+	}
+	return ret;
+}
+
+struct xattr_handler nfs3_xattr_security_handler = {
+	.prefix = XATTR_SECURITY_PREFIX,
+	.list	= nfs3_noop_xattr_list,
+	.get    = nfs3_security_xattr_get,
+	.set    = nfs3_security_xattr_set,
+};
diff --git a/fs/nfs/nfs3xattr_user.c b/fs/nfs/nfs3xattr_user.c
deleted file mode 100644
index c544fcb..0000000
--- a/fs/nfs/nfs3xattr_user.c
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * Support for extended attributes in the the user.* namespace, which are
- * arbitrarily named and managed by users and conveyed via the XATTR
- * protocol extension.
- *
- * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2,
- * as published by the Free Software Foundation.
- */
-#include <linux/fs.h>
-#include <linux/nfs.h>
-#include <linux/nfs3.h>
-#include <linux/nfs_fs.h>
-
-#include "internal.h"
-
-#define NFSDBG_FACILITY	NFSDBG_PROC
-
-/*
- * The generic xattr code will call this for each helper, which is ok for
- * now, because we only support this single namespace.  If support is
- * expanded to more namespaces, we we'll need a custom listxattr operation.
- */
-static size_t nfs3_user_xattr_list(struct dentry *dentry, char *list,
-				   size_t list_len, const char *name,
-				   size_t name_len, int hflags)
-{
-	return nfs3_proc_listxattr(dentry->d_inode, list, list_len);
-}
-
-static int nfs3_user_xattr_get(struct dentry *dentry, const char *name,
-			       void *buffer, size_t size, int hflags)
-{
-	return nfs3_proc_getxattr(dentry->d_inode, XATTR_USER_PREFIX,
-				  name, buffer, size);
-}
-
-static int nfs3_user_xattr_set(struct dentry *dentry, const char *name,
-			       const void *value, size_t size,
-			       int flags, int hflags)
-{
-	return nfs3_proc_setxattr(dentry->d_inode, XATTR_USER_PREFIX,
-				  name, value, size, flags);
-}
-
-struct xattr_handler nfs3_xattr_user_handler = {
-	.prefix = XATTR_USER_PREFIX,
-	.list   = nfs3_user_xattr_list,
-	.get    = nfs3_user_xattr_get,
-	.set    = nfs3_user_xattr_set,
-};
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 62f8db8..b1368c2 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2023,7 +2023,7 @@ static void nfs_fill_super(struct super_block *sb,
 		 */
 		sb->s_flags |= MS_POSIXACL;
 		sb->s_time_gran = 1;
-#if defined(CONFIG_NFS_V3_ACL) || defined (CONFIG_NFS_V3_XATTR_USER)
+#ifdef CONFIG_NFS_V3_XATTR
 		sb->s_xattr = nfs3_xattr_handlers;
 #endif
 	}
diff --git a/fs/nfsd/nfs3xattr.c b/fs/nfsd/nfs3xattr.c
index b5a5faa..d9a7785 100644
--- a/fs/nfsd/nfs3xattr.c
+++ b/fs/nfsd/nfs3xattr.c
@@ -47,11 +47,12 @@ static __be32 nfsd3_proc_getxattr(struct svc_rqst * rqstp,
 	char *name, *xattr_name = argp->xattr_name;
 	unsigned int size_max = argp->xattr_size_max;
 	unsigned int name_len = argp->xattr_name_len;
+	unsigned int name_tot_len = name_len + NFSD_XATTR_PREFIX_LEN;
 
 	dprintk("nfsd: GETXATTR(3)  %s %.*s %u\n", SVCFH_fmt(&argp->fh),
 	        name_len, xattr_name, size_max);
 
-	if (name_len > XATTR_NAME_MAX)
+	if (name_tot_len > XATTR_NAME_MAX)
 		RETURN_STATUS(nfserr);
 
 	if (size_max > XATTR_SIZE_MAX)
@@ -66,27 +67,20 @@ static __be32 nfsd3_proc_getxattr(struct svc_rqst * rqstp,
 	if (nfserr)
 		RETURN_STATUS(nfserr);
 
-	/* Convert xdr string to real string */
-	name = kmalloc(name_len + 1, GFP_KERNEL);
+	/* Convert xattr name to real string and add local prefix */
+	name = kmalloc(name_tot_len + 1, GFP_KERNEL);
 	if (name == NULL)
 		RETURN_STATUS(nfserrno(-ENOMEM));
 
-	ret = snprintf(name, name_len + 1, "%.*s", name_len, xattr_name);
-	if (ret > name_len) {
-		nfserr = nfserrno(-EINVAL);
-		goto cleanup;
-	}
-
-	/* Only the user namespace is currently supported by the server */
-	if (strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
+	ret = snprintf(name, name_tot_len + 1, "%s%.*s",
+		       NFSD_XATTR_PREFIX, name_len, xattr_name);
+	if (ret > name_tot_len) {
 		nfserr = nfserrno(-EINVAL);
 		goto cleanup;
 	}
 
 	ret = nfsd_getxattr(fh->fh_dentry, name, &value);
 	if (ret <= 0) {
-		if (ret == 0)
-			ret = -ENODATA;
 		nfserr = nfserrno(ret);
 		goto cleanup;
 	}
@@ -95,6 +89,8 @@ static __be32 nfsd3_proc_getxattr(struct svc_rqst * rqstp,
 	resp->xattr_val = value;
 	resp->xattr_val_len = ret;
 
+	/* FIXME: verify whether release func is called on error (cf. ACL code) */
+
 cleanup:
 	kfree(name);
 	RETURN_STATUS(nfserr);
@@ -159,11 +155,12 @@ static __be32 nfsd3_proc_setxattr(struct svc_rqst * rqstp,
 	unsigned int name_len = argp->xattr_name_len;
 	unsigned int val_len = argp->xattr_val_len;
 	unsigned int flags = argp->xattr_flags;
+	unsigned int name_tot_len = name_len + NFSD_XATTR_PREFIX_LEN;
 
 	dprintk("nfsd: SETXATTR(3)  %s %.*s %u %#x\n", SVCFH_fmt(&argp->fh),
 		name_len, xattr_name, val_len, flags);
 
-	if (name_len > XATTR_NAME_MAX)
+	if (name_tot_len > XATTR_NAME_MAX)
 		RETURN_STATUS(nfserr);
 
 	if (val_len > XATTR_SIZE_MAX)
@@ -177,19 +174,15 @@ static __be32 nfsd3_proc_setxattr(struct svc_rqst * rqstp,
 	if (nfserr)
 		RETURN_STATUS(nfserr);
 
-	/* Convert xdr string to real string */
-	name = kmalloc(name_len + 1, GFP_KERNEL);
+	/* Convert xattr name to real string and add prefix */
+	name = kmalloc(name_tot_len + 1, GFP_KERNEL);
 	if (name == NULL)
 		RETURN_STATUS(nfserrno(-ENOMEM));
 
-	ret = snprintf(name, name_len + 1, "%.*s", name_len, xattr_name);
-	if (ret > name_len) {
-		nfserr = nfserrno(-EINVAL);
-		goto cleanup;
-	}
+	ret = snprintf(name, name_tot_len + 1, "%s%.*s",
+		       NFSD_XATTR_PREFIX, name_len, xattr_name);
 
-	/* Only the user namespace is currently supported by the server */
-	if (strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
+	if (ret > name_tot_len) {
 		nfserr = nfserrno(-EINVAL);
 		goto cleanup;
 	}
@@ -240,9 +233,30 @@ static int nfs3svc_release_setxattr(struct svc_rqst *rqstp, __be32 *p,
 }
 
 /*
+ * Search the xattr list for those which match the local NFSD prefix. For
+ * each, strip the prefix and copy out the rest of the xattr.
+ */
+static int nfsd3_filter_xattrs(const char *in, char *out, int i_len)
+{
+	int i_idx = 0, o_idx = 0;
+
+	while (i_idx < i_len) {
+		const char *curr = in + i_idx;
+		int c_len = strlen(curr);
+
+		if (!strncmp(NFSD_XATTR_PREFIX, curr, NFSD_XATTR_PREFIX_LEN)) {
+			strcpy(out + o_idx, curr + NFSD_XATTR_PREFIX_LEN);
+			o_idx += c_len - NFSD_XATTR_PREFIX_LEN + 1;
+		}
+
+		i_idx += c_len + 1;
+	}
+
+	return o_idx;
+}
+
+/*
  * LISTXATTR
- *
- * TODO: namespace filtering?
  */
 static __be32 nfsd3_proc_listxattr(struct svc_rqst * rqstp,
 				   struct nfsd3_listxattrargs *argp,
@@ -250,8 +264,8 @@ static __be32 nfsd3_proc_listxattr(struct svc_rqst * rqstp,
 {
 	__be32 nfserr = nfserrno(-EINVAL);
 	svc_fh *fh;
-	char *list;
-	int ret;
+	char *list, *f_list;
+	int ret, f_len;
 	unsigned int list_max = argp->xattr_list_max;
 
 	dprintk("nfsd: LISTXATTR(3)  %s %u\n", SVCFH_fmt(&argp->fh), list_max);
@@ -276,12 +290,27 @@ static __be32 nfsd3_proc_listxattr(struct svc_rqst * rqstp,
 	if (ret <= 0) {
 		if (ret == 0)
 			ret = -ENODATA;
+		kfree(list);
 		RETURN_STATUS(nfserrno(ret));
 	}
 
+	/*
+	 * Filter the xattr list: translate and return only those stored
+	 * with the correct prefix.  The list is a series of nul-terminated
+	 * strings.
+	 */
+	f_list = kmalloc(ret, GFP_ATOMIC);
+	if (f_list == NULL) {
+		kfree(list);
+		RETURN_STATUS(nfserrno(-ENOMEM));
+	}
+
+	f_len = nfsd3_filter_xattrs(list, f_list, ret);
+	kfree(list);
+
 	nfserr = 0;
-	resp->xattr_list = list;
-	resp->xattr_list_len = ret;
+	resp->xattr_list = f_list;
+	resp->xattr_list_len = f_len;
 
 	RETURN_STATUS(nfserr);
 }
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 7237776..b100345 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -166,6 +166,7 @@ void		nfsd_lockd_shutdown(void);
 #define	nfserr_cb_path_down	cpu_to_be32(NFSERR_CB_PATH_DOWN)
 #define	nfserr_locked		cpu_to_be32(NFSERR_LOCKED)
 #define	nfserr_wrongsec		cpu_to_be32(NFSERR_WRONGSEC)
+#define nfserr_nodata			cpu_to_be32(NFSERR_NODATA)
 #define nfserr_badiomode		cpu_to_be32(NFS4ERR_BADIOMODE)
 #define nfserr_badlayout		cpu_to_be32(NFS4ERR_BADLAYOUT)
 #define nfserr_bad_session_digest	cpu_to_be32(NFS4ERR_BAD_SESSION_DIGEST)
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index a047ad6..e364367 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -744,6 +744,9 @@ nfserrno (int errno)
 		{ nfserr_notsupp, -EOPNOTSUPP },
 		{ nfserr_toosmall, -ETOOSMALL },
 		{ nfserr_serverfault, -ESERVERFAULT },
+#ifdef CONFIG_NFSD_V3_XATTR
+		{ nfserr_nodata, -ENODATA },
+#endif
 	};
 	int	i;
 
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 4a4d6ec..54079b2 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -108,6 +108,16 @@ ssize_t nfsd_getxattr(struct dentry *dentry, char *key, void **buf);
 
 #ifdef CONFIG_NFSD_V3_XATTR
 extern struct svc_version nfsd_xattr_version3;
+
+/*
+ * Translation prefix for local storage of remote xattrs.  This is currently
+ * hard-coded, but could be made a configurable per-export option.  We're
+ * using the user namespace, which is widely supported by filesystems, and
+ * allows arbitrary manipulation.
+ */
+#define NFSD_XATTR_PREFIX "nfsd."
+#define NFSD_XATTR_PREFIX_LEN (strlen(NFSD_XATTR_PREFIX))
+
 #else
 #define nfsd_xattr_version3 NULL
 #endif
diff --git a/include/linux/nfs.h b/include/linux/nfs.h
index f387919..05fe996 100644
--- a/include/linux/nfs.h
+++ b/include/linux/nfs.h
@@ -110,6 +110,7 @@
 	NFSERR_FILE_OPEN = 10046,      /*       v4 */
 	NFSERR_ADMIN_REVOKED = 10047,  /*       v4 */
 	NFSERR_CB_PATH_DOWN = 10048,   /*       v4 */
+	NFSERR_NODATA = 10049,	       /*    v3    (XATTR) */
 };
 
 /* NFSv2 file types - beware, these are not the same in NFSv3 */
-- 
1.7.0.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 6/8][RFC v05] NFSv3: Add server namespace support for XATTR protocol implementation
@ 2010-06-21 11:31     ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:31 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

Add support for an NFS server namespace for storing and retrieving
client-supplied xattrs.  All xattrs are now stored on the server under
the 'nfsd' namespace, and are not interpreted by the server at all.
This allows clients to utilize arbitrary xattr namespaces and for the
server to act only as a storage mechanism for the xattrs.

Currently, only the user, trusted and system namespaces are
supported by the client.

A new NFS error code has been implemented, so that -ENODATA
may be propagated cleanly from the server to the client when
accessing xattrs on the server.

For files newly created in the security namespace, a call
is made back into the security subsystem to allow setting
of security xattrs before the file is instantiated on the
client.

NFS ACLs continue to work as expected, because they implement a
hard-coded system xattr namespace which is invoked before the
NFS layer.  e.g. any access to a system.posix_acl_access xattr
is invokes the NFS_ACL protocol.

Signed-off-by: James Morris <jmorris@namei.org>
---
 fs/nfs/Kconfig              |   10 ----
 fs/nfs/Makefile             |    2 +-
 fs/nfs/internal.h           |    5 ++-
 fs/nfs/nfs2xdr.c            |    3 +
 fs/nfs/nfs3proc.c           |    9 +++
 fs/nfs/nfs3xattr.c          |   28 ++++++++++-
 fs/nfs/nfs3xattr_handlers.c |  121 +++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/nfs3xattr_user.c     |   53 -------------------
 fs/nfs/super.c              |    2 +-
 fs/nfsd/nfs3xattr.c         |   87 ++++++++++++++++++++----------
 fs/nfsd/nfsd.h              |    1 +
 fs/nfsd/nfsproc.c           |    3 +
 fs/nfsd/vfs.h               |   10 ++++
 include/linux/nfs.h         |    1 +
 14 files changed, 239 insertions(+), 96 deletions(-)
 create mode 100644 fs/nfs/nfs3xattr_handlers.c
 delete mode 100644 fs/nfs/nfs3xattr_user.c

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index b74faec..d9d2c53 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -86,16 +86,6 @@ config NFS_V3_XATTR
 
 	  If unsure, say N.
 
-config NFS_V3_XATTR_USER
-	bool "Extended attributes in the user namespace (EXPERIMENTAL)"
-	depends on NFS_V3_XATTR
-	help
-	  This option selects extended attributes in the user.* namespace,
-	  which are arbitrarily named and managed by users, and conveyed
-	  via the XATTR protocol extension for NFS version 3.
-
-	  If unsure, say N.
-
 config NFS_V4
 	bool "NFS client support for NFS version 4 (EXPERIMENTAL)"
 	depends on NFS_FS && EXPERIMENTAL
diff --git a/fs/nfs/Makefile b/fs/nfs/Makefile
index 54018ee..b289d7e 100644
--- a/fs/nfs/Makefile
+++ b/fs/nfs/Makefile
@@ -12,7 +12,7 @@ nfs-$(CONFIG_ROOT_NFS)	+= nfsroot.o
 nfs-$(CONFIG_NFS_V3)	+= nfs3proc.o nfs3xdr.o
 nfs-$(CONFIG_NFS_V3_ACL)	+= nfs3acl.o
 nfs-$(CONFIG_NFS_V3_XATTR_API)	+= nfs3xattr.o
-nfs-$(CONFIG_NFS_V3_XATTR_USER)	+= nfs3xattr_user.o
+nfs-$(CONFIG_NFS_V3_XATTR)	+= nfs3xattr_handlers.o
 nfs-$(CONFIG_NFS_V4)	+= nfs4proc.o nfs4xdr.o nfs4state.o nfs4renewd.o \
 			   delegation.o idmap.o \
 			   callback.o callback_xdr.o callback_proc.o \
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 1a4f777..6aff3a9 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -293,9 +293,12 @@ extern int nfs3_proc_setxattr(struct inode *inode, const char *namespace,
 			      size_t size, int flags);
 extern int nfs3_proc_listxattr(struct inode *inode, char *list,
 			       size_t list_len);
+extern int nfs3_init_xattr(struct inode *dir, struct dentry *dentry);
 
-/* nfs3xattr_user.c */
+/* nfs3xattr_handers.c */
 extern struct xattr_handler nfs3_xattr_user_handler;
+extern struct xattr_handler nfs3_xattr_trusted_handler;
+extern struct xattr_handler nfs3_xattr_security_handler;
 
 /* nfs3acl.c */
 extern struct xattr_handler nfs3_xattr_acl_access_handler;
diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 81cf142..f72880b 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -701,6 +701,9 @@ static struct {
 	{ NFSERR_SERVERFAULT,	-EREMOTEIO	},
 	{ NFSERR_BADTYPE,	-EBADTYPE	},
 	{ NFSERR_JUKEBOX,	-EJUKEBOX	},
+#ifdef CONFIG_NFS_V3_XATTR
+	{ NFSERR_NODATA,	-ENODATA	},
+#endif
 	{ -1,			-EIO		}
 };
 
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index fabb4f2..6d71153 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -386,6 +386,9 @@ nfs3_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 			goto out;
 	}
 	status = nfs3_proc_set_default_acl(dir, dentry->d_inode, mode);
+	if (status != 0)
+		goto out;
+	status = nfs3_init_xattr(dir, dentry);
 out:
 	nfs3_free_createdata(data);
 	dprintk("NFS reply create: %d\n", status);
@@ -565,6 +568,9 @@ nfs3_proc_mkdir(struct inode *dir, struct dentry *dentry, struct iattr *sattr)
 		goto out;
 
 	status = nfs3_proc_set_default_acl(dir, dentry->d_inode, mode);
+	if (status != 0)
+		goto out;
+	status = nfs3_init_xattr(dir, dentry);
 out:
 	nfs3_free_createdata(data);
 	dprintk("NFS reply mkdir: %d\n", status);
@@ -702,6 +708,9 @@ nfs3_proc_mknod(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 	if (status != 0)
 		goto out;
 	status = nfs3_proc_set_default_acl(dir, dentry->d_inode, mode);
+	if (status != 0)
+		goto out;
+	status = nfs3_init_xattr(dir, dentry);
 out:
 	nfs3_free_createdata(data);
 	dprintk("NFS reply mknod: %d\n", status);
diff --git a/fs/nfs/nfs3xattr.c b/fs/nfs/nfs3xattr.c
index f34b9a0..3295b17 100644
--- a/fs/nfs/nfs3xattr.c
+++ b/fs/nfs/nfs3xattr.c
@@ -19,8 +19,10 @@
 #define NFSDBG_FACILITY	NFSDBG_PROC
 
 const struct xattr_handler *nfs3_xattr_handlers[] = {
-#ifdef CONFIG_NFS_V3_XATTR_USER
+#ifdef CONFIG_NFS_V3_XATTR
 	&nfs3_xattr_user_handler,
+	&nfs3_xattr_trusted_handler,
+	&nfs3_xattr_security_handler,
 #endif
 #ifdef CONFIG_NFS_V3_ACL
 	&nfs3_xattr_acl_access_handler,
@@ -261,4 +263,28 @@ cleanup:
 	kfree(res.xattr_list);
 	return status;
 }
+
+/*
+ * Create an xattr for a newly created file, if required by the security
+ * subsystem.
+ */
+int nfs3_init_xattr(struct inode *dir, struct dentry *dentry)
+{
+	int ret;
+	size_t len;
+	void *val;
+	struct inode *inode = dentry->d_inode;
+
+	ret = security_inode_init_security(inode, dir, NULL, &val, &len);
+	if (ret) {
+		if (ret == -EOPNOTSUPP)
+			return 0;
+		return ret;
+	}
+
+	ret = security_inode_setsecctx(dentry, val, len);
+	kfree(val);
+
+	return ret;
+}
 #endif	/* CONFIG_NFS_V3_XATTR */
diff --git a/fs/nfs/nfs3xattr_handlers.c b/fs/nfs/nfs3xattr_handlers.c
new file mode 100644
index 0000000..f7eb561
--- /dev/null
+++ b/fs/nfs/nfs3xattr_handlers.c
@@ -0,0 +1,121 @@
+/*
+ * Client support for the NFS_XATTR protocol.
+ *
+ * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ */
+#include <linux/fs.h>
+#include <linux/nfs.h>
+#include <linux/nfs3.h>
+#include <linux/nfs_fs.h>
+
+#include "internal.h"
+
+#define NFSDBG_FACILITY	NFSDBG_PROC
+
+/*
+ * 'user' namespace
+ */
+
+/*
+ * Call the LISTXATTR procedure only once per syscall, when the user
+ * handler is invoked.
+ */
+static size_t nfs3_user_xattr_list(struct dentry *dentry, char *list,
+				   size_t list_len, const char *name,
+				   size_t name_len, int hflags)
+{
+
+	return nfs3_proc_listxattr(dentry->d_inode, list, list_len);
+}
+
+static size_t nfs3_noop_xattr_list(struct dentry *dentry, char *list,
+				   size_t list_len, const char *name,
+				   size_t name_len, int hflags)
+{
+	return 0;
+}
+
+static int nfs3_user_xattr_get(struct dentry *dentry, const char *name,
+			       void *buffer, size_t size, int hflags)
+{
+	return nfs3_proc_getxattr(dentry->d_inode, XATTR_USER_PREFIX,
+				  name, buffer, size);
+}
+
+static int nfs3_user_xattr_set(struct dentry *dentry, const char *name,
+			       const void *value, size_t size,
+			       int flags, int hflags)
+{
+	return nfs3_proc_setxattr(dentry->d_inode, XATTR_USER_PREFIX,
+				  name, value, size, flags);
+}
+
+/*
+ * 'trusted' namespace
+ */
+struct xattr_handler nfs3_xattr_user_handler = {
+	.prefix = XATTR_USER_PREFIX,
+	.list	= nfs3_user_xattr_list,
+	.get    = nfs3_user_xattr_get,
+	.set    = nfs3_user_xattr_set,
+};
+
+static int nfs3_trusted_xattr_get(struct dentry *dentry, const char *name,
+				  void *buffer, size_t size, int hflags)
+{
+	return nfs3_proc_getxattr(dentry->d_inode, XATTR_TRUSTED_PREFIX,
+				  name, buffer, size);
+}
+
+static int nfs3_trusted_xattr_set(struct dentry *dentry, const char *name,
+				  const void *value, size_t size,
+				  int flags, int hflags)
+{
+	return nfs3_proc_setxattr(dentry->d_inode, XATTR_TRUSTED_PREFIX,
+				  name, value, size, flags);
+}
+
+struct xattr_handler nfs3_xattr_trusted_handler = {
+	.prefix = XATTR_TRUSTED_PREFIX,
+	.list	= nfs3_noop_xattr_list,
+	.get    = nfs3_trusted_xattr_get,
+	.set    = nfs3_trusted_xattr_set,
+};
+
+/*
+ * 'security' namespace
+ */
+static int nfs3_security_xattr_get(struct dentry *dentry, const char *name,
+				   void *buffer, size_t size, int hflags)
+{
+	return nfs3_proc_getxattr(dentry->d_inode, XATTR_SECURITY_PREFIX,
+				  name, buffer, size);
+}
+
+static int nfs3_security_xattr_set(struct dentry *dentry, const char *name,
+				   const void *value, size_t size,
+				   int flags, int hflags)
+{
+	int ret;
+
+	ret = nfs3_proc_setxattr(dentry->d_inode, XATTR_SECURITY_PREFIX,
+				  name, value, size, flags);
+
+	/* FIXME: once size probing is fixed, don't translate to zero */
+	if ((flags & XATTR_REPLACE) && ret == -ENODATA) {
+		printk(KERN_DEBUG "%s: ignoring -ENODATA (fixme)\n", __func__);
+		ret = 0;
+	}
+	return ret;
+}
+
+struct xattr_handler nfs3_xattr_security_handler = {
+	.prefix = XATTR_SECURITY_PREFIX,
+	.list	= nfs3_noop_xattr_list,
+	.get    = nfs3_security_xattr_get,
+	.set    = nfs3_security_xattr_set,
+};
diff --git a/fs/nfs/nfs3xattr_user.c b/fs/nfs/nfs3xattr_user.c
deleted file mode 100644
index c544fcb..0000000
--- a/fs/nfs/nfs3xattr_user.c
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * Support for extended attributes in the the user.* namespace, which are
- * arbitrarily named and managed by users and conveyed via the XATTR
- * protocol extension.
- *
- * Copyright (C) 2009 Red Hat, Inc., James Morris <jmorris@redhat.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2,
- * as published by the Free Software Foundation.
- */
-#include <linux/fs.h>
-#include <linux/nfs.h>
-#include <linux/nfs3.h>
-#include <linux/nfs_fs.h>
-
-#include "internal.h"
-
-#define NFSDBG_FACILITY	NFSDBG_PROC
-
-/*
- * The generic xattr code will call this for each helper, which is ok for
- * now, because we only support this single namespace.  If support is
- * expanded to more namespaces, we we'll need a custom listxattr operation.
- */
-static size_t nfs3_user_xattr_list(struct dentry *dentry, char *list,
-				   size_t list_len, const char *name,
-				   size_t name_len, int hflags)
-{
-	return nfs3_proc_listxattr(dentry->d_inode, list, list_len);
-}
-
-static int nfs3_user_xattr_get(struct dentry *dentry, const char *name,
-			       void *buffer, size_t size, int hflags)
-{
-	return nfs3_proc_getxattr(dentry->d_inode, XATTR_USER_PREFIX,
-				  name, buffer, size);
-}
-
-static int nfs3_user_xattr_set(struct dentry *dentry, const char *name,
-			       const void *value, size_t size,
-			       int flags, int hflags)
-{
-	return nfs3_proc_setxattr(dentry->d_inode, XATTR_USER_PREFIX,
-				  name, value, size, flags);
-}
-
-struct xattr_handler nfs3_xattr_user_handler = {
-	.prefix = XATTR_USER_PREFIX,
-	.list   = nfs3_user_xattr_list,
-	.get    = nfs3_user_xattr_get,
-	.set    = nfs3_user_xattr_set,
-};
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 62f8db8..b1368c2 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2023,7 +2023,7 @@ static void nfs_fill_super(struct super_block *sb,
 		 */
 		sb->s_flags |= MS_POSIXACL;
 		sb->s_time_gran = 1;
-#if defined(CONFIG_NFS_V3_ACL) || defined (CONFIG_NFS_V3_XATTR_USER)
+#ifdef CONFIG_NFS_V3_XATTR
 		sb->s_xattr = nfs3_xattr_handlers;
 #endif
 	}
diff --git a/fs/nfsd/nfs3xattr.c b/fs/nfsd/nfs3xattr.c
index b5a5faa..d9a7785 100644
--- a/fs/nfsd/nfs3xattr.c
+++ b/fs/nfsd/nfs3xattr.c
@@ -47,11 +47,12 @@ static __be32 nfsd3_proc_getxattr(struct svc_rqst * rqstp,
 	char *name, *xattr_name = argp->xattr_name;
 	unsigned int size_max = argp->xattr_size_max;
 	unsigned int name_len = argp->xattr_name_len;
+	unsigned int name_tot_len = name_len + NFSD_XATTR_PREFIX_LEN;
 
 	dprintk("nfsd: GETXATTR(3)  %s %.*s %u\n", SVCFH_fmt(&argp->fh),
 	        name_len, xattr_name, size_max);
 
-	if (name_len > XATTR_NAME_MAX)
+	if (name_tot_len > XATTR_NAME_MAX)
 		RETURN_STATUS(nfserr);
 
 	if (size_max > XATTR_SIZE_MAX)
@@ -66,27 +67,20 @@ static __be32 nfsd3_proc_getxattr(struct svc_rqst * rqstp,
 	if (nfserr)
 		RETURN_STATUS(nfserr);
 
-	/* Convert xdr string to real string */
-	name = kmalloc(name_len + 1, GFP_KERNEL);
+	/* Convert xattr name to real string and add local prefix */
+	name = kmalloc(name_tot_len + 1, GFP_KERNEL);
 	if (name == NULL)
 		RETURN_STATUS(nfserrno(-ENOMEM));
 
-	ret = snprintf(name, name_len + 1, "%.*s", name_len, xattr_name);
-	if (ret > name_len) {
-		nfserr = nfserrno(-EINVAL);
-		goto cleanup;
-	}
-
-	/* Only the user namespace is currently supported by the server */
-	if (strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
+	ret = snprintf(name, name_tot_len + 1, "%s%.*s",
+		       NFSD_XATTR_PREFIX, name_len, xattr_name);
+	if (ret > name_tot_len) {
 		nfserr = nfserrno(-EINVAL);
 		goto cleanup;
 	}
 
 	ret = nfsd_getxattr(fh->fh_dentry, name, &value);
 	if (ret <= 0) {
-		if (ret == 0)
-			ret = -ENODATA;
 		nfserr = nfserrno(ret);
 		goto cleanup;
 	}
@@ -95,6 +89,8 @@ static __be32 nfsd3_proc_getxattr(struct svc_rqst * rqstp,
 	resp->xattr_val = value;
 	resp->xattr_val_len = ret;
 
+	/* FIXME: verify whether release func is called on error (cf. ACL code) */
+
 cleanup:
 	kfree(name);
 	RETURN_STATUS(nfserr);
@@ -159,11 +155,12 @@ static __be32 nfsd3_proc_setxattr(struct svc_rqst * rqstp,
 	unsigned int name_len = argp->xattr_name_len;
 	unsigned int val_len = argp->xattr_val_len;
 	unsigned int flags = argp->xattr_flags;
+	unsigned int name_tot_len = name_len + NFSD_XATTR_PREFIX_LEN;
 
 	dprintk("nfsd: SETXATTR(3)  %s %.*s %u %#x\n", SVCFH_fmt(&argp->fh),
 		name_len, xattr_name, val_len, flags);
 
-	if (name_len > XATTR_NAME_MAX)
+	if (name_tot_len > XATTR_NAME_MAX)
 		RETURN_STATUS(nfserr);
 
 	if (val_len > XATTR_SIZE_MAX)
@@ -177,19 +174,15 @@ static __be32 nfsd3_proc_setxattr(struct svc_rqst * rqstp,
 	if (nfserr)
 		RETURN_STATUS(nfserr);
 
-	/* Convert xdr string to real string */
-	name = kmalloc(name_len + 1, GFP_KERNEL);
+	/* Convert xattr name to real string and add prefix */
+	name = kmalloc(name_tot_len + 1, GFP_KERNEL);
 	if (name == NULL)
 		RETURN_STATUS(nfserrno(-ENOMEM));
 
-	ret = snprintf(name, name_len + 1, "%.*s", name_len, xattr_name);
-	if (ret > name_len) {
-		nfserr = nfserrno(-EINVAL);
-		goto cleanup;
-	}
+	ret = snprintf(name, name_tot_len + 1, "%s%.*s",
+		       NFSD_XATTR_PREFIX, name_len, xattr_name);
 
-	/* Only the user namespace is currently supported by the server */
-	if (strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
+	if (ret > name_tot_len) {
 		nfserr = nfserrno(-EINVAL);
 		goto cleanup;
 	}
@@ -240,9 +233,30 @@ static int nfs3svc_release_setxattr(struct svc_rqst *rqstp, __be32 *p,
 }
 
 /*
+ * Search the xattr list for those which match the local NFSD prefix. For
+ * each, strip the prefix and copy out the rest of the xattr.
+ */
+static int nfsd3_filter_xattrs(const char *in, char *out, int i_len)
+{
+	int i_idx = 0, o_idx = 0;
+
+	while (i_idx < i_len) {
+		const char *curr = in + i_idx;
+		int c_len = strlen(curr);
+
+		if (!strncmp(NFSD_XATTR_PREFIX, curr, NFSD_XATTR_PREFIX_LEN)) {
+			strcpy(out + o_idx, curr + NFSD_XATTR_PREFIX_LEN);
+			o_idx += c_len - NFSD_XATTR_PREFIX_LEN + 1;
+		}
+
+		i_idx += c_len + 1;
+	}
+
+	return o_idx;
+}
+
+/*
  * LISTXATTR
- *
- * TODO: namespace filtering?
  */
 static __be32 nfsd3_proc_listxattr(struct svc_rqst * rqstp,
 				   struct nfsd3_listxattrargs *argp,
@@ -250,8 +264,8 @@ static __be32 nfsd3_proc_listxattr(struct svc_rqst * rqstp,
 {
 	__be32 nfserr = nfserrno(-EINVAL);
 	svc_fh *fh;
-	char *list;
-	int ret;
+	char *list, *f_list;
+	int ret, f_len;
 	unsigned int list_max = argp->xattr_list_max;
 
 	dprintk("nfsd: LISTXATTR(3)  %s %u\n", SVCFH_fmt(&argp->fh), list_max);
@@ -276,12 +290,27 @@ static __be32 nfsd3_proc_listxattr(struct svc_rqst * rqstp,
 	if (ret <= 0) {
 		if (ret == 0)
 			ret = -ENODATA;
+		kfree(list);
 		RETURN_STATUS(nfserrno(ret));
 	}
 
+	/*
+	 * Filter the xattr list: translate and return only those stored
+	 * with the correct prefix.  The list is a series of nul-terminated
+	 * strings.
+	 */
+	f_list = kmalloc(ret, GFP_ATOMIC);
+	if (f_list == NULL) {
+		kfree(list);
+		RETURN_STATUS(nfserrno(-ENOMEM));
+	}
+
+	f_len = nfsd3_filter_xattrs(list, f_list, ret);
+	kfree(list);
+
 	nfserr = 0;
-	resp->xattr_list = list;
-	resp->xattr_list_len = ret;
+	resp->xattr_list = f_list;
+	resp->xattr_list_len = f_len;
 
 	RETURN_STATUS(nfserr);
 }
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 7237776..b100345 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -166,6 +166,7 @@ void		nfsd_lockd_shutdown(void);
 #define	nfserr_cb_path_down	cpu_to_be32(NFSERR_CB_PATH_DOWN)
 #define	nfserr_locked		cpu_to_be32(NFSERR_LOCKED)
 #define	nfserr_wrongsec		cpu_to_be32(NFSERR_WRONGSEC)
+#define nfserr_nodata			cpu_to_be32(NFSERR_NODATA)
 #define nfserr_badiomode		cpu_to_be32(NFS4ERR_BADIOMODE)
 #define nfserr_badlayout		cpu_to_be32(NFS4ERR_BADLAYOUT)
 #define nfserr_bad_session_digest	cpu_to_be32(NFS4ERR_BAD_SESSION_DIGEST)
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index a047ad6..e364367 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -744,6 +744,9 @@ nfserrno (int errno)
 		{ nfserr_notsupp, -EOPNOTSUPP },
 		{ nfserr_toosmall, -ETOOSMALL },
 		{ nfserr_serverfault, -ESERVERFAULT },
+#ifdef CONFIG_NFSD_V3_XATTR
+		{ nfserr_nodata, -ENODATA },
+#endif
 	};
 	int	i;
 
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 4a4d6ec..54079b2 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -108,6 +108,16 @@ ssize_t nfsd_getxattr(struct dentry *dentry, char *key, void **buf);
 
 #ifdef CONFIG_NFSD_V3_XATTR
 extern struct svc_version nfsd_xattr_version3;
+
+/*
+ * Translation prefix for local storage of remote xattrs.  This is currently
+ * hard-coded, but could be made a configurable per-export option.  We're
+ * using the user namespace, which is widely supported by filesystems, and
+ * allows arbitrary manipulation.
+ */
+#define NFSD_XATTR_PREFIX "nfsd."
+#define NFSD_XATTR_PREFIX_LEN (strlen(NFSD_XATTR_PREFIX))
+
 #else
 #define nfsd_xattr_version3 NULL
 #endif
diff --git a/include/linux/nfs.h b/include/linux/nfs.h
index f387919..05fe996 100644
--- a/include/linux/nfs.h
+++ b/include/linux/nfs.h
@@ -110,6 +110,7 @@
 	NFSERR_FILE_OPEN = 10046,      /*       v4 */
 	NFSERR_ADMIN_REVOKED = 10047,  /*       v4 */
 	NFSERR_CB_PATH_DOWN = 10048,   /*       v4 */
+	NFSERR_NODATA = 10049,	       /*    v3    (XATTR) */
 };
 
 /* NFSv2 file types - beware, these are not the same in NFSv3 */
-- 
1.7.0.1


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

* [PATCH 7/8][RFC v05] NFSv3: Add xattr and xattrsec mount options to support XATTR protocol
  2010-06-21 11:25 [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR) James Morris
                   ` (5 preceding siblings ...)
       [not found] ` <alpine.LRH.2.00.1006212051530.13583-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
@ 2010-06-21 11:32 ` James Morris
  2010-06-21 11:33 ` [PATCH 8/8][RFC v05] SELinux/NFSv3: Enable xattr labeling behavior for SELinux with the " James Morris
  7 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:32 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

Add xattr and xattrsec mount options to allow administrative support
for the XATTR protocol.

Option use:

   xattr        Enable the XATTR protocol (default if supported)
   noxattr      Disable the XATTR protocol

   xattrsec     Utilize security xattrs via the XATTR protocol
   noxattrsec   Do not utilize security xattrs (default)

The default is to have xattrs enabled by default if they're supported
at the client and the server.

The xattrsec option is used to indicate to the security subsystem on
the client that security labels for the mounted fs should be conveyed
via the XATTR protocol.

Note that the use of these options requires an updated userspace mount
utility (patch available separately).

Signed-off-by: James Morris <jmorris@namei.org>
---
 fs/nfs/nfsroot.c          |   12 ++++++++++++
 fs/nfs/super.c            |   19 +++++++++++++++++++
 include/linux/nfs_fs.h    |    6 ++++++
 include/linux/nfs_mount.h |   13 ++++++-------
 4 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfsroot.c b/fs/nfs/nfsroot.c
index 6bd19d8..cdc9644 100644
--- a/fs/nfs/nfsroot.c
+++ b/fs/nfs/nfsroot.c
@@ -128,6 +128,8 @@ enum {
 	Opt_nointr, Opt_posix, Opt_noposix, Opt_cto, Opt_nocto, Opt_ac, 
 	Opt_noac, Opt_lock, Opt_nolock, Opt_v2, Opt_v3, Opt_udp, Opt_tcp,
 	Opt_acl, Opt_noacl,
+	Opt_xattr, Opt_noxattr,
+	Opt_xattrsec, Opt_noxattrsec,
 	/* Error token */
 	Opt_err
 };
@@ -164,6 +166,10 @@ static const match_table_t tokens __initconst = {
 	{Opt_tcp, "tcp"},
 	{Opt_acl, "acl"},
 	{Opt_noacl, "noacl"},
+	{Opt_xattr, "xattr"},
+	{Opt_noxattr, "noxattr"},
+	{Opt_xattrsec, "xattrsec"},
+	{Opt_noxattrsec, "noxattrsec"},
 	{Opt_err, NULL}
 	
 };
@@ -275,6 +281,12 @@ static int __init root_nfs_parse(char *name, char *buf)
 			case Opt_noacl:
 				nfs_data.flags |= NFS_MOUNT_NOACL;
 				break;
+			case Opt_xattr:
+				nfs_data.flags &= ~NFS_MOUNT_NOXATTR;
+				break;
+			case Opt_noxattr:
+				nfs_data.flags |= NFS_MOUNT_NOXATTR;
+				break;
 			default:
 				printk(KERN_WARNING "Root-NFS: unknown "
 					"option: %s\n", p);
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index b1368c2..f3def8c 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -77,6 +77,8 @@ enum {
 	Opt_v2, Opt_v3, Opt_v4,
 	Opt_udp, Opt_tcp, Opt_rdma,
 	Opt_acl, Opt_noacl,
+	Opt_xattr, Opt_noxattr,
+	Opt_xattrsec, Opt_noxattrsec,
 	Opt_rdirplus, Opt_nordirplus,
 	Opt_sharecache, Opt_nosharecache,
 	Opt_resvport, Opt_noresvport,
@@ -134,6 +136,10 @@ static const match_table_t nfs_mount_option_tokens = {
 	{ Opt_rdma, "rdma" },
 	{ Opt_acl, "acl" },
 	{ Opt_noacl, "noacl" },
+	{ Opt_xattr, "xattr" },
+	{ Opt_noxattr, "noxattr" },
+	{ Opt_xattrsec, "xattrsec" },
+	{ Opt_noxattrsec, "noxattrsec" },
 	{ Opt_rdirplus, "rdirplus" },
 	{ Opt_nordirplus, "nordirplus" },
 	{ Opt_sharecache, "sharecache" },
@@ -755,6 +761,7 @@ static void nfs_umount_begin(struct super_block *sb)
 
 	server = NFS_SB(sb);
 	/* -EIO all pending I/O */
+	/* FIXME: client_xattr ? */
 	rpc = server->client_acl;
 	if (!IS_ERR(rpc))
 		rpc_killall_tasks(rpc);
@@ -1022,6 +1029,18 @@ static int nfs_parse_mount_options(char *raw,
 		case Opt_noacl:
 			mnt->flags |= NFS_MOUNT_NOACL;
 			break;
+		case Opt_xattr:
+			mnt->flags &= ~NFS_MOUNT_NOXATTR;
+			break;
+		case Opt_noxattr:
+			mnt->flags |= NFS_MOUNT_NOXATTR;
+			break;
+		case Opt_xattrsec:
+			mnt->flags |= NFS_MOUNT_XATTRSEC;
+			break;
+		case Opt_noxattrsec:
+			mnt->flags &= ~NFS_MOUNT_XATTRSEC;
+			break;
 		case Opt_rdirplus:
 			mnt->flags &= ~NFS_MOUNT_NORDIRPLUS;
 			break;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 9b0d2de..7bf0aa5 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -53,6 +53,7 @@
 #include <linux/nfs4.h>
 #include <linux/nfs_xdr.h>
 #include <linux/nfs_fs_sb.h>
+#include <linux/nfs_mount.h>
 
 #include <linux/mempool.h>
 
@@ -294,6 +295,11 @@ static inline int nfs_server_capable(struct inode *inode, int cap)
 	return NFS_SERVER(inode)->caps & cap;
 }
 
+static inline bool nfs_server_xattrsec(struct inode *inode)
+{
+	return NFS_SERVER(inode)->flags & NFS_MOUNT_XATTRSEC;
+}
+
 static inline int NFS_USE_READDIRPLUS(struct inode *inode)
 {
 	return test_bit(NFS_INO_ADVISE_RDPLUS, &NFS_I(inode)->flags);
diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
index 04bb4ee..c44355b 100644
--- a/include/linux/nfs_mount.h
+++ b/include/linux/nfs_mount.h
@@ -63,14 +63,13 @@ struct nfs_mount_data {
 #define NFS_MOUNT_SECFLAVOUR	0x2000	/* 5 */
 #define NFS_MOUNT_NORDIRPLUS	0x4000	/* 5 */
 #define NFS_MOUNT_UNSHARED	0x8000	/* 5 */
-#define NFS_MOUNT_FLAGMASK	0xFFFF
+#define NFS_MOUNT_NOXATTR	0x10000	/* 6 */
+#define NFS_MOUNT_XATTRSEC	0x20000	/* 6 */ /* utilize security xattrs */
+#define NFS_MOUNT_FLAGMASK	0x3FFFF
 
 /* The following are for internal use only */
-#define NFS_MOUNT_LOOKUP_CACHE_NONEG	0x10000
-#define NFS_MOUNT_LOOKUP_CACHE_NONE	0x20000
-#define NFS_MOUNT_NORESVPORT		0x40000
-
-/* FIXME: determine semantics and modify flagmask if exposed to userland */
-#define NFS_MOUNT_NOXATTR		0x80000
+#define NFS_MOUNT_LOOKUP_CACHE_NONEG	0x100000
+#define NFS_MOUNT_LOOKUP_CACHE_NONE	0x200000
+#define NFS_MOUNT_NORESVPORT		0x400000
 
 #endif
-- 
1.7.0.1


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

* [PATCH 8/8][RFC v05] SELinux/NFSv3: Enable xattr labeling behavior for SELinux with the XATTR protocol
  2010-06-21 11:25 [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR) James Morris
                   ` (6 preceding siblings ...)
  2010-06-21 11:32 ` [PATCH 7/8][RFC v05] NFSv3: Add xattr and xattrsec mount options to support XATTR protocol James Morris
@ 2010-06-21 11:33 ` James Morris
  7 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 11:33 UTC (permalink / raw)
  To: linux-nfs
  Cc: linux-security-module, Trond Myklebust, J. Bruce Fields,
	Neil Brown, linux-fsdevel, Stephen Smalley

When the new 'xattrsec' mount option is specified, and when the XATTR
protocol is supported at the client and the server, enable 'xattr' labeling
behavor for SELinux, if supported in policy.

This allows per-mount use of NFSv3 xattr labeling:

  $ sudo mount.nfs host:/export /mnt -o "nfsvers=3,xattrsec"

There must also be an appropriate addition to SELinux policy
to support this, e.g.:

  fs_use_xattr nfs gen_context(system_u:object_r:fs_t,s0);

To cater for the case of no XATTR support available or desired,
'genfs' labeling behavior may also still be specified in policy.

If the xattrsec mount option is supplied, but the XATTR protocol
is unavailable, then the SELinux code will not fall back to
genfs behavior and the mount will fail.

With xattr labeling successfully enabled for the mount, SELinux
access control and labeling policy will be enforced at the client
using the XATTR protocol, with security labels being stored
on the server.

Signed-off-by: James Morris <jmorris@namei.org>
---
 security/selinux/hooks.c            |   32 +++++++++++++++++++++++++++-----
 security/selinux/include/security.h |    2 +-
 security/selinux/ss/services.c      |   23 ++++++++++++++++++++---
 3 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 5c9f25b..3eaa13e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -77,6 +77,7 @@
 #include <linux/mutex.h>
 #include <linux/posix-timers.h>
 #include <linux/syslog.h>
+#include <linux/nfs_fs.h>	/* for nfs_server_capable() */
 
 #include "avc.h"
 #include "objsec.h"
@@ -582,7 +583,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 				struct security_mnt_opts *opts)
 {
 	const struct cred *cred = current_cred();
-	int rc = 0, i;
+	int rc = 0, use_xattr = 0, i;
 	struct superblock_security_struct *sbsec = sb->s_security;
 	const char *name = sb->s_type->name;
 	struct inode *inode = sbsec->sb->s_root->d_inode;
@@ -697,11 +698,32 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 	if (strcmp(sb->s_type->name, "proc") == 0)
 		sbsec->flags |= SE_SBPROC;
 
+	/*
+	 * Special handling for NFSv3: if the xattrsec mount option was
+	 * specified, check to see if the XATTR protocol is supported, and
+	 * if so, require SECURITY_FS_USE_XATTR behavior.
+	 */
+	if (!strcmp(sb->s_type->name, "nfs")) {
+		if (nfs_server_xattrsec(inode)) {
+			if (nfs_server_capable(inode, NFS_CAP_XATTR))
+				use_xattr = 1;
+			else {
+				printk(KERN_WARNING "SELinux: xattrsec "
+				       "specified but XATTR unsupported\n");
+				rc = -EOPNOTSUPP;
+				goto out;
+			}
+		}
+	}
+
 	/* Determine the labeling behavior to use for this filesystem type. */
-	rc = security_fs_use((sbsec->flags & SE_SBPROC) ? "proc" : sb->s_type->name, &sbsec->behavior, &sbsec->sid);
+	rc = security_fs_use((sbsec->flags & SE_SBPROC) ?
+			     "proc" : sb->s_type->name, &sbsec->behavior,
+			     &sbsec->sid, use_xattr);
 	if (rc) {
-		printk(KERN_WARNING "%s: security_fs_use(%s) returned %d\n",
-		       __func__, sb->s_type->name, rc);
+		printk(KERN_WARNING "%s: security_fs_use(%s) (use_xattr=%d) "
+		       "returned %d\n", __func__, sb->s_type->name,
+		       use_xattr, rc);
 		goto out;
 	}
 
@@ -1282,7 +1304,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
 						   context, len);
 		}
 		dput(dentry);
-		if (rc < 0) {
+		if (rc <= 0) {
 			if (rc != -ENODATA) {
 				printk(KERN_WARNING "SELinux: %s:  getxattr returned "
 				       "%d for dev=%s ino=%ld\n", __func__,
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 1f7c249..b0dde76 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -163,7 +163,7 @@ int security_get_allow_unknown(void);
 #define SECURITY_FS_USE_MNTPOINT	6 /* use mountpoint labeling */
 
 int security_fs_use(const char *fstype, unsigned int *behavior,
-	u32 *sid);
+	u32 *sid, int use_xattr);
 
 int security_genfs_sid(const char *fstype, char *name, u16 sclass,
 	u32 *sid);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 1de60ce..17cb1b7 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2238,11 +2238,12 @@ out:
  * @fstype: filesystem type
  * @behavior: labeling behavior
  * @sid: SID for filesystem (superblock)
+ * @use_xattr: use xattr labeling behavior for NFS
  */
 int security_fs_use(
 	const char *fstype,
 	unsigned int *behavior,
-	u32 *sid)
+	u32 *sid, int use_xattr)
 {
 	int rc = 0;
 	struct ocontext *c;
@@ -2251,8 +2252,19 @@ int security_fs_use(
 
 	c = policydb.ocontexts[OCON_FSUSE];
 	while (c) {
-		if (strcmp(fstype, c->u.name) == 0)
-			break;
+		/*
+		 * Without significant redesign, we need to add a special
+		 * case for NFS here.  TODO: consider using the new 'native'
+		 * labeling behavior from LNFS.
+		 */
+		if (strcmp(fstype, c->u.name) == 0) {
+			if (strcmp(fstype, "nfs") == 0) {
+				if (use_xattr &&
+				    (c->v.behavior == SECURITY_FS_USE_XATTR))
+					break;
+			} else
+				break;
+		}
 		c = c->next;
 	}
 
@@ -2267,6 +2279,11 @@ int security_fs_use(
 		}
 		*sid = c->sid[0];
 	} else {
+		if (use_xattr) {
+			/* xattr required, must not fall back to genfs */
+			rc = -EINVAL;
+			goto out;
+		}
 		rc = security_genfs_sid(fstype, "/", SECCLASS_DIR, sid);
 		if (rc) {
 			*behavior = SECURITY_FS_USE_NONE;
-- 
1.7.0.1


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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-21 11:29 ` [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol James Morris
@ 2010-06-21 20:02   ` Chuck Lever
       [not found]     ` <4C1FC553.4030904-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
       [not found]   ` <alpine.LRH.2.00.1006212128160.13583-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
  1 sibling, 1 reply; 30+ messages in thread
From: Chuck Lever @ 2010-06-21 20:02 UTC (permalink / raw)
  To: James Morris
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley

On 06/21/10 07:29 AM, James Morris wrote:
> Add client support for the Linux NFSv3 extended attribute
> side protocol (XATTR).
>
> This extends Linux extended attributes over the network to
> servers which support the protocol.
>
> Operation is currently limited to the user.* namespace.
>
> Signed-off-by: James Morris<jmorris@namei.org>
> ---
>   fs/nfs/Kconfig            |   32 ++++++
>   fs/nfs/Makefile           |    1 +
>   fs/nfs/client.c           |   51 +++++++++-
>   fs/nfs/internal.h         |   11 ++
>   fs/nfs/nfs3xattr.c        |  241 ++++++++++++++++++++++++++++++++++++++++++++-
>   fs/nfs/nfs3xattr_user.c   |   53 ++++++++++
>   fs/nfs/nfs3xdr.c          |  187 +++++++++++++++++++++++++++++++++++
>   fs/nfs/super.c            |    2 +-
>   include/linux/nfs_fs_sb.h |    3 +-
>   include/linux/nfs_mount.h |    3 +
>   include/linux/nfs_xattr.h |   21 ++++
>   include/linux/nfs_xdr.h   |   45 +++++++++
>   12 files changed, 646 insertions(+), 4 deletions(-)
>   create mode 100644 fs/nfs/nfs3xattr_user.c
>   create mode 100644 include/linux/nfs_xattr.h

   [ ... snipped ... ]

> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
> index 75dcfc7..08d5ec9 100644
> --- a/fs/nfs/nfs3xdr.c
> +++ b/fs/nfs/nfs3xdr.c
> @@ -87,6 +87,26 @@
>   #define ACL3_setaclres_sz	(1+NFS3_post_op_attr_sz)
>
>   /*
> + * FIXME: currently, the RPC layer will allocate the maximum buffer size
> + * here for each call (which can be ~ 64k).  The Labeled NFS prototype code
> + * uses 4k, although we should not impose limits for NFS which don't exist
> + * in the OS unless absolutely necsssary.  We likely need a dynamic scheme
> + * here, possibly using pages.
> + */

Encoding directly into RPC buffers is usually only for small objects, a 
few hundred bytes at most.  What are the challenges to using the page 
cache for these right now?  We already do that for symlinks and ACLs, right?

Is there a way the server can advertise a size limit for these objects, 
similar to rsize and wsize?

> +#define XATTR3_xattrname_sz	(1+(XATTR_NAME_MAX>>2))
> +#define XATTR3_xattrval_sz	(1+(XATTR_SIZE_MAX>>2))
> +#define XATTR3_xattrlist_sz	(1+(XATTR_LIST_MAX>>2))
> +
> +#define XATTR3_getxattrargs_sz	(NFS3_fh_sz+XATTR3_xattrname_sz+1)
> +#define XATTR3_getxattrres_sz	(1+NFS3_post_op_attr_sz+XATTR3_xattrval_sz)
> +
> +#define XATTR3_setxattrargs_sz	(NFS3_fh_sz+XATTR3_xattrname_sz+XATTR3_xattrval_sz+1)
> +#define XATTR3_setxattrres_sz	(1+NFS3_post_op_attr_sz)
> +
> +#define XATTR3_listxattrargs_sz	(NFS3_fh_sz+1)
> +#define XATTR3_listxattrres_sz	(1+NFS3_post_op_attr_sz+XATTR3_xattrlist_sz)
> +
> +/*
>    * Map file type to S_IFMT bits
>    */
>   static const umode_t nfs_type2fmt[] = {
> @@ -726,6 +746,72 @@ nfs3_xdr_setaclargs(struct rpc_rqst *req, __be32 *p,
>   }
>   #endif  /* CONFIG_NFS_V3_ACL */
>
> +#ifdef CONFIG_NFS_V3_XATTR
> +/*
> + * Special case of xdr_encode_opaque, where the xattr helpers hand us
> + * separate namespace and name buffers, which we encode as a single XDR
> + * string over the wire.  Neither namespace nor name may be empty or null.
> + */
> +static __be32 *xattr_encode_name(__be32 *p, const char *namespace, const char *name)

By convention this function would be called "xdr_encode_xattr_name()".

Are these names going to be UTF-8 strings?

> +{
> +	unsigned int nslen, namelen, totlen, quadlen, padding;
> +
> +	nslen = strlen(namespace);
> +	namelen = strlen(name);
> +	totlen = nslen + namelen;
> +	quadlen = XDR_QUADLEN(totlen);
> +	padding = (quadlen<<  2) - totlen;
> +
> +	*p++ = cpu_to_be32(totlen);
> +	memcpy(p, namespace, nslen);
> +	memcpy((char *)p + nslen, name, namelen);
> +
> +	if (padding != 0)
> +		memset((char *)p + totlen, 0, padding);
> +	p += quadlen;
> +	return p;
> +}
> +
> +/*
> + * Encode GETXATTR arguments
> + */
> +static int nfs3_xdr_getxattrargs(struct rpc_rqst *req, __be32 *p,
> +				 struct nfs3_getxattrargs *args)
> +{
> +	p = xdr_encode_fhandle(p, args->fh);
> +	p = xattr_encode_name(p, args->xattr_namespace, args->xattr_name);
> +	*p++ = htonl(args->xattr_size_max);
> +	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
> +	return 0;
> +}
> +
> +/*
> + * Encode SETXATTR arguments
> + */
> +static int nfs3_xdr_setxattrargs(struct rpc_rqst *req, __be32 *p,
> +				 struct nfs3_setxattrargs *args)
> +{
> +	p = xdr_encode_fhandle(p, args->fh);
> +	p = xattr_encode_name(p, args->xattr_namespace, args->xattr_name);
> +	p = xdr_encode_array(p, args->xattr_val, args->xattr_val_len);
> +	*p++ = htonl(args->xattr_flags);
> +	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
> +	return 0;
> +}
> +
> +/*
> + * Encode LISTXATTR arguments
> + */
> +static int nfs3_xdr_listxattrargs(struct rpc_rqst *req, __be32 *p,
> +				  struct nfs3_listxattrargs *args)
> +{
> +	p = xdr_encode_fhandle(p, args->fh);
> +	*p++ = htonl(args->xattr_list_max);
> +	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
> +	return 0;
> +}
> +#endif	/* CONFIG_NFS_V3_XATTR */
> +
>   /*
>    * NFS XDR decode functions
>    */
> @@ -1135,6 +1221,69 @@ nfs3_xdr_setaclres(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
>   }
>   #endif  /* CONFIG_NFS_V3_ACL */
>
> +#ifdef CONFIG_NFS_V3_XATTR
> +/*
> + * Decode GETXATTR reply
> + *
> + * FIXME: determine appropriate error returns
> + */
> +static int nfs3_xdr_getxattrres(struct rpc_rqst *req, __be32 *p,
> +				struct nfs3_getxattrres *res)
> +{
> +	char *xattr_val;
> +	unsigned int xattr_max_size = res->xattr_val_len;
> +	int status = ntohl(*p++);
> +
> +	if (status != 0)
> +		return nfs_stat_to_errno(status);
> +
> +	p = xdr_decode_post_op_attr(p, res->fattr);
> +	p = xdr_decode_string_inplace(p,&xattr_val,
> +	&res->xattr_val_len,
> +	                              xattr_max_size);
> +	if (p == NULL)
> +		return -EINVAL;
> +	memcpy(res->xattr_val, xattr_val, res->xattr_val_len);
> +	return 0;
> +}
> +
> +/*
> + * Decode SETXATTR reply
> + */
> +static int nfs3_xdr_setxattrres(struct rpc_rqst *req, __be32 *p,
> +				struct nfs3_setxattrres *res)
> +{
> +	int status = ntohl(*p++);
> +
> +	if (status)
> +		return nfs_stat_to_errno(status);
> +	xdr_decode_post_op_attr(p, res->fattr);
> +	return 0;
> +}
> +
> +/*
> + * Decode LISTXATTR reply
> + */
> +static int nfs3_xdr_listxattrres(struct rpc_rqst *req, __be32 *p,
> +				 struct nfs3_listxattrres *res)
> +{
> +	char *xattr_list;
> +	unsigned int size = res->xattr_list_len;
> +	int status = ntohl(*p++);
> +
> +	if (status != 0)
> +		return nfs_stat_to_errno(status);
> +
> +	p = xdr_decode_post_op_attr(p, res->fattr);
> +	p = xdr_decode_string_inplace(p,&xattr_list,
> +	&res->xattr_list_len, size);
> +	if (p == NULL)
> +		return -EINVAL;
> +	memcpy(res->xattr_list, xattr_list, res->xattr_list_len);
> +	return 0;
> +}
> +#endif	/* CONFIG_NFS_V3_XATTR */
> +
>   #define PROC(proc, argtype, restype, timer)				\
>   [NFS3PROC_##proc] = {							\
>   	.p_proc      = NFS3PROC_##proc,					\
> @@ -1206,3 +1355,41 @@ struct rpc_version		nfsacl_version3 = {
>   	.procs			= nfs3_acl_procedures,
>   };
>   #endif  /* CONFIG_NFS_V3_ACL */
> +
> +#ifdef CONFIG_NFS_V3_XATTR
> +static struct rpc_procinfo	nfs3_xattr_procedures[] = {
> +	[XATTRPROC3_GETXATTR] = {
> +		.p_proc		= XATTRPROC3_GETXATTR,
> +		.p_encode	= (kxdrproc_t) nfs3_xdr_getxattrargs,
> +		.p_decode	= (kxdrproc_t) nfs3_xdr_getxattrres,
> +		.p_arglen	= XATTR3_getxattrargs_sz,
> +		.p_replen	= XATTR3_getxattrres_sz,
> +		.p_timer	= 1,
> +		.p_name		= "GETXATTR",
> +	},
> +	[XATTRPROC3_SETXATTR] = {
> +		.p_proc		= XATTRPROC3_SETXATTR,
> +		.p_encode	= (kxdrproc_t) nfs3_xdr_setxattrargs,
> +		.p_decode	= (kxdrproc_t) nfs3_xdr_setxattrres,
> +		.p_arglen	= XATTR3_setxattrargs_sz,
> +		.p_replen	= XATTR3_setxattrres_sz,
> +		.p_timer	= 1,
> +		.p_name		= "SETXATTR",
> +	},
> +	[XATTRPROC3_LISTXATTR] = {
> +		.p_proc		= XATTRPROC3_LISTXATTR,
> +		.p_encode	= (kxdrproc_t) nfs3_xdr_listxattrargs,
> +		.p_decode	= (kxdrproc_t) nfs3_xdr_listxattrres,
> +		.p_arglen	= XATTR3_listxattrargs_sz,
> +		.p_replen	= XATTR3_listxattrres_sz,
> +		.p_timer	= 1,
> +		.p_name		= "LISTXATTR",
> +	},
> +};
> +
> +struct rpc_version		nfs_xattr_version3 = {
> +	.number			= 3,
> +	.nrprocs		= ARRAY_SIZE(nfs3_xattr_procedures),
> +	.procs			= nfs3_xattr_procedures,
> +};
> +#endif	/* CONFIG_NFS_V3_XATTR */

   [ ... snipped ... ]

> diff --git a/include/linux/nfs_xattr.h b/include/linux/nfs_xattr.h
> new file mode 100644
> index 0000000..98fdbed
> --- /dev/null
> +++ b/include/linux/nfs_xattr.h
> @@ -0,0 +1,21 @@
> +/*
> + * Extended attribute protocol for NFSv3 (XATTR)
> + *
> + *
> + * Copyright (C) 2009 Red Hat, Inc., James Morris<jmorris@redhat.com>
> + *
> + */
> +#ifndef __LINUX_NFS_XATTR_H
> +#define __LINUX_NFS_XATTR_H
> +
> +#include<linux/xattr.h>
> +
> +#define NFS_XATTR_PROGRAM	391063	/* TODO: find another value */

RFC 5531, appendix B proscribes the appropriate method for requesting an 
RPC program number assignment.

Even though the NFSACL protocol doesn't have one, I have to agree with 
Deniel that we should have the RPCL definition of the on-the-wire 
protocol first, so we can review it and ensure our user space and kernel 
XDR functions remain consistent and correct over time.  Is there an RFC?

Especially if we expect non-Linux implementations, I would think an 
IANA-assigned program number and a proper RPCL definition would be 
apriori requirements.

> +
> +/* xattr procedure numbers */
> +#define XATTRPROC3_GETXATTR	1
> +#define XATTRPROC3_SETXATTR	2
> +#define XATTRPROC3_LISTXATTR	3
> +#define XATTRPROC3_RMXATTR	4

NIT: Fwiw, I tend to prefer enums for this.

> +
> +#endif  /* __LINUX_NFS_XATTR_H */


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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-21 20:02   ` Chuck Lever
@ 2010-06-21 23:21         ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 23:21 UTC (permalink / raw)
  To: Chuck Lever
  Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	J. Bruce Fields, Neil Brown,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Stephen Smalley

On Mon, 21 Jun 2010, Chuck Lever wrote:

> >   /*
> > + * FIXME: currently, the RPC layer will allocate the maximum buffer size
> > + * here for each call (which can be ~ 64k).  The Labeled NFS prototype code
> > + * uses 4k, although we should not impose limits for NFS which don't exist
> > + * in the OS unless absolutely necsssary.  We likely need a dynamic scheme
> > + * here, possibly using pages.
> > + */
> 
> Encoding directly into RPC buffers is usually only for small objects, a few
> hundred bytes at most.  What are the challenges to using the page cache for
> these right now?  We already do that for symlinks and ACLs, right?

There's nothing stopping it -- the code just needs to be written that way.

> Is there a way the server can advertise a size limit for these objects,
> similar to rsize and wsize?

I guess we could add this, although most xattrs are quite small anyway, 
with upper limits defined by the OS (and then likely the same defined in 
the protocol).

> > +/*
> > + * Special case of xdr_encode_opaque, where the xattr helpers hand us
> > + * separate namespace and name buffers, which we encode as a single XDR
> > + * string over the wire.  Neither namespace nor name may be empty or null.
> > + */
> > +static __be32 *xattr_encode_name(__be32 *p, const char *namespace, const
> > char *name)
> 
> By convention this function would be called "xdr_encode_xattr_name()".

Ok.

> 
> Are these names going to be UTF-8 strings?

I believe so.

> > +#define NFS_XATTR_PROGRAM	391063	/* TODO: find another value */
> 
> RFC 5531, appendix B proscribes the appropriate method for requesting an RPC
> program number assignment.
> 
> Even though the NFSACL protocol doesn't have one, I have to agree with Deniel
> that we should have the RPCL definition of the on-the-wire protocol first, so
> we can review it and ensure our user space and kernel XDR functions remain
> consistent and correct over time.

Indeed, this is something I'm hoping to do soon.

>  Is there an RFC?

No, there have been several implementations, but never an RFC for xattrs.  

I'm using the XFS implementation as a starting point, and following the 
ACL model of implementing a side-protocol.

> Especially if we expect non-Linux implementations, I would think an
> IANA-assigned program number and a proper RPCL definition would be apriori
> requirements.

It's possible the FreeBSD folk may want to interop with this, as they have 
similar name/value xattrs.

(Btw, slides from a presentation I did on this last year at LinuxCon are 
at http://namei.org/presentations/linuxcon09_nfsv3xattrs.pdf)

Thanks for reviewing the code!


-- 
James Morris
<jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
@ 2010-06-21 23:21         ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-21 23:21 UTC (permalink / raw)
  To: Chuck Lever
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley

On Mon, 21 Jun 2010, Chuck Lever wrote:

> >   /*
> > + * FIXME: currently, the RPC layer will allocate the maximum buffer size
> > + * here for each call (which can be ~ 64k).  The Labeled NFS prototype code
> > + * uses 4k, although we should not impose limits for NFS which don't exist
> > + * in the OS unless absolutely necsssary.  We likely need a dynamic scheme
> > + * here, possibly using pages.
> > + */
> 
> Encoding directly into RPC buffers is usually only for small objects, a few
> hundred bytes at most.  What are the challenges to using the page cache for
> these right now?  We already do that for symlinks and ACLs, right?

There's nothing stopping it -- the code just needs to be written that way.

> Is there a way the server can advertise a size limit for these objects,
> similar to rsize and wsize?

I guess we could add this, although most xattrs are quite small anyway, 
with upper limits defined by the OS (and then likely the same defined in 
the protocol).

> > +/*
> > + * Special case of xdr_encode_opaque, where the xattr helpers hand us
> > + * separate namespace and name buffers, which we encode as a single XDR
> > + * string over the wire.  Neither namespace nor name may be empty or null.
> > + */
> > +static __be32 *xattr_encode_name(__be32 *p, const char *namespace, const
> > char *name)
> 
> By convention this function would be called "xdr_encode_xattr_name()".

Ok.

> 
> Are these names going to be UTF-8 strings?

I believe so.

> > +#define NFS_XATTR_PROGRAM	391063	/* TODO: find another value */
> 
> RFC 5531, appendix B proscribes the appropriate method for requesting an RPC
> program number assignment.
> 
> Even though the NFSACL protocol doesn't have one, I have to agree with Deniel
> that we should have the RPCL definition of the on-the-wire protocol first, so
> we can review it and ensure our user space and kernel XDR functions remain
> consistent and correct over time.

Indeed, this is something I'm hoping to do soon.

>  Is there an RFC?

No, there have been several implementations, but never an RFC for xattrs.  

I'm using the XFS implementation as a starting point, and following the 
ACL model of implementing a side-protocol.

> Especially if we expect non-Linux implementations, I would think an
> IANA-assigned program number and a proper RPCL definition would be apriori
> requirements.

It's possible the FreeBSD folk may want to interop with this, as they have 
similar name/value xattrs.

(Btw, slides from a presentation I did on this last year at LinuxCon are 
at http://namei.org/presentations/linuxcon09_nfsv3xattrs.pdf)

Thanks for reviewing the code!


-- 
James Morris
<jmorris@namei.org>

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-21 23:21         ` James Morris
  (?)
@ 2010-06-22 15:32         ` Chuck Lever
       [not found]           ` <4C20D779.5040008-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
  -1 siblings, 1 reply; 30+ messages in thread
From: Chuck Lever @ 2010-06-22 15:32 UTC (permalink / raw)
  To: James Morris
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley

On 06/21/2010 07:21 PM, James Morris wrote:
> On Mon, 21 Jun 2010, Chuck Lever wrote:
>>> +#define NFS_XATTR_PROGRAM	391063	/* TODO: find another value */
>>
>> RFC 5531, appendix B proscribes the appropriate method for requesting an RPC
>> program number assignment.
>>
>> Even though the NFSACL protocol doesn't have one, I have to agree with Deniel
>> that we should have the RPCL definition of the on-the-wire protocol first, so
>> we can review it and ensure our user space and kernel XDR functions remain
>> consistent and correct over time.
>
> Indeed, this is something I'm hoping to do soon.
>
>>   Is there an RFC?
>
> No, there have been several implementations, but never an RFC for xattrs.

> I'm using the XFS implementation as a starting point, and following the
> ACL model of implementing a side-protocol.

>> Especially if we expect non-Linux implementations, I would think an
>> IANA-assigned program number and a proper RPCL definition would be apriori
>> requirements.
>
> It's possible the FreeBSD folk may want to interop with this, as they have
> similar name/value xattrs.

This is why apriori documentation (such as an internet RFC or similar 
that describes the wire protocol) is important.  For an example of a 
side band protocol description, you can look at RFC 1094 or RFC 1813 and 
look at the sections that describe the MOUNT and NLM protocols.

Another place to look is at NFSv4.  Some of the operations that can be 
performed on NFSv4 xattrs are probably nearly what you would want for an 
NFSv3 implementation.  I think it is desirable for anything done for 
NFSv3 to be compatible with NFSv4, as that is already a standard.

> (Btw, slides from a presentation I did on this last year at LinuxCon are
> at http://namei.org/presentations/linuxcon09_nfsv3xattrs.pdf)

NFSACL was added to our NFSv3 implementation because there was a desire 
to interoperate with an existing (albeit nonstandard) implementation. 
Here you are building a new side band protocol from scratch.

Before you go too much farther, you should justify the need to enhance a 
closed standard like NFSv3 rather than expanding on NFSv4 for your 
purposes.  Since NFSv4 already has a lot of what is needed, it seems 
like you'd just need a couple of changes in order to support name/value 
pairs.  You would have a shot at actually making this an internet standard.

I think this community would prefer to advance reasons for users to 
adopt NFSv4, rather than tie users even more to NFSv3.

-- 
Chuck Lever

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-22 15:32         ` Chuck Lever
@ 2010-06-23  0:26               ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-23  0:26 UTC (permalink / raw)
  To: Chuck Lever
  Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	J. Bruce Fields, Neil Brown,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Stephen Smalley,
	Christoph Hellwig, Casey Schaufler

On Tue, 22 Jun 2010, Chuck Lever wrote:

> Another place to look is at NFSv4.  Some of the operations that can be
> performed on NFSv4 xattrs are probably nearly what you would want for an NFSv3
> implementation.  I think it is desirable for anything done for NFSv3 to be
> compatible with NFSv4, as that is already a standard.

Are you referring to named attributes?  I've looked into this, as have 
others, and they appear to be fundamentally incompatible with Linux/BSD 
xattrs.  They're filesystems within files, with different semantics to the 
name/value string model that we use.  I've heard a few developers say 
they've looked at implementing xattrs over NAs, but found it unworkable.

There's the issue of namespacing -- named attributes have no structured 
namespace, whereas, Linux/BSD xattrs use namespaces to denote semantics.

NAs are also intended to be managed at the user level without kernel 
interaction, which conflicts with the semantics of our system namespaces.

In the past, I've seen several discussions on the issue conclude that 
NFSv4 should have distinct Linux/BSD xattr OPs, although I don't know what 
the current thinking is.

> 
> > (Btw, slides from a presentation I did on this last year at LinuxCon are
> > at http://namei.org/presentations/linuxcon09_nfsv3xattrs.pdf)
> 
> NFSACL was added to our NFSv3 implementation because there was a desire to
> interoperate with an existing (albeit nonstandard) implementation. Here you
> are building a new side band protocol from scratch.

Well, it is implemented in IRIX -- I used their GPL code as a starting 
point.  I'm not sure what that was not originally done through the IETF -- 
possibly because there are so many different OS implementations of xattrs 
with different semantics (some of which are slight).

There are also several proprietary NFSv3 xattr implementations out there 
across various OSs -- it would be useful to provide users of these an 
open, maintained option, as well as a pathway to NFSv4.

> Before you go too much farther, you should justify the need to enhance a
> closed standard like NFSv3 rather than expanding on NFSv4 for your purposes.
> Since NFSv4 already has a lot of what is needed, it seems like you'd just need
> a couple of changes in order to support name/value pairs.

As mentioned, I don't think the NFSv3 NAs are a good match to the 
Linux/BSD xattr semantics.

e.g. what happens if a client creates a named attribute called 
"system.posix_acl_access" ?  

My reading of the spec is that a compliant server implementation would 
have to allow this, from any user-level client application, without 
interpretation of the NA itself.

If this is is the case, then it would clash with our ACL mechanism, which 
uses xattrs locally for storage, with system-level interpretation and 
mediation.

>  You would have a shot at actually making this an internet standard.
> 
> I think this community would prefer to advance reasons for users to adopt
> NFSv4, rather than tie users even more to NFSv3.

There are two main drivers for this NFSv3 work.

One is to simply provide xattr support over NFS for Linux (and possibly 
FreeBSD), given that it xattrs are a common filesystem feature and that 
many people are still using NFSv3.

Since I started this work last year, more people do seem to be using NFSv4 
(it seems to be the default for some distros now).  I'd be interested if 
anyone has figures or informed guesses on NFSv4 adoption -- certainly, if 
NFSv3 is going to effectively disappear soon, it affects the rationale for 
this work.

The other driver is the integration of MAC security into general purpose 
OSs.  SELinux and Smack extend Unix inode security metadata with MAC 
labels implemented as xattrs.  There is currently no way to convey these 
labels over NFS, as there is no support for xattrs in the NFS protocol.

There is effort underway for NFSv4 in this area, designed around the full 
security requirements of networked MAC labeling (Labeled NFS).  This 
is a far more comprehensive effort, and obviously does not provide a 
solution for NFSv3 users.

With this NFSv3 xattr code, we are able to provide useful partial support 
for MAC security labeling, for the scenario where the server simply stores 
security labels without interpreting them.  This allows clients to perform 
fine-grained security labeling over NFSv3, and enforce policy locally.

There is considerable demand for NFS security labeling from users.  

They've been deploying MAC security in production for several years now 
but have always had to limit what they do regarding NFS.  This would 
provide a useful partial solution for these users via NFSv3, as an 
intermediate step towards adopting NFSv4 and Labeled NFS.

One currently pressing use-case is where systems are deployed in VMs with 
remote filesystems.  It becomes very problematic to deploy security 
schemes such as sVirt in this manner without fine-grained security 
labeling, hindering attempts to take advantage of its security benefits 
(see the notes on page 4 of the Atsec virt comparison study [1]).

Another more general case is simply being able to use NFS and MAC security 
at the same time.  A significant majority of Fedora users have SELinux 
enabled [2], for example.  If they want to deploy NFS home directories, 
they either lose fine-grained security labeling there or disable security 
entirely.  In other words, lack of NFS support is hindering security 
adoption.

I would not see this as tying people in to NFSv3, rather, providing useful 
functionality until they adopt NFSv4 with all of these features supported.  
The NFSv4 LNFS effort will provide a more complete solution for security 
labeling, and is being worked on in parallel, but it is not clear how long 
the IETF processes for the various aspects of that project will take.

In summary, the benefits are:

- General xattr support for Linux NFSv3 users, and possibly for compatible 
OSs such as FreeBSD

- Storage of security labels over NFSv3 for users deploying Linux MAC 
security schemes (SELinux and Smack)

These objectives can be met relatively simply and quickly with this XATTR 
protocol, which has precedents in the Linux ACL work and in existing NFSv3 
xattr implementations.



[1] http://www.redhat.com/f/pdf/rhev/kvm_security_comparison.pdf
[2] http://smolt.fedoraproject.org/static/stats/stats.html

-- 
James Morris
<jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
@ 2010-06-23  0:26               ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-23  0:26 UTC (permalink / raw)
  To: Chuck Lever
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley,
	Christoph Hellwig, Casey Schaufler

On Tue, 22 Jun 2010, Chuck Lever wrote:

> Another place to look is at NFSv4.  Some of the operations that can be
> performed on NFSv4 xattrs are probably nearly what you would want for an NFSv3
> implementation.  I think it is desirable for anything done for NFSv3 to be
> compatible with NFSv4, as that is already a standard.

Are you referring to named attributes?  I've looked into this, as have 
others, and they appear to be fundamentally incompatible with Linux/BSD 
xattrs.  They're filesystems within files, with different semantics to the 
name/value string model that we use.  I've heard a few developers say 
they've looked at implementing xattrs over NAs, but found it unworkable.

There's the issue of namespacing -- named attributes have no structured 
namespace, whereas, Linux/BSD xattrs use namespaces to denote semantics.

NAs are also intended to be managed at the user level without kernel 
interaction, which conflicts with the semantics of our system namespaces.

In the past, I've seen several discussions on the issue conclude that 
NFSv4 should have distinct Linux/BSD xattr OPs, although I don't know what 
the current thinking is.

> 
> > (Btw, slides from a presentation I did on this last year at LinuxCon are
> > at http://namei.org/presentations/linuxcon09_nfsv3xattrs.pdf)
> 
> NFSACL was added to our NFSv3 implementation because there was a desire to
> interoperate with an existing (albeit nonstandard) implementation. Here you
> are building a new side band protocol from scratch.

Well, it is implemented in IRIX -- I used their GPL code as a starting 
point.  I'm not sure what that was not originally done through the IETF -- 
possibly because there are so many different OS implementations of xattrs 
with different semantics (some of which are slight).

There are also several proprietary NFSv3 xattr implementations out there 
across various OSs -- it would be useful to provide users of these an 
open, maintained option, as well as a pathway to NFSv4.

> Before you go too much farther, you should justify the need to enhance a
> closed standard like NFSv3 rather than expanding on NFSv4 for your purposes.
> Since NFSv4 already has a lot of what is needed, it seems like you'd just need
> a couple of changes in order to support name/value pairs.

As mentioned, I don't think the NFSv3 NAs are a good match to the 
Linux/BSD xattr semantics.

e.g. what happens if a client creates a named attribute called 
"system.posix_acl_access" ?  

My reading of the spec is that a compliant server implementation would 
have to allow this, from any user-level client application, without 
interpretation of the NA itself.

If this is is the case, then it would clash with our ACL mechanism, which 
uses xattrs locally for storage, with system-level interpretation and 
mediation.

>  You would have a shot at actually making this an internet standard.
> 
> I think this community would prefer to advance reasons for users to adopt
> NFSv4, rather than tie users even more to NFSv3.

There are two main drivers for this NFSv3 work.

One is to simply provide xattr support over NFS for Linux (and possibly 
FreeBSD), given that it xattrs are a common filesystem feature and that 
many people are still using NFSv3.

Since I started this work last year, more people do seem to be using NFSv4 
(it seems to be the default for some distros now).  I'd be interested if 
anyone has figures or informed guesses on NFSv4 adoption -- certainly, if 
NFSv3 is going to effectively disappear soon, it affects the rationale for 
this work.

The other driver is the integration of MAC security into general purpose 
OSs.  SELinux and Smack extend Unix inode security metadata with MAC 
labels implemented as xattrs.  There is currently no way to convey these 
labels over NFS, as there is no support for xattrs in the NFS protocol.

There is effort underway for NFSv4 in this area, designed around the full 
security requirements of networked MAC labeling (Labeled NFS).  This 
is a far more comprehensive effort, and obviously does not provide a 
solution for NFSv3 users.

With this NFSv3 xattr code, we are able to provide useful partial support 
for MAC security labeling, for the scenario where the server simply stores 
security labels without interpreting them.  This allows clients to perform 
fine-grained security labeling over NFSv3, and enforce policy locally.

There is considerable demand for NFS security labeling from users.  

They've been deploying MAC security in production for several years now 
but have always had to limit what they do regarding NFS.  This would 
provide a useful partial solution for these users via NFSv3, as an 
intermediate step towards adopting NFSv4 and Labeled NFS.

One currently pressing use-case is where systems are deployed in VMs with 
remote filesystems.  It becomes very problematic to deploy security 
schemes such as sVirt in this manner without fine-grained security 
labeling, hindering attempts to take advantage of its security benefits 
(see the notes on page 4 of the Atsec virt comparison study [1]).

Another more general case is simply being able to use NFS and MAC security 
at the same time.  A significant majority of Fedora users have SELinux 
enabled [2], for example.  If they want to deploy NFS home directories, 
they either lose fine-grained security labeling there or disable security 
entirely.  In other words, lack of NFS support is hindering security 
adoption.

I would not see this as tying people in to NFSv3, rather, providing useful 
functionality until they adopt NFSv4 with all of these features supported.  
The NFSv4 LNFS effort will provide a more complete solution for security 
labeling, and is being worked on in parallel, but it is not clear how long 
the IETF processes for the various aspects of that project will take.

In summary, the benefits are:

- General xattr support for Linux NFSv3 users, and possibly for compatible 
OSs such as FreeBSD

- Storage of security labels over NFSv3 for users deploying Linux MAC 
security schemes (SELinux and Smack)

These objectives can be met relatively simply and quickly with this XATTR 
protocol, which has precedents in the Linux ACL work and in existing NFSv3 
xattr implementations.



[1] http://www.redhat.com/f/pdf/rhev/kvm_security_comparison.pdf
[2] http://smolt.fedoraproject.org/static/stats/stats.html

-- 
James Morris
<jmorris@namei.org>

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-23  0:26               ` James Morris
  (?)
@ 2010-06-23 15:56               ` Casey Schaufler
  -1 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2010-06-23 15:56 UTC (permalink / raw)
  To: James Morris
  Cc: Chuck Lever, linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley,
	Christoph Hellwig, Casey Schaufler

James Morris wrote:
> On Tue, 22 Jun 2010, Chuck Lever wrote:
>
>   
>> Another place to look is at NFSv4.  Some of the operations that can be
>> performed on NFSv4 xattrs are probably nearly what you would want for an NFSv3
>> implementation.  I think it is desirable for anything done for NFSv3 to be
>> compatible with NFSv4, as that is already a standard.
>>     
>
> Are you referring to named attributes?  I've looked into this, as have 
> others, and they appear to be fundamentally incompatible with Linux/BSD 
> xattrs.  They're filesystems within files, with different semantics to the 
> name/value string model that we use.  I've heard a few developers say 
> they've looked at implementing xattrs over NAs, but found it unworkable.
>
> There's the issue of namespacing -- named attributes have no structured 
> namespace, whereas, Linux/BSD xattrs use namespaces to denote semantics.
>
> NAs are also intended to be managed at the user level without kernel 
> interaction, which conflicts with the semantics of our system namespaces.
>
> In the past, I've seen several discussions on the issue conclude that 
> NFSv4 should have distinct Linux/BSD xattr OPs, although I don't know what 
> the current thinking is.
>
>   
>>> (Btw, slides from a presentation I did on this last year at LinuxCon are
>>> at http://namei.org/presentations/linuxcon09_nfsv3xattrs.pdf)
>>>       
>> NFSACL was added to our NFSv3 implementation because there was a desire to
>> interoperate with an existing (albeit nonstandard) implementation. Here you
>> are building a new side band protocol from scratch.
>>     
>
> Well, it is implemented in IRIX -- I used their GPL code as a starting 
> point.  I'm not sure what that was not originally done through the IETF -- 
> possibly because there are so many different OS implementations of xattrs 
> with different semantics (some of which are slight).
>   

The IRIX implementation was not proposed to the IETF. This was
primarily a cost/benefit decision as no one else was using xattrs
the way that IRIX was an interoperability was not a major concern.
The Linux xattr scheme is only distinguishable from the IRIX
usage if you dig deeply, and Linux has much greater emphasis on
transparency than IRIX.

The important bit is that there is an existing worked example of
this model that has already been deployed, the code is available,
and it is compatible with the Linux way of doing xattrs.

> There are also several proprietary NFSv3 xattr implementations out there 
> across various OSs -- it would be useful to provide users of these an 
> open, maintained option, as well as a pathway to NFSv4.
>
>   
>> Before you go too much farther, you should justify the need to enhance a
>> closed standard like NFSv3 rather than expanding on NFSv4 for your purposes.
>> Since NFSv4 already has a lot of what is needed, it seems like you'd just need
>> a couple of changes in order to support name/value pairs.
>>     
>
> As mentioned, I don't think the NFSv3 NAs are a good match to the 
> Linux/BSD xattr semantics.
>
> e.g. what happens if a client creates a named attribute called 
> "system.posix_acl_access" ?  
>
> My reading of the spec is that a compliant server implementation would 
> have to allow this, from any user-level client application, without 
> interpretation of the NA itself.
>
> If this is is the case, then it would clash with our ACL mechanism, which 
> uses xattrs locally for storage, with system-level interpretation and 
> mediation.
>
>   
>>  You would have a shot at actually making this an internet standard.
>>
>> I think this community would prefer to advance reasons for users to adopt
>> NFSv4, rather than tie users even more to NFSv3.
>>     
>
> There are two main drivers for this NFSv3 work.
>
> One is to simply provide xattr support over NFS for Linux (and possibly 
> FreeBSD), given that it xattrs are a common filesystem feature and that 
> many people are still using NFSv3.
>
> Since I started this work last year, more people do seem to be using NFSv4 
> (it seems to be the default for some distros now).  I'd be interested if 
> anyone has figures or informed guesses on NFSv4 adoption -- certainly, if 
> NFSv3 is going to effectively disappear soon, it affects the rationale for 
> this work.
>
> The other driver is the integration of MAC security into general purpose 
> OSs.  SELinux and Smack extend Unix inode security metadata with MAC 
> labels implemented as xattrs.  There is currently no way to convey these 
> labels over NFS, as there is no support for xattrs in the NFS protocol.
>
> There is effort underway for NFSv4 in this area, designed around the full 
> security requirements of networked MAC labeling (Labeled NFS).  This 
> is a far more comprehensive effort, and obviously does not provide a 
> solution for NFSv3 users.
>
> With this NFSv3 xattr code, we are able to provide useful partial support 
> for MAC security labeling, for the scenario where the server simply stores 
> security labels without interpreting them.  This allows clients to perform 
> fine-grained security labeling over NFSv3, and enforce policy locally.
>
> There is considerable demand for NFS security labeling from users.  
>
> They've been deploying MAC security in production for several years now 
> but have always had to limit what they do regarding NFS.  This would 
> provide a useful partial solution for these users via NFSv3, as an 
> intermediate step towards adopting NFSv4 and Labeled NFS.
>
> One currently pressing use-case is where systems are deployed in VMs with 
> remote filesystems.  It becomes very problematic to deploy security 
> schemes such as sVirt in this manner without fine-grained security 
> labeling, hindering attempts to take advantage of its security benefits 
> (see the notes on page 4 of the Atsec virt comparison study [1]).
>
> Another more general case is simply being able to use NFS and MAC security 
> at the same time.  A significant majority of Fedora users have SELinux 
> enabled [2], for example.  If they want to deploy NFS home directories, 
> they either lose fine-grained security labeling there or disable security 
> entirely.  In other words, lack of NFS support is hindering security 
> adoption.
>
> I would not see this as tying people in to NFSv3, rather, providing useful 
> functionality until they adopt NFSv4 with all of these features supported.  
> The NFSv4 LNFS effort will provide a more complete solution for security 
> labeling, and is being worked on in parallel, but it is not clear how long 
> the IETF processes for the various aspects of that project will take.
>
> In summary, the benefits are:
>
> - General xattr support for Linux NFSv3 users, and possibly for compatible 
> OSs such as FreeBSD
>
> - Storage of security labels over NFSv3 for users deploying Linux MAC 
> security schemes (SELinux and Smack)
>
> These objectives can be met relatively simply and quickly with this XATTR 
> protocol, which has precedents in the Linux ACL work and in existing NFSv3 
> xattr implementations.
>
>
>
> [1] http://www.redhat.com/f/pdf/rhev/kvm_security_comparison.pdf
> [2] http://smolt.fedoraproject.org/static/stats/stats.html
>
>   


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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-23  0:26               ` James Morris
  (?)
  (?)
@ 2010-06-23 17:29               ` Chuck Lever
       [not found]                 ` <4C224463.90306-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
  -1 siblings, 1 reply; 30+ messages in thread
From: Chuck Lever @ 2010-06-23 17:29 UTC (permalink / raw)
  To: James Morris
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley,
	Christoph Hellwig, Casey Schaufler

Hi James-

Good points, see below.

On 06/22/10 08:26 PM, James Morris wrote:
> On Tue, 22 Jun 2010, Chuck Lever wrote:
>
>> Another place to look is at NFSv4.  Some of the operations that can be
>> performed on NFSv4 xattrs are probably nearly what you would want for an NFSv3
>> implementation.  I think it is desirable for anything done for NFSv3 to be
>> compatible with NFSv4, as that is already a standard.
>
> Are you referring to named attributes?  I've looked into this, as have
> others, and they appear to be fundamentally incompatible with Linux/BSD
> xattrs.  They're filesystems within files, with different semantics to the
> name/value string model that we use.  I've heard a few developers say
> they've looked at implementing xattrs over NAs, but found it unworkable.
>
> There's the issue of namespacing -- named attributes have no structured
> namespace, whereas, Linux/BSD xattrs use namespaces to denote semantics.
>
> NAs are also intended to be managed at the user level without kernel
> interaction, which conflicts with the semantics of our system namespaces.
>
> In the past, I've seen several discussions on the issue conclude that
> NFSv4 should have distinct Linux/BSD xattr OPs, although I don't know what
> the current thinking is.

My thought was to extend NFSv4 to provide native xattr support alongside 
named attribute support.  If NFSv4 is getting security label support 
then that's a moot point, unless there are other use cases for exposing 
xattrs to NFS clients.

Overall, I think everyone is better off in the long run if we get the 
needed features into NFSv4.

>>> (Btw, slides from a presentation I did on this last year at LinuxCon are
>>> at http://namei.org/presentations/linuxcon09_nfsv3xattrs.pdf)
>>
>> NFSACL was added to our NFSv3 implementation because there was a desire to
>> interoperate with an existing (albeit nonstandard) implementation. Here you
>> are building a new side band protocol from scratch.
>
> Well, it is implemented in IRIX -- I used their GPL code as a starting
> point.  I'm not sure what that was not originally done through the IETF --
> possibly because there are so many different OS implementations of xattrs
> with different semantics (some of which are slight).
>
> There are also several proprietary NFSv3 xattr implementations out there
> across various OSs -- it would be useful to provide users of these an
> open, maintained option, as well as a pathway to NFSv4.

Given that there are disparate proprietary NFSv3 xattr implementations 
how do you plan to ensure interoperability without a protocol 
specification or at least an RPCL definition?  Is interoperability even 
practical at this point?

You say "it would be useful to provide users of these an open, 
maintained option" but so far, security labels appear to be the only 
specific use case you've mentioned.  What other interesting applications 
of xattrs would be useful to expose via NFS?

>> Before you go too much farther, you should justify the need to enhance a
>> closed standard like NFSv3 rather than expanding on NFSv4 for your purposes.
>> Since NFSv4 already has a lot of what is needed, it seems like you'd just need
>> a couple of changes in order to support name/value pairs.
>
> As mentioned, I don't think the NFSv3 NAs are a good match to the
> Linux/BSD xattr semantics.
>
> e.g. what happens if a client creates a named attribute called
> "system.posix_acl_access" ?
>
> My reading of the spec is that a compliant server implementation would
> have to allow this, from any user-level client application, without
> interpretation of the NA itself.
>
> If this is is the case, then it would clash with our ACL mechanism, which
> uses xattrs locally for storage, with system-level interpretation and
> mediation.

>>   You would have a shot at actually making this an internet standard.
>>
>> I think this community would prefer to advance reasons for users to adopt
>> NFSv4, rather than tie users even more to NFSv3.
>
> There are two main drivers for this NFSv3 work.
>
> One is to simply provide xattr support over NFS for Linux (and possibly
> FreeBSD), given that it xattrs are a common filesystem feature and that
> many people are still using NFSv3.

> Since I started this work last year, more people do seem to be using NFSv4
> (it seems to be the default for some distros now).  I'd be interested if
> anyone has figures or informed guesses on NFSv4 adoption -- certainly, if
> NFSv3 is going to effectively disappear soon, it affects the rationale for
> this work.

Assume that NFSv4 will be the dominant deployed NFS version at some 
point in the future.  We can't really say when that will be.  We also 
know there are still active NFSv2 deployments, which suggests NFSv3 
won't be going away soon.

But the salient point, I think, is that if NFSv4 supports security 
labels, then those who require this feature will upgrade, independent of 
the broader adoption rate of NFSv4.

> The other driver is the integration of MAC security into general purpose
> OSs.  SELinux and Smack extend Unix inode security metadata with MAC
> labels implemented as xattrs.  There is currently no way to convey these
> labels over NFS, as there is no support for xattrs in the NFS protocol.
>
> There is effort underway for NFSv4 in this area, designed around the full
> security requirements of networked MAC labeling (Labeled NFS).  This
> is a far more comprehensive effort, and obviously does not provide a
> solution for NFSv3 users.

Having a design document would allow wider public assessment of whether 
there will be transition issues for users between your proposed NFSv3 
xattr protocol and the labeled NFS work.  Do you have a published 
analysis of how NFSv3 xattr users would transition to NFSv4 labeled NFS?

> With this NFSv3 xattr code, we are able to provide useful partial support
> for MAC security labeling, for the scenario where the server simply stores
> security labels without interpreting them.  This allows clients to perform
> fine-grained security labeling over NFSv3, and enforce policy locally.

Given that NFSv3 deployments overwhelmingly use AUTH_SYS authentication 
and do not use wire encryption, how do you guarantee that xattrs aren't 
modified during transit between server and client?

Again, having an RFC-like document that discusses NFS on-the-wire 
authentication and integrity requirements for xattrs and security labels 
would be useful to hammer out and verify implementation requirements.

> There is considerable demand for NFS security labeling from users.
>
> They've been deploying MAC security in production for several years now
> but have always had to limit what they do regarding NFS.  This would
> provide a useful partial solution for these users via NFSv3, as an
> intermediate step towards adopting NFSv4 and Labeled NFS.

To play devil's advocate for a moment, what's the practical difference 
between not having support for security labels now, and having it 
sometime in the near future with NFSv4?

> One currently pressing use-case is where systems are deployed in VMs with
> remote filesystems.  It becomes very problematic to deploy security
> schemes such as sVirt in this manner without fine-grained security
> labeling, hindering attempts to take advantage of its security benefits
> (see the notes on page 4 of the Atsec virt comparison study [1]).
>
> Another more general case is simply being able to use NFS and MAC security
> at the same time.  A significant majority of Fedora users have SELinux
> enabled [2], for example.  If they want to deploy NFS home directories,
> they either lose fine-grained security labeling there or disable security
> entirely.  In other words, lack of NFS support is hindering security
> adoption.

Understood.

> I would not see this as tying people in to NFSv3, rather, providing useful
> functionality until they adopt NFSv4 with all of these features supported.
> The NFSv4 LNFS effort will provide a more complete solution for security
> labeling, and is being worked on in parallel, but it is not clear how long
> the IETF processes for the various aspects of that project will take.

This gives NFSv3 users one more excuse not to upgrade (or simply to 
postpone an upgrade even further).  We want to encourage people to adopt 
NFSv4.  Security label support would be a compelling reason to upgrade.

I don't think you need to have a lot of concern about the IETF process. 
  Once there is rough consensus, you can proceed with a Linux prototype.

> In summary, the benefits are:
>
> - General xattr support for Linux NFSv3 users, and possibly for compatible
> OSs such as FreeBSD
>
> - Storage of security labels over NFSv3 for users deploying Linux MAC
> security schemes (SELinux and Smack)
>
> These objectives can be met relatively simply and quickly with this XATTR
> protocol, which has precedents in the Linux ACL work and in existing NFSv3
> xattr implementations.

To be clear, I'm not objecting to this work, but rather to understand 
the big picture.  Naturally RH can include this feature in their 
products, and the NFS maintainers can decide they'll include NFSv3 xattr 
support anyway, regardless of my concerns.  And if the only purpose of 
your post was for review before including it in RH products, then you 
might ignore the following concerns.

However, since we're talking about more than a few lines of code and a 
rather limited lifetime of usefulness, I think that before upstream 
considers accepting this new side band protocol, there are larger issues 
than whether it works today between two Linux systems, and avoids crashing.

   o  Do we understand the use cases for xattr over NFSv3?
   o  Do we understand the interoperability requirements?
   o  Will NFSv3's known security issues prevent this from being 
ultimately useful?
   o  Is the work to support this feature upstream worth the year or two 
it will be useful before NFSv4 labels are available?
   o  Are there other near term solutions?
   o  How do we ensure this implementation continues to meet its 
specified requirements over time (ie is there a protocol spec? how will 
implementations be tested)?

I think you should consider some next steps:

   o  Talk with other O/S and NFS server implementers who have xattrs to 
see if there are real interoperability concerns
   o  Produce an RPCL and a design document
   o  Find other compelling use cases for exposing xattrs to NFS clients
   o  Tie into the labeled NFS work to see whether a Linux prototype on 
NFSv4 at this time would be useful
   o  Consider proposing xattr support in NFSv4 to the NFSv4 working group

And if you want to consider some specific mechanical items:

   o  Use the page cache instead of the RPC buffer for storing xattrs 
during RPC operations
   o  Get an RPC program number from IANA

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-23  0:26               ` James Morris
@ 2010-06-23 18:35                   ` J. Bruce Fields
  -1 siblings, 0 replies; 30+ messages in thread
From: J. Bruce Fields @ 2010-06-23 18:35 UTC (permalink / raw)
  To: James Morris
  Cc: Chuck Lever, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	Neil Brown, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	Stephen Smalley, Christoph Hellwig, Casey Schaufler

On Wed, Jun 23, 2010 at 10:26:18AM +1000, James Morris wrote:
> On Tue, 22 Jun 2010, Chuck Lever wrote:
> 
> > Another place to look is at NFSv4.  Some of the operations that can be
> > performed on NFSv4 xattrs are probably nearly what you would want for an NFSv3
> > implementation.  I think it is desirable for anything done for NFSv3 to be
> > compatible with NFSv4, as that is already a standard.
> 
> Are you referring to named attributes?  I've looked into this, as have 
> others, and they appear to be fundamentally incompatible with Linux/BSD 
> xattrs.  They're filesystems within files, with different semantics to the 
> name/value string model that we use.  I've heard a few developers say 
> they've looked at implementing xattrs over NAs, but found it unworkable.

Yes, if we were going to add xattr's to NFSv4, we'd want to define
entirely new operations for it rather than relying on the named
attribute mechanism.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
@ 2010-06-23 18:35                   ` J. Bruce Fields
  0 siblings, 0 replies; 30+ messages in thread
From: J. Bruce Fields @ 2010-06-23 18:35 UTC (permalink / raw)
  To: James Morris
  Cc: Chuck Lever, linux-nfs, linux-security-module, Trond Myklebust,
	Neil Brown, linux-fsdevel, Stephen Smalley, Christoph Hellwig,
	Casey Schaufler

On Wed, Jun 23, 2010 at 10:26:18AM +1000, James Morris wrote:
> On Tue, 22 Jun 2010, Chuck Lever wrote:
> 
> > Another place to look is at NFSv4.  Some of the operations that can be
> > performed on NFSv4 xattrs are probably nearly what you would want for an NFSv3
> > implementation.  I think it is desirable for anything done for NFSv3 to be
> > compatible with NFSv4, as that is already a standard.
> 
> Are you referring to named attributes?  I've looked into this, as have 
> others, and they appear to be fundamentally incompatible with Linux/BSD 
> xattrs.  They're filesystems within files, with different semantics to the 
> name/value string model that we use.  I've heard a few developers say 
> they've looked at implementing xattrs over NAs, but found it unworkable.

Yes, if we were going to add xattr's to NFSv4, we'd want to define
entirely new operations for it rather than relying on the named
attribute mechanism.

--b.

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-23 18:35                   ` J. Bruce Fields
  (?)
@ 2010-06-23 18:58                   ` Trond Myklebust
  2010-06-23 22:51                     ` James Morris
  -1 siblings, 1 reply; 30+ messages in thread
From: Trond Myklebust @ 2010-06-23 18:58 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: James Morris, Chuck Lever, linux-nfs, linux-security-module,
	Neil Brown, linux-fsdevel, Stephen Smalley, Christoph Hellwig,
	Casey Schaufler

On Wed, 2010-06-23 at 14:35 -0400, J. Bruce Fields wrote:
> On Wed, Jun 23, 2010 at 10:26:18AM +1000, James Morris wrote:
> > On Tue, 22 Jun 2010, Chuck Lever wrote:
> > 
> > > Another place to look is at NFSv4.  Some of the operations that can be
> > > performed on NFSv4 xattrs are probably nearly what you would want for an NFSv3
> > > implementation.  I think it is desirable for anything done for NFSv3 to be
> > > compatible with NFSv4, as that is already a standard.
> > 
> > Are you referring to named attributes?  I've looked into this, as have 
> > others, and they appear to be fundamentally incompatible with Linux/BSD 
> > xattrs.  They're filesystems within files, with different semantics to the 
> > name/value string model that we use.  I've heard a few developers say 
> > they've looked at implementing xattrs over NAs, but found it unworkable.
> 
> Yes, if we were going to add xattr's to NFSv4, we'd want to define
> entirely new operations for it rather than relying on the named
> attribute mechanism.

Agreed, however there is already work underway in the IETF on the
'labeled NFS' protocol for supporting security labels in NFSv4.

See http://linux-nfs.org/pipermail/labeled-nfs/

Is there really much interest in adding NFSv4 support for xattrs beyond
the security label use case?

Cheers
  Trond

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-23 17:29               ` Chuck Lever
@ 2010-06-23 21:39                     ` Casey Schaufler
  0 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2010-06-23 21:39 UTC (permalink / raw)
  To: Chuck Lever
  Cc: James Morris, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	J. Bruce Fields, Neil Brown,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Stephen Smalley,
	Christoph Hellwig

Chuck Lever wrote:
> Hi James-
>
> Good points, see below.
>
> On 06/22/10 08:26 PM, James Morris wrote:
>> On Tue, 22 Jun 2010, Chuck Lever wrote:
>>
>>> Another place to look is at NFSv4.  Some of the operations that can be
>>> performed on NFSv4 xattrs are probably nearly what you would want
>>> for an NFSv3
>>> implementation.  I think it is desirable for anything done for NFSv3
>>> to be
>>> compatible with NFSv4, as that is already a standard.
>>
>> Are you referring to named attributes?  I've looked into this, as have
>> others, and they appear to be fundamentally incompatible with Linux/BSD
>> xattrs.  They're filesystems within files, with different semantics
>> to the
>> name/value string model that we use.  I've heard a few developers say
>> they've looked at implementing xattrs over NAs, but found it unworkable.
>>
>> There's the issue of namespacing -- named attributes have no structured
>> namespace, whereas, Linux/BSD xattrs use namespaces to denote semantics.
>>
>> NAs are also intended to be managed at the user level without kernel
>> interaction, which conflicts with the semantics of our system
>> namespaces.
>>
>> In the past, I've seen several discussions on the issue conclude that
>> NFSv4 should have distinct Linux/BSD xattr OPs, although I don't know
>> what
>> the current thinking is.
>
> My thought was to extend NFSv4 to provide native xattr support
> alongside named attribute support.  If NFSv4 is getting security label
> support then that's a moot point, unless there are other use cases for
> exposing xattrs to NFS clients.
>
> Overall, I think everyone is better off in the long run if we get the
> needed features into NFSv4.

Agreed, but we're talking years before that happens. Work on labeled
NFS has been ongoing since 1985 and there is no reason to believe that
NFSv4 is going to progress at a quicker pace than any of its predecessors.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
@ 2010-06-23 21:39                     ` Casey Schaufler
  0 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2010-06-23 21:39 UTC (permalink / raw)
  To: Chuck Lever
  Cc: James Morris, linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley,
	Christoph Hellwig

Chuck Lever wrote:
> Hi James-
>
> Good points, see below.
>
> On 06/22/10 08:26 PM, James Morris wrote:
>> On Tue, 22 Jun 2010, Chuck Lever wrote:
>>
>>> Another place to look is at NFSv4.  Some of the operations that can be
>>> performed on NFSv4 xattrs are probably nearly what you would want
>>> for an NFSv3
>>> implementation.  I think it is desirable for anything done for NFSv3
>>> to be
>>> compatible with NFSv4, as that is already a standard.
>>
>> Are you referring to named attributes?  I've looked into this, as have
>> others, and they appear to be fundamentally incompatible with Linux/BSD
>> xattrs.  They're filesystems within files, with different semantics
>> to the
>> name/value string model that we use.  I've heard a few developers say
>> they've looked at implementing xattrs over NAs, but found it unworkable.
>>
>> There's the issue of namespacing -- named attributes have no structured
>> namespace, whereas, Linux/BSD xattrs use namespaces to denote semantics.
>>
>> NAs are also intended to be managed at the user level without kernel
>> interaction, which conflicts with the semantics of our system
>> namespaces.
>>
>> In the past, I've seen several discussions on the issue conclude that
>> NFSv4 should have distinct Linux/BSD xattr OPs, although I don't know
>> what
>> the current thinking is.
>
> My thought was to extend NFSv4 to provide native xattr support
> alongside named attribute support.  If NFSv4 is getting security label
> support then that's a moot point, unless there are other use cases for
> exposing xattrs to NFS clients.
>
> Overall, I think everyone is better off in the long run if we get the
> needed features into NFSv4.

Agreed, but we're talking years before that happens. Work on labeled
NFS has been ongoing since 1985 and there is no reason to believe that
NFSv4 is going to progress at a quicker pace than any of its predecessors.


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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-23 18:58                   ` Trond Myklebust
@ 2010-06-23 22:51                     ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-23 22:51 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: J. Bruce Fields, Chuck Lever, linux-nfs, linux-security-module,
	Neil Brown, linux-fsdevel, Stephen Smalley, Christoph Hellwig,
	Casey Schaufler

On Wed, 23 Jun 2010, Trond Myklebust wrote:

> Is there really much interest in adding NFSv4 support for xattrs beyond
> the security label use case?

I'm not sure -- I would imagine the rationale would be similar to the case 
for named attributes.  I'll see if I can find out more from desktop 
developers.


- James
-- 
James Morris
<jmorris@namei.org>

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-23 17:29               ` Chuck Lever
@ 2010-06-23 23:49                     ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-23 23:49 UTC (permalink / raw)
  To: Chuck Lever
  Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	J. Bruce Fields, Neil Brown,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Stephen Smalley,
	Christoph Hellwig, Casey Schaufler, David Patrick Quigley

On Wed, 23 Jun 2010, Chuck Lever wrote:

> > There are also several proprietary NFSv3 xattr implementations out there
> > across various OSs -- it would be useful to provide users of these an
> > open, maintained option, as well as a pathway to NFSv4.
> 
> Given that there are disparate proprietary NFSv3 xattr implementations how do
> you plan to ensure interoperability without a protocol specification or at
> least an RPCL definition?  Is interoperability even practical at this point?

I intend to publish a specification and RPCL once the basic direction of 
the implementation is worked out.

For the case of proprietary implementations, the primary aim would not be 
interop, but to provide an option migrate to an open, mainstream 
implementation.

Note that this implementation I've posted could be made to interop with 
the IRIX version without much trouble.  I'm guessing the use-case here 
would be legacy IRIX clients which could be migrated to a Linux server.  

There's also the possibility of interop with FreeBSD.

> 
> You say "it would be useful to provide users of these an open, maintained
> option" but so far, security labels appear to be the only specific use case
> you've mentioned.  What other interesting applications of xattrs would be
> useful to expose via NFS?

As I mentioned to Trond, I'm not sure.  There are desktop apps which use 
xattrs -- I guess the question is how important it is that they use xattrs 
over NFS rather than their existing workarounds.

I wonder if OSX may be able to make use of them, rather than dropping ._ 
files all over my Linux NFS server.

> But the salient point, I think, is that if NFSv4 supports security labels,
> then those who require this feature will upgrade, independent of the broader
> adoption rate of NFSv4.

Security labeling is a standard feature of some OSs now, with general 
applicability.  By "those who require", you're potentially talking about 
most Fedora, RHEL, CentOS, OEL etc. users.

There's a lot of NFSv3 deployed out there and I don't see what's gained by 
forcing those people to choose between having security features enabled 
and upgrading to NFSv4.

We also don't know how long it will take to get all of the Labeled NFS 
changes into NFSv4.  In addition to extending the NFSv4 protocol itself, 
changes are also required in the RPC layer.  I've cc'd Dave Quigley, who 
may have a better idea of when the LNFS changes might be seen in 
production.

> > There is effort underway for NFSv4 in this area, designed around the full
> > security requirements of networked MAC labeling (Labeled NFS).  This
> > is a far more comprehensive effort, and obviously does not provide a
> > solution for NFSv3 users.
> 
> Having a design document would allow wider public assessment of whether there
> will be transition issues for users between your proposed NFSv3 xattr protocol
> and the labeled NFS work.  Do you have a published analysis of how NFSv3 xattr
> users would transition to NFSv4 labeled NFS?

No, but I'd expect that there would be transparent migration from NFSv3 
xattr to LNFS "Guest Mode":

https://tools.ietf.org/id/draft-quigley-nfsv4-sec-label-requirements-02.txt

> > With this NFSv3 xattr code, we are able to provide useful partial support
> > for MAC security labeling, for the scenario where the server simply stores
> > security labels without interpreting them.  This allows clients to perform
> > fine-grained security labeling over NFSv3, and enforce policy locally.
> 
> Given that NFSv3 deployments overwhelmingly use AUTH_SYS authentication and do
> not use wire encryption, how do you guarantee that xattrs aren't modified
> during transit between server and client?

For NFSv3, this is not addressed by the XATTR protocol, it's only about 
label transport.  You could use stronger NFS security, or even IPSec, 
although MAC labeling is not enforced on the server, so you do not have a 
complete solution.

These issues are addressed by LNFS, and not really viable for NFSv3.

> > There is considerable demand for NFS security labeling from users.
> > 
> > They've been deploying MAC security in production for several years now
> > but have always had to limit what they do regarding NFS.  This would
> > provide a useful partial solution for these users via NFSv3, as an
> > intermediate step towards adopting NFSv4 and Labeled NFS.
> 
> To play devil's advocate for a moment, what's the practical difference between
> not having support for security labels now, and having it sometime in the near
> future with NFSv4?

People can deploy stronger security sooner.

> of my concerns.  And if the only purpose of your post was for review before
> including it in RH products, then you might ignore the following concerns.

The aim is to have this integrated upstream, for use by Fedora, RHEL, 
CentOS, Oracle Enterprise Linux, Smack, embedded/mobile distros, Ubuntu, 
Gentoo, OpenSUSE etc.

> However, since we're talking about more than a few lines of code and a rather
> limited lifetime of usefulness, I think that before upstream considers
> accepting this new side band protocol, there are larger issues than whether it
> works today between two Linux systems, and avoids crashing.
> 
>   o  Do we understand the use cases for xattr over NFSv3?
>   o  Do we understand the interoperability requirements?
>   o  Will NFSv3's known security issues prevent this from being ultimately
> useful?
>   o  Is the work to support this feature upstream worth the year or two it
> will be useful before NFSv4 labels are available?
>   o  Are there other near term solutions?
>   o  How do we ensure this implementation continues to meet its specified
> requirements over time (ie is there a protocol spec? how will implementations
> be tested)?

These would be good topics to cover in a specification.

> I think you should consider some next steps:
> 
>   o  Talk with other O/S and NFS server implementers who have xattrs to see if
> there are real interoperability concerns
>   o  Produce an RPCL and a design document
>   o  Find other compelling use cases for exposing xattrs to NFS clients
>   o  Tie into the labeled NFS work to see whether a Linux prototype on NFSv4
> at this time would be useful

Prototype LNFS code has existed for quite some time:
http://selinuxproject.org/page/Labeled_NFS

>   o  Consider proposing xattr support in NFSv4 to the NFSv4 working group
> 
> And if you want to consider some specific mechanical items:
> 
>   o  Use the page cache instead of the RPC buffer for storing xattrs during
> RPC operations
>   o  Get an RPC program number from IANA
> 

Agreed.

Ok, thanks for all the feedback.


- James
-- 
James Morris
<jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
@ 2010-06-23 23:49                     ` James Morris
  0 siblings, 0 replies; 30+ messages in thread
From: James Morris @ 2010-06-23 23:49 UTC (permalink / raw)
  To: Chuck Lever
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley,
	Christoph Hellwig, Casey Schaufler, David Patrick Quigley

On Wed, 23 Jun 2010, Chuck Lever wrote:

> > There are also several proprietary NFSv3 xattr implementations out there
> > across various OSs -- it would be useful to provide users of these an
> > open, maintained option, as well as a pathway to NFSv4.
> 
> Given that there are disparate proprietary NFSv3 xattr implementations how do
> you plan to ensure interoperability without a protocol specification or at
> least an RPCL definition?  Is interoperability even practical at this point?

I intend to publish a specification and RPCL once the basic direction of 
the implementation is worked out.

For the case of proprietary implementations, the primary aim would not be 
interop, but to provide an option migrate to an open, mainstream 
implementation.

Note that this implementation I've posted could be made to interop with 
the IRIX version without much trouble.  I'm guessing the use-case here 
would be legacy IRIX clients which could be migrated to a Linux server.  

There's also the possibility of interop with FreeBSD.

> 
> You say "it would be useful to provide users of these an open, maintained
> option" but so far, security labels appear to be the only specific use case
> you've mentioned.  What other interesting applications of xattrs would be
> useful to expose via NFS?

As I mentioned to Trond, I'm not sure.  There are desktop apps which use 
xattrs -- I guess the question is how important it is that they use xattrs 
over NFS rather than their existing workarounds.

I wonder if OSX may be able to make use of them, rather than dropping ._ 
files all over my Linux NFS server.

> But the salient point, I think, is that if NFSv4 supports security labels,
> then those who require this feature will upgrade, independent of the broader
> adoption rate of NFSv4.

Security labeling is a standard feature of some OSs now, with general 
applicability.  By "those who require", you're potentially talking about 
most Fedora, RHEL, CentOS, OEL etc. users.

There's a lot of NFSv3 deployed out there and I don't see what's gained by 
forcing those people to choose between having security features enabled 
and upgrading to NFSv4.

We also don't know how long it will take to get all of the Labeled NFS 
changes into NFSv4.  In addition to extending the NFSv4 protocol itself, 
changes are also required in the RPC layer.  I've cc'd Dave Quigley, who 
may have a better idea of when the LNFS changes might be seen in 
production.

> > There is effort underway for NFSv4 in this area, designed around the full
> > security requirements of networked MAC labeling (Labeled NFS).  This
> > is a far more comprehensive effort, and obviously does not provide a
> > solution for NFSv3 users.
> 
> Having a design document would allow wider public assessment of whether there
> will be transition issues for users between your proposed NFSv3 xattr protocol
> and the labeled NFS work.  Do you have a published analysis of how NFSv3 xattr
> users would transition to NFSv4 labeled NFS?

No, but I'd expect that there would be transparent migration from NFSv3 
xattr to LNFS "Guest Mode":

https://tools.ietf.org/id/draft-quigley-nfsv4-sec-label-requirements-02.txt

> > With this NFSv3 xattr code, we are able to provide useful partial support
> > for MAC security labeling, for the scenario where the server simply stores
> > security labels without interpreting them.  This allows clients to perform
> > fine-grained security labeling over NFSv3, and enforce policy locally.
> 
> Given that NFSv3 deployments overwhelmingly use AUTH_SYS authentication and do
> not use wire encryption, how do you guarantee that xattrs aren't modified
> during transit between server and client?

For NFSv3, this is not addressed by the XATTR protocol, it's only about 
label transport.  You could use stronger NFS security, or even IPSec, 
although MAC labeling is not enforced on the server, so you do not have a 
complete solution.

These issues are addressed by LNFS, and not really viable for NFSv3.

> > There is considerable demand for NFS security labeling from users.
> > 
> > They've been deploying MAC security in production for several years now
> > but have always had to limit what they do regarding NFS.  This would
> > provide a useful partial solution for these users via NFSv3, as an
> > intermediate step towards adopting NFSv4 and Labeled NFS.
> 
> To play devil's advocate for a moment, what's the practical difference between
> not having support for security labels now, and having it sometime in the near
> future with NFSv4?

People can deploy stronger security sooner.

> of my concerns.  And if the only purpose of your post was for review before
> including it in RH products, then you might ignore the following concerns.

The aim is to have this integrated upstream, for use by Fedora, RHEL, 
CentOS, Oracle Enterprise Linux, Smack, embedded/mobile distros, Ubuntu, 
Gentoo, OpenSUSE etc.

> However, since we're talking about more than a few lines of code and a rather
> limited lifetime of usefulness, I think that before upstream considers
> accepting this new side band protocol, there are larger issues than whether it
> works today between two Linux systems, and avoids crashing.
> 
>   o  Do we understand the use cases for xattr over NFSv3?
>   o  Do we understand the interoperability requirements?
>   o  Will NFSv3's known security issues prevent this from being ultimately
> useful?
>   o  Is the work to support this feature upstream worth the year or two it
> will be useful before NFSv4 labels are available?
>   o  Are there other near term solutions?
>   o  How do we ensure this implementation continues to meet its specified
> requirements over time (ie is there a protocol spec? how will implementations
> be tested)?

These would be good topics to cover in a specification.

> I think you should consider some next steps:
> 
>   o  Talk with other O/S and NFS server implementers who have xattrs to see if
> there are real interoperability concerns
>   o  Produce an RPCL and a design document
>   o  Find other compelling use cases for exposing xattrs to NFS clients
>   o  Tie into the labeled NFS work to see whether a Linux prototype on NFSv4
> at this time would be useful

Prototype LNFS code has existed for quite some time:
http://selinuxproject.org/page/Labeled_NFS

>   o  Consider proposing xattr support in NFSv4 to the NFSv4 working group
> 
> And if you want to consider some specific mechanical items:
> 
>   o  Use the page cache instead of the RPC buffer for storing xattrs during
> RPC operations
>   o  Get an RPC program number from IANA
> 

Agreed.

Ok, thanks for all the feedback.


- James
-- 
James Morris
<jmorris@namei.org>

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
       [not found]   ` <alpine.LRH.2.00.1006212128160.13583-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
@ 2010-06-24  4:33       ` Serge E. Hallyn
  0 siblings, 0 replies; 30+ messages in thread
From: Serge E. Hallyn @ 2010-06-24  4:33 UTC (permalink / raw)
  To: James Morris
  Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	J. Bruce Fields, Neil Brown,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Stephen Smalley

Quoting James Morris (jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org):
...
> +int nfs3_proc_getxattr(struct inode *inode, const char *namespace,
> +		       const char *name, void *value, size_t size)
> +{
...
> +	res.xattr_val_len = size;
> +	res.xattr_val = kmalloc(size, GFP_KERNEL);
> +	if (!res.xattr_val)
> +		return -ENOMEM;
> +
> +	dprintk("NFS call getxattr %s%s %zd\n", namespace, name, size);
> +
> +	msg.rpc_proc = &server->client_xattr->cl_procinfo[XATTRPROC3_GETXATTR];
> +	nfs_fattr_init(&fattr);
> +	status = rpc_call_sync(server->client_xattr, &msg, 0);
> +
> +	dprintk("NFS reply getxattr: status=%d len=%d\n",
> +		status, res.xattr_val_len);
> +
> +	switch (status) {
> +	case 0:
> +		status = nfs_refresh_inode(inode, &fattr);
> +		break;
> +	case -EPFNOSUPPORT:
> +	case -EPROTONOSUPPORT:
> +		dprintk("NFS_V3_XATTR extension not supported; disabling\n");
> +		server->caps &= ~NFS_CAP_XATTR;
> +	case -ENOTSUPP:
> +		status = -EOPNOTSUPP;
> +	default:
> +		goto cleanup;
> +	}
> +
> +	status = res.xattr_val_len;
> +	if (status <= size)

res.xattr_val_len was set to size, as was status, and none of the
3 has been changed, so here status can't be > size can it?

Was this just a safety to prevent overrun, or did you mean to
do some other check?  (If a safety, then you'll still return
status > size, but with garbage in value, so i think you'd want
to also change status)

> +		memcpy(value, res.xattr_val, status);
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
@ 2010-06-24  4:33       ` Serge E. Hallyn
  0 siblings, 0 replies; 30+ messages in thread
From: Serge E. Hallyn @ 2010-06-24  4:33 UTC (permalink / raw)
  To: James Morris
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley

Quoting James Morris (jmorris@namei.org):
...
> +int nfs3_proc_getxattr(struct inode *inode, const char *namespace,
> +		       const char *name, void *value, size_t size)
> +{
...
> +	res.xattr_val_len = size;
> +	res.xattr_val = kmalloc(size, GFP_KERNEL);
> +	if (!res.xattr_val)
> +		return -ENOMEM;
> +
> +	dprintk("NFS call getxattr %s%s %zd\n", namespace, name, size);
> +
> +	msg.rpc_proc = &server->client_xattr->cl_procinfo[XATTRPROC3_GETXATTR];
> +	nfs_fattr_init(&fattr);
> +	status = rpc_call_sync(server->client_xattr, &msg, 0);
> +
> +	dprintk("NFS reply getxattr: status=%d len=%d\n",
> +		status, res.xattr_val_len);
> +
> +	switch (status) {
> +	case 0:
> +		status = nfs_refresh_inode(inode, &fattr);
> +		break;
> +	case -EPFNOSUPPORT:
> +	case -EPROTONOSUPPORT:
> +		dprintk("NFS_V3_XATTR extension not supported; disabling\n");
> +		server->caps &= ~NFS_CAP_XATTR;
> +	case -ENOTSUPP:
> +		status = -EOPNOTSUPP;
> +	default:
> +		goto cleanup;
> +	}
> +
> +	status = res.xattr_val_len;
> +	if (status <= size)

res.xattr_val_len was set to size, as was status, and none of the
3 has been changed, so here status can't be > size can it?

Was this just a safety to prevent overrun, or did you mean to
do some other check?  (If a safety, then you'll still return
status > size, but with garbage in value, so i think you'd want
to also change status)

> +		memcpy(value, res.xattr_val, status);

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-24  4:33       ` Serge E. Hallyn
  (?)
@ 2010-06-24  8:35       ` James Morris
  2010-06-24 13:44         ` Serge E. Hallyn
  -1 siblings, 1 reply; 30+ messages in thread
From: James Morris @ 2010-06-24  8:35 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley

On Wed, 23 Jun 2010, Serge E. Hallyn wrote:

> > +	status = res.xattr_val_len;
> > +	if (status <= size)
> 
> res.xattr_val_len was set to size, as was status, and none of the
> 3 has been changed, so here status can't be > size can it?

res is part of msg, and is updated by the RPC layer when decoding the 
response (via xdr_decode_string_inplace()).

> Was this just a safety to prevent overrun, or did you mean to
> do some other check?  (If a safety, then you'll still return
> status > size, but with garbage in value, so i think you'd want
> to also change status)
> 
> > +		memcpy(value, res.xattr_val, status);
> 

Yes, the check stops us from copying more than the max. expected size to 
'value'.

It looks like we do need to return -EINVAL if the check fails, and likely 
the same with listxattr().  (Or is there a better error code for reporting 
invalid messages from the peer?)


- James
-- 
James Morris
<jmorris@namei.org>

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

* Re: [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol
  2010-06-24  8:35       ` James Morris
@ 2010-06-24 13:44         ` Serge E. Hallyn
  0 siblings, 0 replies; 30+ messages in thread
From: Serge E. Hallyn @ 2010-06-24 13:44 UTC (permalink / raw)
  To: James Morris
  Cc: linux-nfs, linux-security-module, Trond Myklebust,
	J. Bruce Fields, Neil Brown, linux-fsdevel, Stephen Smalley

Quoting James Morris (jmorris@namei.org):
> On Wed, 23 Jun 2010, Serge E. Hallyn wrote:
> 
> > > +	status = res.xattr_val_len;
> > > +	if (status <= size)
> > 
> > res.xattr_val_len was set to size, as was status, and none of the
> > 3 has been changed, so here status can't be > size can it?
> 
> res is part of msg, and is updated by the RPC layer when decoding the 
> response (via xdr_decode_string_inplace()).

Ah, I see, it's passed in as a member of msg.  Thanks.

> > Was this just a safety to prevent overrun, or did you mean to
> > do some other check?  (If a safety, then you'll still return
> > status > size, but with garbage in value, so i think you'd want
> > to also change status)
> > 
> > > +		memcpy(value, res.xattr_val, status);
> > 
> 
> Yes, the check stops us from copying more than the max. expected size to 
> 'value'.
> 
> It looks like we do need to return -EINVAL if the check fails, and likely 
> the same with listxattr().  (Or is there a better error code for reporting 
> invalid messages from the peer?)

That'd be my pick.

-serge

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

end of thread, other threads:[~2010-06-24 13:43 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-21 11:25 [PATCH 0/8][v05][RFC] NFSv3: implement extended attribute protocol (XATTR) James Morris
2010-06-21 11:27 ` [PATCH 1/8][RFC v05] NFSv3: convert client to generic xattr API James Morris
2010-06-21 11:28 ` [PATCH 2/8][RFC v05] NFSv3: add xattr API config option for client James Morris
2010-06-21 11:29 ` [PATCH 3/8][RFC v05] NFSv3: add client implementation of XATTR protocol James Morris
2010-06-21 20:02   ` Chuck Lever
     [not found]     ` <4C1FC553.4030904-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2010-06-21 23:21       ` James Morris
2010-06-21 23:21         ` James Morris
2010-06-22 15:32         ` Chuck Lever
     [not found]           ` <4C20D779.5040008-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2010-06-23  0:26             ` James Morris
2010-06-23  0:26               ` James Morris
2010-06-23 15:56               ` Casey Schaufler
2010-06-23 17:29               ` Chuck Lever
     [not found]                 ` <4C224463.90306-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2010-06-23 21:39                   ` Casey Schaufler
2010-06-23 21:39                     ` Casey Schaufler
2010-06-23 23:49                   ` James Morris
2010-06-23 23:49                     ` James Morris
     [not found]               ` <alpine.LRH.2.00.1006230857450.25778-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
2010-06-23 18:35                 ` J. Bruce Fields
2010-06-23 18:35                   ` J. Bruce Fields
2010-06-23 18:58                   ` Trond Myklebust
2010-06-23 22:51                     ` James Morris
     [not found]   ` <alpine.LRH.2.00.1006212128160.13583-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
2010-06-24  4:33     ` Serge E. Hallyn
2010-06-24  4:33       ` Serge E. Hallyn
2010-06-24  8:35       ` James Morris
2010-06-24 13:44         ` Serge E. Hallyn
2010-06-21 11:30 ` [PATCH 4/8][RFC v05] NFSv3: add server " James Morris
2010-06-21 11:30 ` [PATCH 5/8][RFC v05] XATTR: add new top level nfsd namespace and implement ext3 support James Morris
     [not found] ` <alpine.LRH.2.00.1006212051530.13583-CK9fWmtY32x9JUWOpEiw7w@public.gmane.org>
2010-06-21 11:31   ` [PATCH 6/8][RFC v05] NFSv3: Add server namespace support for XATTR protocol implementation James Morris
2010-06-21 11:31     ` James Morris
2010-06-21 11:32 ` [PATCH 7/8][RFC v05] NFSv3: Add xattr and xattrsec mount options to support XATTR protocol James Morris
2010-06-21 11:33 ` [PATCH 8/8][RFC v05] SELinux/NFSv3: Enable xattr labeling behavior for SELinux with the " James Morris

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.