Skip to contentSkip to conversation

Extended attribute fixes, etc.

PATCH In review Andreas Gruenbacher 3 participants 9 messages Feb 11, 2003 – Feb 15, 2003

Subsystems (4)
VFS XFS filesystem ext2 file system JFS filesystem
lore
Share & tools
lore

Andreas Gruenbacher

author
#0 lore raw

PATCH 1/1 Extended attribute fixes, etc.

Hi Andrew,

here are five patches against 2.5.60. Each file contains a brief description of what it does.

The first two fix an [un]lock_kernel() bug, and an iops->listxattr() bug that for some reason hasn't found its way into 2.5 yet. (Both bugs do not exist in the current 2.4.19/2.4.20 patches at acl.bestbits.at).

The third to fifth are all steps towards trusted extended attributes, which are useful for privileged processes (mostly daemons). One use for this is Hierarchical Storage Management, in which a user space daemon stores online/offline information for files in trusted EA's, and the kernel communicates requests to bring files online to that daemon. This class of EA's will also find its way into XFS and ReiserFS, and expectedly also into JFS in this form. (Trusted EAs are included in the 2.4.19/2.4.20 patches as well.)

The intended patch order is:

kernel_lock_bug.diff
ext2_ext3_listxattr-bug.diff
xattr-flags.diff
xattr-flags-policy.diff
xattr-trusted.diff

Could you please feed the patches to Linus?

Thank you very much,
Andreas Gruenbacher.

This patch fixes a bug in the ext2 and ext3 listxattr operation: Even if an attribute is hidden from the user, the terminating NULL character was included in the listxattr result. After the patch this doesn't happen anymore.

Index: linux-2.5.60/fs/ext2/acl.c

fs/ext2/acl.c +6 -6
@@ -421,26 +421,26 @@
 ext2_xattr_list_acl_access(char *list, struct inode *inode,
 			   const char *name, int name_len)
 {
-	const size_t len = sizeof(XATTR_NAME_ACL_ACCESS)-1;
+	const size_t size = sizeof(XATTR_NAME_ACL_ACCESS);
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
 	if (list)
-		memcpy(list, XATTR_NAME_ACL_ACCESS, len);
-	return len;
+		memcpy(list, XATTR_NAME_ACL_ACCESS, size);
+	return size;
 }
 
 static size_t
 ext2_xattr_list_acl_default(char *list, struct inode *inode,
 			    const char *name, int name_len)
 {
-	const size_t len = sizeof(XATTR_NAME_ACL_DEFAULT)-1;
+	const size_t size = sizeof(XATTR_NAME_ACL_DEFAULT);
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
 	if (list)
-		memcpy(list, XATTR_NAME_ACL_DEFAULT, len);
-	return len;
+		memcpy(list, XATTR_NAME_ACL_DEFAULT, size);
+	return size;
 }
 
 static int

Index: linux-2.5.60/fs/ext2/xattr.c

fs/ext2/xattr.c +3 -6
@@ -409,10 +409,9 @@
 			goto bad_block;
 
 		handler = ext2_xattr_handler(entry->e_name_index);
-		if (handler) {
+		if (handler)
 			size += handler->list(NULL, inode, entry->e_name,
-					      entry->e_name_len) + 1;
-		}
+					      entry->e_name_len);
 	}
 
 	if (ext2_xattr_cache_insert(bh))

@@ -433,11 +432,9 @@
 		struct ext2_xattr_handler *handler;
 		
 		handler = ext2_xattr_handler(entry->e_name_index);
-		if (handler) {
+		if (handler)
 			buf += handler->list(buf, inode, entry->e_name,
 					     entry->e_name_len);
-			*buf++ = '\0';
-		}
 	}
 	error = size;

Index: linux-2.5.60/fs/ext2/xattr_user.c

fs/ext2/xattr_user.c +2 -1
@@ -29,8 +29,9 @@
 	if (list) {
 		memcpy(list, XATTR_USER_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
+		list[prefix_len + name_len] = '\0';
 	}
-	return prefix_len + name_len;
+	return prefix_len + name_len + 1;
 }
 
 static int

Index: linux-2.5.60/fs/ext3/acl.c

fs/ext3/acl.c +6 -6
@@ -434,26 +434,26 @@
 ext3_xattr_list_acl_access(char *list, struct inode *inode,
 			   const char *name, int name_len)
 {
-	const size_t len = sizeof(XATTR_NAME_ACL_ACCESS)-1;
+	const size_t size = sizeof(XATTR_NAME_ACL_ACCESS);
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
 	if (list)
-		memcpy(list, XATTR_NAME_ACL_ACCESS, len);
-	return len;
+		memcpy(list, XATTR_NAME_ACL_ACCESS, size);
+	return size;
 }
 
 static size_t
 ext3_xattr_list_acl_default(char *list, struct inode *inode,
 			    const char *name, int name_len)
 {
-	const size_t len = sizeof(XATTR_NAME_ACL_DEFAULT)-1;
+	const size_t size = sizeof(XATTR_NAME_ACL_DEFAULT);
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
 	if (list)
-		memcpy(list, XATTR_NAME_ACL_DEFAULT, len);
-	return len;
+		memcpy(list, XATTR_NAME_ACL_DEFAULT, size);
+	return size;
 }
 
 static int

Index: linux-2.5.60/fs/ext3/xattr.c

fs/ext3/xattr.c +3 -6
@@ -402,10 +402,9 @@
 			goto bad_block;
 
 		handler = ext3_xattr_handler(entry->e_name_index);
