[PATCH] xattr: re-introduce validity check before xattr cache insert

* ext[23]_xattr_list():

  - Before inserting an xattr block into the cache, make sure that the
    block is not corrupted.  The check got moved after inserting into the
    cache in the xattr consolidation patches, so corrupted blocks could become
    visible to cache users.

  - Take a variable out of the loop that calls the ->list handlers.

* A few cosmetic changes.
Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

fs/ext2/acl.c

@@ -400,7 +400,7 @@ ext2_xattr_list_acl_access(struct inode *inode, char *list, size_t list_size,
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
-	if (list && (size <= list_size))
+	if (list && size <= list_size)
 		memcpy(list, XATTR_NAME_ACL_ACCESS, size);
 	return size;
 }
@@ -413,7 +413,7 @@ ext2_xattr_list_acl_default(struct inode *inode, char *list, size_t list_size,
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
-	if (list && (size <= list_size))
+	if (list && size <= list_size)
 		memcpy(list, XATTR_NAME_ACL_DEFAULT, size);
 	return size;
 }

fs/ext2/xattr.c

@@ -270,8 +270,8 @@ ext2_xattr_list(struct inode *inode, char *buffer, size_t buffer_size)
 {
 	struct buffer_head *bh = NULL;
 	struct ext2_xattr_entry *entry;
-	size_t total_size = 0;
-	char *buf, *end;
+	char *end;
+	size_t rest = buffer_size;
 	int error;
 
 	ea_idebug(inode, "buffer=%p, buffer_size=%ld",
@@ -298,36 +298,39 @@ bad_block:	ext2_error(inode->i_sb, "ext2_xattr_list",
 		goto cleanup;
 	}
 
+	/* check the on-disk data structure */
+	entry = FIRST_ENTRY(bh);
+	while (!IS_LAST_ENTRY(entry)) {
+		struct ext2_xattr_entry *next = EXT2_XATTR_NEXT(entry);
+
+		if ((char *)next >= end)
+			goto bad_block;
+		entry = next;
+	}
 	if (ext2_xattr_cache_insert(bh))
 		ea_idebug(inode, "cache insert failed");
 
 	/* list the attribute names */
-	buf = buffer;
 	for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry);
 	     entry = EXT2_XATTR_NEXT(entry)) {
-		struct xattr_handler *handler;
-		struct ext2_xattr_entry *next = EXT2_XATTR_NEXT(entry);
-		
-		if ((char *)next >= end)
-			goto bad_block;
+		struct xattr_handler *handler =
+			ext2_xattr_handler(entry->e_name_index);
 
-		handler = ext2_xattr_handler(entry->e_name_index);
 		if (handler) {
-			size_t size = handler->list(inode, buf, buffer_size,
+			size_t size = handler->list(inode, buffer, rest,
 						    entry->e_name,
 						    entry->e_name_len);
-			if (buf) {
-				if (size > buffer_size) {
+			if (buffer) {
+				if (size > rest) {
 					error = -ERANGE;
 					goto cleanup;
 				}
-				buf += size;
-				buffer_size -= size;
+				buffer += size;
 			}
-			total_size += size;
+			rest -= size;
 		}
 	}
-	error = total_size;
+	error = buffer_size - rest;  /* total size */
 
 cleanup:
 	brelse(bh);

fs/ext2/xattr_security.c

@@ -17,7 +17,7 @@ ext2_xattr_security_list(struct inode *inode, char *list, size_t list_size,
 	const int prefix_len = sizeof(XATTR_SECURITY_PREFIX)-1;
 	const size_t total_len = prefix_len + name_len + 1;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_SECURITY_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';

fs/ext2/xattr_trusted.c

@@ -24,7 +24,7 @@ ext2_xattr_trusted_list(struct inode *inode, char *list, size_t list_size,
 	if (!capable(CAP_SYS_ADMIN))
 		return 0;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_TRUSTED_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';

fs/ext2/xattr_user.c

@@ -23,7 +23,7 @@ ext2_xattr_user_list(struct inode *inode, char *list, size_t list_size,
 	if (!test_opt(inode->i_sb, XATTR_USER))
 		return 0;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_USER_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';

fs/ext3/acl.c

@@ -423,7 +423,7 @@ ext3_xattr_list_acl_access(struct inode *inode, char *list, size_t list_len,
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
-	if (list && (size <= list_len))
+	if (list && size <= list_len)
 		memcpy(list, XATTR_NAME_ACL_ACCESS, size);
 	return size;
 }
@@ -436,7 +436,7 @@ ext3_xattr_list_acl_default(struct inode *inode, char *list, size_t list_len,
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
-	if (list && (size <= list_len))
+	if (list && size <= list_len)
 		memcpy(list, XATTR_NAME_ACL_DEFAULT, size);
 	return size;
 }

fs/ext3/xattr.c

@@ -267,8 +267,8 @@ ext3_xattr_list(struct inode *inode, char *buffer, size_t buffer_size)
 {
 	struct buffer_head *bh = NULL;
 	struct ext3_xattr_entry *entry;
-	size_t total_size = 0;
-	char *buf, *end;
+	char *end;
+	size_t rest = buffer_size;
 	int error;
 
 	ea_idebug(inode, "buffer=%p, buffer_size=%ld",
@@ -295,36 +295,39 @@ bad_block:	ext3_error(inode->i_sb, "ext3_xattr_list",
 		goto cleanup;
 	}
 
+	/* check the on-disk data structure */
+	entry = FIRST_ENTRY(bh);
+	while (!IS_LAST_ENTRY(entry)) {
+		struct ext3_xattr_entry *next = EXT3_XATTR_NEXT(entry);
+
+		if ((char *)next >= end)
+			goto bad_block;
+		entry = next;
+	}
 	if (ext3_xattr_cache_insert(bh))
 		ea_idebug(inode, "cache insert failed");
 
 	/* list the attribute names */
-	buf = buffer;
 	for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry);
 	     entry = EXT3_XATTR_NEXT(entry)) {
-		struct xattr_handler *handler;
-		struct ext3_xattr_entry *next = EXT3_XATTR_NEXT(entry);
-
-		if ((char *)next >= end)
-			goto bad_block;
+		struct xattr_handler *handler =
+			ext3_xattr_handler(entry->e_name_index);
 
-		handler = ext3_xattr_handler(entry->e_name_index);
 		if (handler) {
-			size_t size = handler->list(inode, buf, buffer_size,
+			size_t size = handler->list(inode, buffer, rest,
 						    entry->e_name,
 						    entry->e_name_len);
-			if (buf) {
-				if (size > buffer_size) {
+			if (buffer) {
+				if (size > rest) {
 					error = -ERANGE;
 					goto cleanup;
 				}
-				buf += size;
-				buffer_size -= size;
+				buffer += size;
 			}
-			total_size += size;
+			rest -= size;
 		}
 	}
-	error = total_size;
+	error = buffer_size - rest;  /* total size */
 
 cleanup:
 	brelse(bh);

fs/ext3/xattr_security.c

@@ -19,7 +19,7 @@ ext3_xattr_security_list(struct inode *inode, char *list, size_t list_size,
 	const size_t total_len = prefix_len + name_len + 1;
 
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_SECURITY_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';

fs/ext3/xattr_trusted.c

@@ -25,7 +25,7 @@ ext3_xattr_trusted_list(struct inode *inode, char *list, size_t list_size,
 	if (!capable(CAP_SYS_ADMIN))
 		return 0;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_TRUSTED_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';

fs/ext3/xattr_user.c

@@ -25,7 +25,7 @@ ext3_xattr_user_list(struct inode *inode, char *list, size_t list_size,
 	if (!test_opt(inode->i_sb, XATTR_USER))
 		return 0;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_USER_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';

fs/xattr.c

@@ -352,7 +352,8 @@ sys_fremovexattr(int fd, char __user *name)
 }
 
 
-static const char *strcmp_prefix(const char *a, const char *a_prefix)
+static const char *
+strcmp_prefix(const char *a, const char *a_prefix)
 {
 	while (*a_prefix && *a == *a_prefix) {
 		a++;