-		if (handler) {
+		if (handler)
 			size += handler->list(NULL, inode, entry->e_name,
-					      entry->e_name_len) + 1;
-		}
+					      entry->e_name_len);
 	}
 
 	if (ext3_xattr_cache_insert(bh))

@@ -426,11 +425,9 @@
 		struct ext3_xattr_handler *handler;
 
 		handler = ext3_xattr_handler(entry->e_name_index);
-		if (handler) {
+		if (handler)
 			buf += handler->list(buf, inode, entry->e_name,
 					     entry->e_name_len);
-			*buf++ = '\0';
-		}
 	}
 	error = size;

Index: linux-2.5.60/fs/ext3/xattr_user.c

fs/ext3/xattr_user.c +2 -1
@@ -31,8 +31,9 @@
 	if (list) {
 		memcpy(list, XATTR_USER_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
+		list[prefix_len + name_len] = '\0';
 	}
-	return prefix_len + name_len;
+	return prefix_len + name_len + 1;
 }
 
 static int

This patch fixes an unbalanced lock_kernel()/unlock_kernel() path in the ext3 extended attributes code. Instead of fixing this in fs/ext3/xattr_user.c, the locking code is modev to fs/ext3/xattr.c, since most other types of extended attributes will need the exact same functioanlity.

Andreas Gruenbacher <[email protected]>

Index: linux-2.5.60/fs/ext3/xattr.c

fs/ext3/xattr.c +42 -10

96 lines not on this page · raw message

Index: linux-2.5.60/fs/ext3/xattr.h

fs/ext3/xattr.h +10 -2
@@ -73,7 +73,8 @@
 
 extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
 extern int ext3_xattr_list(struct inode *, char *, size_t);
-extern int ext3_xattr_set(handle_t *handle, struct inode *, int, const char *, const void *, size_t, int);
+extern int ext3_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
+extern int ext3_xattr_set_handle(handle_t *, struct inode *, int, const char *, const void *, size_t, int);
 
 extern void ext3_xattr_delete_inode(handle_t *, struct inode *);
 extern void ext3_xattr_put_super(struct super_block *);

@@ -101,7 +102,14 @@
 }
 
 static inline int
-ext3_xattr_set(handle_t *handle, struct inode *inode, int name_index,
+ext3_xattr_set(struct inode *inode, int name_index, const char *name,
+	       const void *value, size_t size, int flags)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int
+ext3_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 	       const char *name, const void *value, size_t size, int flags)
 {
 	return -EOPNOTSUPP;

Index: linux-2.5.60/fs/ext3/xattr_user.c

fs/ext3/xattr_user.c +2 -10
@@ -61,7 +61,6 @@
 ext3_xattr_user_set(struct inode *inode, const char *name,
 		    const void *value, size_t size, int flags)
 {
-	handle_t *handle;
 	int error;
 
 	if (strcmp(name, "") == 0)

@@ -79,16 +78,9 @@
 	if (error)
 		return error;
   
-	lock_kernel();
-	handle = ext3_journal_start(inode, EXT3_XATTR_TRANS_BLOCKS);
-	if (IS_ERR(handle))
-		return PTR_ERR(handle);
-	error = ext3_xattr_set(handle, inode, EXT3_XATTR_INDEX_USER, name,
-			       value, size, flags);
-	ext3_journal_stop(handle, inode);
-	unlock_kernel();
+	return ext3_xattr_set(inode, EXT3_XATTR_INDEX_USER, name,
+			      value, size, flags);
 
-	return error;
 }
 
 struct ext3_xattr_handler ext3_xattr_user_handler = {

This patch adds trusted extended attributes. Trusted extended attributes are visible and accessible only to processes that have the CAP_SYS_ADMIN capability. Attributes in this class are used to implement mechanisms in user space (i.e., outside the kernel) which keep information in extended attributes to which ordinary processes have no access. HSM is an example.

Index: linux-2.5.60/fs/ext2/Makefile

fs/ext2/Makefile +1 -1
@@ -8,7 +8,7 @@
 	     ioctl.o namei.o super.o symlink.o
 
 ifeq ($(CONFIG_EXT2_FS_XATTR),y)
-ext2-objs += xattr.o xattr_user.o
+ext2-objs += xattr.o xattr_user.o xattr_trusted.o
 endif
 
 ifeq ($(CONFIG_EXT2_FS_POSIX_ACL),y)

Index: linux-2.5.60/fs/ext2/xattr.c

fs/ext2/xattr.c +17 -6
@@ -1096,27 +1096,35 @@
 {
 	int	err;
 	
-	err = ext2_xattr_register(EXT2_XATTR_INDEX_USER, &ext2_xattr_user_handler);
+	err = ext2_xattr_register(EXT2_XATTR_INDEX_USER,
+				  &ext2_xattr_user_handler);
 	if (err)
 		return err;
+	err = ext2_xattr_register(EXT2_XATTR_INDEX_TRUSTED,
+				  &ext2_xattr_trusted_handler);
+	if (err)
+		goto out;
 #ifdef CONFIG_EXT2_FS_POSIX_ACL
 	err = init_ext2_acl();
 	if (err)
-		goto out;
+		goto out1;
 #endif
 	ext2_xattr_cache = mb_cache_create("ext2_xattr", NULL,
 		sizeof(struct mb_cache_entry) +
 		sizeof(struct mb_cache_entry_index), 1, 6);
 	if (!ext2_xattr_cache) {
 		err = -ENOMEM;
-		goto out1;
+		goto out2;
 	}
 	return 0;
-out1:
+out2:
 #ifdef CONFIG_EXT2_FS_POSIX_ACL
 	exit_ext2_acl();
-out:
+out1:
 #endif
+	ext2_xattr_unregister(EXT2_XATTR_INDEX_TRUSTED,
+			      &ext2_xattr_trusted_handler);
+out:
 	ext2_xattr_unregister(EXT2_XATTR_INDEX_USER,
 			      &ext2_xattr_user_handler);
 	return err;

@@ -1129,5 +1137,8 @@
 #ifdef CONFIG_EXT2_FS_POSIX_ACL
 	exit_ext2_acl();
 #endif
-	ext2_xattr_unregister(EXT2_XATTR_INDEX_USER, &ext2_xattr_user_handler);
+	ext2_xattr_unregister(EXT2_XATTR_INDEX_TRUSTED,
+			      &ext2_xattr_trusted_handler);
+	ext2_xattr_unregister(EXT2_XATTR_INDEX_USER,
+			      &ext2_xattr_user_handler);
 }

Index: linux-2.5.60/fs/ext2/xattr.h

fs/ext2/xattr.h +2 -0
@@ -21,6 +21,7 @@
 #define EXT2_XATTR_INDEX_USER			1
 #define EXT2_XATTR_INDEX_POSIX_ACL_ACCESS	2
 #define EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT	3
+#define EXT2_XATTR_INDEX_TRUSTED		4
 
 struct ext2_xattr_header {
 	__u32	h_magic;	/* magic number for identification */

@@ -132,4 +133,5 @@
 # endif  /* CONFIG_EXT2_FS_XATTR */
 
 extern struct ext2_xattr_handler ext2_xattr_user_handler;
+extern struct ext2_xattr_handler ext2_xattr_trusted_handler;

Index: linux-2.5.60/fs/ext2/xattr_trusted.c

fs/ext2/xattr_trusted.c +63 -0

63 lines not on this page · raw message

Index: linux-2.5.60/fs/ext3/Makefile

fs/ext3/Makefile +1 -1
@@ -8,7 +8,7 @@
 		ioctl.o namei.o super.o symlink.o hash.o
 
 ifeq ($(CONFIG_EXT3_FS_XATTR),y)
-ext3-objs += xattr.o xattr_user.o
+ext3-objs += xattr.o xattr_user.o xattr_trusted.o
 endif
 
 ifeq ($(CONFIG_EXT3_FS_POSIX_ACL),y)

Index: linux-2.5.60/fs/ext3/xattr.c

fs/ext3/xattr.c +17 -6
@@ -1133,27 +1133,35 @@
 {
 	int	err;
 	
-	err = ext3_xattr_register(EXT3_XATTR_INDEX_USER, &ext3_xattr_user_handler);
+	err = ext3_xattr_register(EXT3_XATTR_INDEX_USER,
+				  &ext3_xattr_user_handler);
 	if (err)
 		return err;
+	err = ext3_xattr_register(EXT3_XATTR_INDEX_TRUSTED,
+				  &ext3_xattr_trusted_handler);
+	if (err)
+		goto out;
 #ifdef CONFIG_EXT3_FS_POSIX_ACL
 	err = init_ext3_acl();
 	if (err)
-		goto out;
+		goto out1;
 #endif
 	ext3_xattr_cache = mb_cache_create("ext3_xattr", NULL,
 		sizeof(struct mb_cache_entry) +
 		sizeof(struct mb_cache_entry_index), 1, 6);
 	if (!ext3_xattr_cache) {
 		err = -ENOMEM;
-		goto out1;
+		goto out2;
 	}
 	return 0;
-out1:
+out2:
 #ifdef CONFIG_EXT3_FS_POSIX_ACL
 	exit_ext3_acl();
-out:
+out1:
 #endif
+	ext3_xattr_unregister(EXT3_XATTR_INDEX_TRUSTED,
+			      &ext3_xattr_trusted_handler);
+out:
 	ext3_xattr_unregister(EXT3_XATTR_INDEX_USER,
 			      &ext3_xattr_user_handler);
 	return err;

@@ -1168,5 +1176,8 @@
 #ifdef CONFIG_EXT3_FS_POSIX_ACL
 	exit_ext3_acl();
 #endif
-	ext3_xattr_unregister(EXT3_XATTR_INDEX_USER, &ext3_xattr_user_handler);
+	ext3_xattr_unregister(EXT3_XATTR_INDEX_TRUSTED,
+			      &ext3_xattr_trusted_handler);
+	ext3_xattr_unregister(EXT3_XATTR_INDEX_USER,
+			      &ext3_xattr_user_handler);
 }

Index: linux-2.5.60/fs/ext3/xattr.h

fs/ext3/xattr.h +2 -0
@@ -20,6 +20,7 @@
 #define EXT3_XATTR_INDEX_USER			1
 #define EXT3_XATTR_INDEX_POSIX_ACL_ACCESS	2
 #define EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT	3
+#define EXT3_XATTR_INDEX_TRUSTED		4
 
 struct ext3_xattr_header {
 	__u32	h_magic;	/* magic number for identification */

@@ -139,3 +140,4 @@
 # endif  /* CONFIG_EXT3_FS_XATTR */
 
 extern struct ext3_xattr_handler ext3_xattr_user_handler;
+extern struct ext3_xattr_handler ext3_xattr_trusted_handler;

Index: linux-2.5.60/fs/ext3/xattr_trusted.c

fs/ext3/xattr_trusted.c +64 -0

64 lines not on this page · raw message

This adds the XATTR_KERNEL_CONTEXT extended attributes flag. Kernel code may use this flag to override extended attribute permission restrictions that would otherwise be imposed on the calling process.

fs/ext2/xattr_user.c +22 -19

72 lines not on this page · raw message

Index: linux-2.5.60/fs/ext3/xattr_user.c

fs/ext3/xattr_user.c +22 -20

75 lines not on this page · raw message

Index: linux-2.5.60/include/linux/xattr.h

include/linux/xattr.h +3 -2
@@ -9,7 +9,8 @@
 #ifndef _LINUX_XATTR_H
 #define _LINUX_XATTR_H
 
-#define XATTR_CREATE	0x1	/* set value, fail if attr already exists */
-#define XATTR_REPLACE	0x2	/* set value, fail if attr does not exist */
+#define XATTR_CREATE		0x1	/* fail if attr already exists */
+#define XATTR_REPLACE		0x2	/* fail if attr does not exist */
+#define XATTR_KERNEL_CONTEXT	0x4	/* called from kernel context */
 
 #endif	/* _LINUX_XATTR_H */

This adds flags parameters to the getxattr, listxattr, and removexattr inode operations. This is in preparation for the next patch, which allows in-kernel code (i.e., modules) to override extended attribute permission restrictions (which in turn is used by HSM implementations and the like).

Index: linux-2.5.60/fs/ext2/acl.c

fs/ext2/acl.c +6 -5
@@ -419,7 +419,7 @@
  */
 static size_t
 ext2_xattr_list_acl_access(char *list, struct inode *inode,
-			   const char *name, int name_len)
+			   const char *name, int name_len, int flags)
 {
 	const size_t size = sizeof(XATTR_NAME_ACL_ACCESS);
 

@@ -432,7 +432,7 @@
 
 static size_t
 ext2_xattr_list_acl_default(char *list, struct inode *inode,
-			    const char *name, int name_len)
+			    const char *name, int name_len, int flags)
 {
 	const size_t size = sizeof(XATTR_NAME_ACL_DEFAULT);
 

@@ -465,7 +465,7 @@
 
 static int
 ext2_xattr_get_acl_access(struct inode *inode, const char *name,
-			  void *buffer, size_t size)
+			  void *buffer, size_t size, int flags)
 {
 	if (strcmp(name, "") != 0)
 		return -EINVAL;

@@ -474,7 +474,7 @@
 
 static int
 ext2_xattr_get_acl_default(struct inode *inode, const char *name,
-			   void *buffer, size_t size)
+			   void *buffer, size_t size, int flags)
 {
 	if (strcmp(name, "") != 0)
 		return -EINVAL;

@@ -482,7 +482,8 @@
 }
 
 static int
-ext2_xattr_set_acl(struct inode *inode, int type, const void *value, size_t size)
+ext2_xattr_set_acl(struct inode *inode, int type, const void *value,
+		   size_t size)
 {
 	struct posix_acl *acl;
 	int error;

Index: linux-2.5.60/fs/ext2/xattr.c

fs/ext2/xattr.c +10 -9
@@ -199,7 +199,7 @@
  */
 ssize_t
 ext2_getxattr(struct dentry *dentry, const char *name,
-	      void *buffer, size_t size)
+	      void *buffer, size_t size, int flags)
 {
 	struct ext2_xattr_handler *handler;
 	struct inode *inode = dentry->d_inode;

@@ -207,7 +207,7 @@
 	handler = ext2_xattr_resolve_name(&name);
 	if (!handler)
 		return -EOPNOTSUPP;
-	return handler->get(inode, name, buffer, size);
+	return handler->get(inode, name, buffer, size, flags);
 }
 
 /*

@@ -217,9 +217,9 @@
  * BKL held [before 2.5.x]
  */
 ssize_t
-ext2_listxattr(struct dentry *dentry, char *buffer, size_t size)
+ext2_listxattr(struct dentry *dentry, char *buffer, size_t size, int flags)
 {
-	return ext2_xattr_list(dentry->d_inode, buffer, size);
+	return ext2_xattr_list(dentry->d_inode, buffer, size, flags);
 }
 
 /*

@@ -250,7 +250,7 @@
  * BKL held [before 2.5.x]
  */
 int
-ext2_removexattr(struct dentry *dentry, const char *name)
+ext2_removexattr(struct dentry *dentry, const char *name, int flags)
 {
 	struct ext2_xattr_handler *handler;
 	struct inode *inode = dentry->d_inode;

@@ -258,7 +258,7 @@
 	handler = ext2_xattr_resolve_name(&name);
 	if (!handler)
 		return -EOPNOTSUPP;
-	return handler->set(inode, name, NULL, 0, XATTR_REPLACE);
+	return handler->set(inode, name, NULL, 0, flags | XATTR_REPLACE);
 }
 
 /*

@@ -371,7 +371,8 @@
  * used / required on success.
  */
 int
-ext2_xattr_list(struct inode *inode, char *buffer, size_t buffer_size)
+ext2_xattr_list(struct inode *inode, char *buffer, size_t buffer_size,
+		int flags)
 {
 	struct buffer_head *bh = NULL;
 	struct ext2_xattr_entry *entry;

@@ -411,7 +412,7 @@
 		handler = ext2_xattr_handler(entry->e_name_index);
 		if (handler)
 			size += handler->list(NULL, inode, entry->e_name,
-					      entry->e_name_len);
+					      entry->e_name_len, flags);
 	}
 
 	if (ext2_xattr_cache_insert(bh))

@@ -434,7 +435,7 @@
 		handler = ext2_xattr_handler(entry->e_name_index);
 		if (handler)
 			buf += handler->list(buf, inode, entry->e_name,
-					     entry->e_name_len);
+					     entry->e_name_len, flags);
 	}
 	error = size;

Index: linux-2.5.60/fs/ext2/xattr.h

fs/ext2/xattr.h +6 -6
@@ -57,9 +57,9 @@
 struct ext2_xattr_handler {
 	char *prefix;
 	size_t (*list)(char *list, struct inode *inode, const char *name,
-		       int name_len);
+		       int name_len, int flags);
 	int (*get)(struct inode *inode, const char *name, void *buffer,
-		   size_t size);
+		   size_t size, int flags);
 	int (*set)(struct inode *inode, const char *name, const void *buffer,
 		   size_t size, int flags);
 };

@@ -68,12 +68,12 @@
 extern void ext2_xattr_unregister(int, struct ext2_xattr_handler *);
 
 extern int ext2_setxattr(struct dentry *, const char *, const void *, size_t, int);
-extern ssize_t ext2_getxattr(struct dentry *, const char *, void *, size_t);
-extern ssize_t ext2_listxattr(struct dentry *, char *, size_t);
-extern int ext2_removexattr(struct dentry *, const char *);
+extern ssize_t ext2_getxattr(struct dentry *, const char *, void *, size_t, int);
+extern ssize_t ext2_listxattr(struct dentry *, char *, size_t, int);
+extern int ext2_removexattr(struct dentry *, const char *, int);
 
 extern int ext2_xattr_get(struct inode *, int, const char *, void *, size_t);
-extern int ext2_xattr_list(struct inode *, char *, size_t);
+extern int ext2_xattr_list(struct inode *, char *, size_t, int flags);
 extern int ext2_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
 
 extern void ext2_xattr_delete_inode(struct inode *);

Index: linux-2.5.60/fs/ext2/xattr_user.c

fs/ext2/xattr_user.c +2 -2
@@ -19,7 +19,7 @@
 
 static size_t
 ext2_xattr_user_list(char *list, struct inode *inode,
-		     const char *name, int name_len)
+		     const char *name, int name_len, int flags)
 {
 	const int prefix_len = sizeof(XATTR_USER_PREFIX)-1;
 

@@ -36,7 +36,7 @@
 
 static int
 ext2_xattr_user_get(struct inode *inode, const char *name,
-		    void *buffer, size_t size)
+		    void *buffer, size_t size, int flags)
 {
 	int error;

Index: linux-2.5.60/fs/ext3/acl.c

fs/ext3/acl.c +6 -5
@@ -432,7 +432,7 @@
  */
 static size_t
 ext3_xattr_list_acl_access(char *list, struct inode *inode,
-			   const char *name, int name_len)
+			   const char *name, int name_len, int flags)
 {
 	const size_t size = sizeof(XATTR_NAME_ACL_ACCESS);
 

@@ -445,7 +445,7 @@
 
 static size_t
 ext3_xattr_list_acl_default(char *list, struct inode *inode,
-			    const char *name, int name_len)
+			    const char *name, int name_len, int flags)
 {
 	const size_t size = sizeof(XATTR_NAME_ACL_DEFAULT);
 

@@ -478,7 +478,7 @@
 
 static int
 ext3_xattr_get_acl_access(struct inode *inode, const char *name,
-			  void *buffer, size_t size)
+			  void *buffer, size_t size, int flags)
 {
 	if (strcmp(name, "") != 0)
 		return -EINVAL;

@@ -487,7 +487,7 @@
 
 static int
 ext3_xattr_get_acl_default(struct inode *inode, const char *name,
-			   void *buffer, size_t size)
+			   void *buffer, size_t size, int flags)
 {
 	if (strcmp(name, "") != 0)
 		return -EINVAL;

@@ -495,7 +495,8 @@
 }
 
 static int
-ext3_xattr_set_acl(struct inode *inode, int type, const void *value, size_t size)
+ext3_xattr_set_acl(struct inode *inode, int type, const void *value,
+		   size_t size)
 {
 	handle_t *handle;
 	struct posix_acl *acl;

Index: linux-2.5.60/fs/ext3/xattr.c

fs/ext3/xattr.c +10 -9
@@ -195,7 +195,7 @@
  */
 ssize_t
 ext3_getxattr(struct dentry *dentry, const char *name,
-	      void *buffer, size_t size)
+	      void *buffer, size_t size, int flags)
 {
 	struct ext3_xattr_handler *handler;
 	struct inode *inode = dentry->d_inode;

@@ -203,7 +203,7 @@
 	handler = ext3_xattr_resolve_name(&name);
 	if (!handler)
 		return -EOPNOTSUPP;
-	return handler->get(inode, name, buffer, size);
+	return handler->get(inode, name, buffer, size, flags);
 }
 
 /*

@@ -212,9 +212,9 @@
  * dentry->d_inode->i_sem down
  */
 ssize_t
-ext3_listxattr(struct dentry *dentry, char *buffer, size_t size)
+ext3_listxattr(struct dentry *dentry, char *buffer, size_t size, int flags)
 {
-	return ext3_xattr_list(dentry->d_inode, buffer, size);
+	return ext3_xattr_list(dentry->d_inode, buffer, size, flags);
 }
 
 /*

@@ -243,7 +243,7 @@
  * dentry->d_inode->i_sem down
  */
 int
-ext3_removexattr(struct dentry *dentry, const char *name)
+ext3_removexattr(struct dentry *dentry, const char *name, int flags)
 {
 	struct ext3_xattr_handler *handler;
 	struct inode *inode = dentry->d_inode;

@@ -251,7 +251,7 @@
 	handler = ext3_xattr_resolve_name(&name);
 	if (!handler)
 		return -EOPNOTSUPP;
-	return handler->set(inode, name, NULL, 0, XATTR_REPLACE);
+	return handler->set(inode, name, NULL, 0, flags | XATTR_REPLACE);
 }
 
 /*

@@ -364,7 +364,8 @@
  * used / required on success.
  */
 int
-ext3_xattr_list(struct inode *inode, char *buffer, size_t buffer_size)
+ext3_xattr_list(struct inode *inode, char *buffer, size_t buffer_size,
+		int flags)
 {
 	struct buffer_head *bh = NULL;
 	struct ext3_xattr_entry *entry;

@@ -404,7 +405,7 @@
 		handler = ext3_xattr_handler(entry->e_name_index);
 		if (handler)
 			size += handler->list(NULL, inode, entry->e_name,
-					      entry->e_name_len);
+					      entry->e_name_len, flags);
 	}
 
 	if (ext3_xattr_cache_insert(bh))

@@ -427,7 +428,7 @@
 		handler = ext3_xattr_handler(entry->e_name_index);
 		if (handler)
 			buf += handler->list(buf, inode, entry->e_name,
-					     entry->e_name_len);
+					     entry->e_name_len, flags);
 	}
 	error = size;

Index: linux-2.5.60/fs/ext3/xattr.h

fs/ext3/xattr.h +6 -6
@@ -56,9 +56,9 @@
 struct ext3_xattr_handler {
 	char *prefix;
 	size_t (*list)(char *list, struct inode *inode, const char *name,
-		       int name_len);
+		       int name_len, int flags);
 	int (*get)(struct inode *inode, const char *name, void *buffer,
-		   size_t size);
+		   size_t size, int flags);
 	int (*set)(struct inode *inode, const char *name, const void *buffer,
 		   size_t size, int flags);
 };

@@ -67,12 +67,12 @@
 extern void ext3_xattr_unregister(int, struct ext3_xattr_handler *);
 
 extern int ext3_setxattr(struct dentry *, const char *, const void *, size_t, int);
-extern ssize_t ext3_getxattr(struct dentry *, const char *, void *, size_t);
-extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
-extern int ext3_removexattr(struct dentry *, const char *);
+extern ssize_t ext3_getxattr(struct dentry *, const char *, void *, size_t, int);
+extern ssize_t ext3_listxattr(struct dentry *, char *, size_t, int);
+extern int ext3_removexattr(struct dentry *, const char *, int);
 
 extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
-extern int ext3_xattr_list(struct inode *, char *, size_t);
+extern int ext3_xattr_list(struct inode *, char *, size_t, int flags);
 extern int ext3_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
 extern int ext3_xattr_set_handle(handle_t *, struct inode *, int, const char *, const void *, size_t, int);

Index: linux-2.5.60/fs/ext3/xattr_user.c

fs/ext3/xattr_user.c +2 -2
@@ -21,7 +21,7 @@
 
 static size_t
 ext3_xattr_user_list(char *list, struct inode *inode,
-		     const char *name, int name_len)
+		     const char *name, int name_len, int flags)
 {
 	const int prefix_len = sizeof(XATTR_USER_PREFIX)-1;
 

@@ -38,7 +38,7 @@
 
 static int
 ext3_xattr_user_get(struct inode *inode, const char *name,
-		    void *buffer, size_t size)
+		    void *buffer, size_t size, int flags)
 {
 	int error;

Index: linux-2.5.60/fs/xattr.c

fs/xattr.c +3 -3
@@ -160,7 +160,7 @@
 		if (error)
 			goto out;
 		down(&d->d_inode->i_sem);
-		error = d->d_inode->i_op->getxattr(d, kname, kvalue, size);
+		error = d->d_inode->i_op->getxattr(d, kname, kvalue, size, 0);
 		up(&d->d_inode->i_sem);
 	}
 

@@ -233,7 +233,7 @@
 		if (error)
 			goto out;
 		down(&d->d_inode->i_sem);
-		error = d->d_inode->i_op->listxattr(d, klist, size);
+		error = d->d_inode->i_op->listxattr(d, klist, size, 0);
 		up(&d->d_inode->i_sem);
 	}
 

@@ -308,7 +308,7 @@
 		if (error)
 			goto out;
 		down(&d->d_inode->i_sem);
-		error = d->d_inode->i_op->removexattr(d, kname);
+		error = d->d_inode->i_op->removexattr(d, kname, 0);
 		up(&d->d_inode->i_sem);
 	}
 out:

Index: linux-2.5.60/include/linux/fs.h

include/linux/fs.h +3 -3
@@ -743,9 +743,9 @@
 	int (*setattr) (struct dentry *, struct iattr *);
 	int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *);
 	int (*setxattr) (struct dentry *, const char *,const void *,size_t,int);
-	ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t);
-	ssize_t (*listxattr) (struct dentry *, char *, size_t);
-	int (*removexattr) (struct dentry *, const char *);
+	ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t,int);
+	ssize_t (*listxattr) (struct dentry *, char *, size_t, int);
+	int (*removexattr) (struct dentry *, const char *, int);
 };
 
 struct seq_file;

Index: linux-2.5.60/fs/jfs/xattr.c

fs/jfs/xattr.c +4 -3
@@ -962,12 +962,13 @@
 }
 
 ssize_t jfs_getxattr(struct dentry *dentry, const char *name, void *data,
-		     size_t buf_size)
+		     size_t buf_size, int flags)
 {
 	return __jfs_getxattr(dentry->d_inode, name, data, buf_size);
 }
 
-ssize_t jfs_listxattr(struct dentry * dentry, char *data, size_t buf_size)
+ssize_t jfs_listxattr(struct dentry * dentry, char *data, size_t buf_size,
+		      int flags)
 {
 	struct inode *inode = dentry->d_inode;
 	char *buffer;

@@ -1013,7 +1014,7 @@
 	return size;
 }
 
-int jfs_removexattr(struct dentry *dentry, const char *name)
+int jfs_removexattr(struct dentry *dentry, const char *name, int flags)
 {
 	return __jfs_setxattr(dentry->d_inode, name, 0, 0, XATTR_REPLACE);
 }

Index: linux-2.5.60/fs/jfs/jfs_xattr.h

fs/jfs/jfs_xattr.h +3 -3
@@ -57,8 +57,8 @@
 extern int jfs_setxattr(struct dentry *, const char *, const void *, size_t,
 			int);
 extern ssize_t __jfs_getxattr(struct inode *, const char *, void *, size_t);
-extern ssize_t jfs_getxattr(struct dentry *, const char *, void *, size_t);
-extern ssize_t jfs_listxattr(struct dentry *, char *, size_t);
-extern int jfs_removexattr(struct dentry *, const char *);
+extern ssize_t jfs_getxattr(struct dentry *, const char *, void *, size_t, int);
+extern ssize_t jfs_listxattr(struct dentry *, char *, size_t, int);
+extern int jfs_removexattr(struct dentry *, const char *, int);
 
 #endif	/* H_JFS_XATTR */

Index: linux-2.5.60/fs/xfs/linux/xfs_iops.c

fs/xfs/linux/xfs_iops.c +6 -3
@@ -640,7 +640,8 @@
 	struct dentry	*dentry,
 	const char	*name,
 	void		*data,
-	size_t		size)
+	size_t		size,
+	int		flags)
 {
 	ssize_t		error;
 	int		xflags = 0;

@@ -697,7 +698,8 @@
 linvfs_listxattr(
 	struct dentry		*dentry,
 	char			*data,
-	size_t			size)
+	size_t			size,
+	int			flags)
 {
 	ssize_t			error;
 	int			result = 0;

@@ -741,7 +743,8 @@
 STATIC int
 linvfs_removexattr(
 	struct dentry	*dentry,
-	const char	*name)
+	const char	*name,
+	int		flags)
 {
 	int		error;
 	int		xflags = 0;
#1 lore raw
Quoted 6 lines · Andreas Gruenbacher

Hi Andrew,

here are five patches against 2.5.60. Each file contains a brief description of what it does.

Minor point:

Quoted 15 lines

int
ext3_xattr_set(struct inode *inode, int name_index, const char *name,
const void *value, size_t value_len, int flags)

{
	handle_t *handle;
	int error;

	lock_kernel();
	handle = ext3_journal_start(inode, EXT3_XATTR_TRANS_BLOCKS);
	if (IS_ERR(handle))
		error = PTR_ERR(handle);
	else
		error = ext3_xattr_set_handle(handle, inode, name_index, name,
					      value, value_len, flags);
	ext3_journal_stop(handle, inode);

ext3_journal_stop() can return an error code - most notable -EIO if it was a synchronous transaction, or the filesystem has detected corruption.

Quoted 8 lines

The third to fifth are all steps towards trusted extended attributes, which are useful for privileged processes (mostly daemons). One use for this is Hierarchical Storage Management, in which a user space daemon stores online/offline information for files in trusted EA's, and the kernel communicates requests to bring files online to that daemon. This class of EA's will also find its way into XFS and ReiserFS, and expectedly also into JFS in this form. (Trusted EAs are included in the 2.4.19/2.4.20 patches as well.)

So is this new code actually functional yet? As in: something in-kernel using it?

If not, what is involved in completing the kernel side of trusted EA's?

author
#2 lore raw
Quoted 10 lines · Andrew Morton
Quoted 4 lines · Andreas Gruenbacher

Hi Andrew,

here are five patches against 2.5.60. Each file contains a brief description of what it does.

Minor point:
ext3_journal_stop() can return an error code - most notable -EIO if it was a synchronous transaction, or the filesystem has detected corruption.

Thanks, I have overlooked this third bug. An incremental patch on top of my previous kernel_lock_bug.diff is attached. (I have also uploaded the patches to <http://acl.bestbits.at/pre/v2.5/> in the meantime).

Quoted 10 lines
Quoted 9 lines

The third to fifth are all steps towards trusted extended attributes, which are useful for privileged processes (mostly daemons). One use for this is Hierarchical Storage Management, in which a user space daemon stores online/offline information for files in trusted EA's, and the kernel communicates requests to bring files online to that daemon. This class of EA's will also find its way into XFS and ReiserFS, and expectedly also into JFS in this form. (Trusted EAs are included in the 2.4.19/2.4.20 patches as well.)

Quoted 5 lines

So is this new code actually functional yet? As in: something in-kernel using it?

If not, what is involved in completing the kernel side of trusted EA's?

The important point for me now is to get the iops xattr-flags and xattr-flags-policy patches into 2.5 so that the API won't change during 2.6. The xattr-trusted patch only affects file systems locally, so it's far less critical.

The kernel side of trusted EAs is completely implemented with the patches I sent. In the future there will very likely be modules actually making use of the XATTR_KERNEL_CONTEXT flag, but Trusted EAs are quite useful from user space alone.

Cheers,
Andreas.

fs/ext3/xattr.c +3 -3
diff -u linux-2.5.60/fs/ext3/xattr.c linux-2.5.60/fs/ext3/xattr.c
@@ -848,7 +848,7 @@
 	       const void *value, size_t value_len, int flags)
 {
 	handle_t *handle;
-	int error;
+	int error, error2;
 
 	lock_kernel();
 	handle = ext3_journal_start(inode, EXT3_XATTR_TRANS_BLOCKS);

@@ -857,10 +857,10 @@
 	else
 		error = ext3_xattr_set_handle(handle, inode, name_index, name,
 					      value, value_len, flags);
-	ext3_journal_stop(handle, inode);
+	error2 = ext3_journal_stop(handle, inode);
 	unlock_kernel();
 
-	return error;
+	return error ? error : error2;
 }
 
 /*
#3 lore raw
In reply to Andreas Gruenbacher
Quoted 8 lines · Andreas Gruenbacher

The third to fifth are all steps towards trusted extended attributes, which are useful for privileged processes (mostly daemons). One use for this is Hierarchical Storage Management, in which a user space daemon stores online/offline information for files in trusted EA's, and the kernel communicates requests to bring files online to that daemon. This class of EA's will also find its way into XFS and ReiserFS, and expectedly also into JFS in this form. (Trusted EAs are included in the 2.4.19/2.4.20 patches as well.)

Please don't do the ugly flags stuff. We have fsuids and fsgids for exactly that reason (and because we're still lacking a credentials cache..).

author
#4 lore raw
Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Andreas Gruenbacher

The third to fifth are all steps towards trusted extended attributes, which are useful for privileged processes (mostly daemons). One use for this is Hierarchical Storage Management, in which a user space daemon stores online/offline information for files in trusted EA's, and the kernel communicates requests to bring files online to that daemon. This class of EA's will also find its way into XFS and ReiserFS, and expectedly also into JFS in this form. (Trusted EAs are included in the 2.4.19/2.4.20 patches as well.)

Quoted 3 lines

Please don't do the ugly flags stuff. We have fsuids and fsgids for exactly that reason (and because we're still lacking a credentials cache..).

The XATTR_KERNEL_CONTEXT flag cannot be substituted by a uid/gid change; it is unrelated to that; that's the whole point of it. It would be possible to raise some other flag (such as a capability, etc.) instead of passing an explicit flag, but that seems uglier and more problematic/error prone to me.

-- Andreas.

#5 lore raw
Quoted 9 lines · Andreas Gruenbacher
Quoted 3 lines

Please don't do the ugly flags stuff. We have fsuids and fsgids for exactly that reason (and because we're still lacking a credentials cache..).

The XATTR_KERNEL_CONTEXT flag cannot be substituted by a uid/gid change; it is unrelated to that; that's the whole point of it. It would be possible to raise some other flag (such as a capability, etc.) instead of passing an explicit flag, but that seems uglier and more problematic/error prone to me.

Then raise CAP_DAC_OVERRIDE. The right thing would be to pass down a struct cred, so we could pass down the magic sys_cred that allows all access (look at the XFS ACL code for details on that..), but unfortuantately we still don't have proper credentials although there were numerous patches around in the last years and we really want it for other reasons.

Magic flags that change the DAC checks work are ugly and will most certainly lead to bugs in some implementations sooner or later.

author
#6 lore raw
Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Andreas Gruenbacher
Quoted 3 lines

Please don't do the ugly flags stuff. We have fsuids and fsgids for exactly that reason (and because we're still lacking a credentials cache..).

The XATTR_KERNEL_CONTEXT flag cannot be substituted by a uid/gid change; it is unrelated to that; that's the whole point of it. It would be possible to raise some other flag (such as a capability, etc.) instead of passing an explicit flag, but that seems uglier and more problematic/error prone to me.

Quoted 6 lines

Then raise CAP_DAC_OVERRIDE. The right thing would be to pass down a struct cred, so we could pass down the magic sys_cred that allows all access (look at the XFS ACL code for details on that..), but unfortuantately we still don't have proper credentials although there were numerous patches around in the last years and we really want it for other reasons.

That sounds quite reasonable. I would have to raise CAP_SYS_ADMIN for trusted EA's, though. Do you see any potential side effects while a pretty powerful capability like CAP_SYS_ADMIN is temporarily raised?

Quoted 2 lines

Magic flags that change the DAC checks work are ugly and will most certainly lead to bugs in some implementations sooner or later.

Maybe.

Thanks,
Andreas.

#7 lore raw
Quoted 3 lines · Andreas Gruenbacher

That sounds quite reasonable. I would have to raise CAP_SYS_ADMIN for trusted EA's, though. Do you see any potential side effects while a pretty powerful capability like CAP_SYS_ADMIN is temporarily raised?

Okay, something I missed when looking over your patches, otherwise I'd have shutde earlier :) Do you really think you want CAP_SYS_ADMIN for trusted EAs? Soon we'll get CAP_SYS_ADMIN as catchall like old suser()..

Let me check what XFS uses for that purpose as soon as I'm back in the office.

author
#8 lore raw
Quoted 6 lines · Christoph Hellwig
Quoted 4 lines · Andreas Gruenbacher

That sounds quite reasonable. I would have to raise CAP_SYS_ADMIN for trusted EA's, though. Do you see any potential side effects while a pretty powerful capability like CAP_SYS_ADMIN is temporarily raised?

Quoted 7 lines

Okay, something I missed when looking over your patches, otherwise I'd have shutde earlier :) Do you really think you want CAP_SYS_ADMIN for trusted EAs? Soon we'll get CAP_SYS_ADMIN as catchall like old suser()..

Let me check what XFS uses for that purpose as soon as I'm back in the office.

The intention of Trusted Extended Attributes is for processes that perform tasks that are relevant for the proper functioning of the system, to allow them to use EAs. Other, non-privileged processes shall have no access whatsoever to those EAs. This level of protection would otherwise only be possible by providing a kernel module.

I would be quite happy with a new CAP_TRUSTED_PROCESS or whatever, but going that route for all sorts of applications then we might soon end up with an large number of capabilities. Maybe I'm wrong on that, though.

Cheers,
Andreas.