[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>

diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c
index e91766a44d8ecd95a01526acec6ebf5064a3ff70..fb716b3f5ee0888538710372a4965f3d3fd55afb 100644 (file)
--- a/fs/ext2/acl.c
+++ b/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;
 }

diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index a0d8fe369e878d923a5c10deadccd8040986412f..fffddd16b064373bfffc8e3f927fd983c1e987ce 100644 (file)
--- a/fs/ext2/xattr.c
+++ b/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);

diff --git a/fs/ext2/xattr_security.c b/fs/ext2/xattr_security.c
index c342cdff28b008318891c8ec3388aa2a8d23383d..6a6c59fbe59951d902208ef781c7a963f4617f64 100644 (file)
--- a/fs/ext2/xattr_security.c
+++ b/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';

diff --git a/fs/ext2/xattr_trusted.c b/fs/ext2/xattr_trusted.c
index 5ab0eb60fe905d1619930c65a12d4a4c5fd7516c..52b30ee6a25f31003fe3ad96c5ab087e3f27805d 100644 (file)
--- a/fs/ext2/xattr_trusted.c
+++ b/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';

diff --git a/fs/ext2/xattr_user.c b/fs/ext2/xattr_user.c
index 68a73d7439b50819fa8a0aad179751f57908b5a4..0c03ea131a948da241849d9bb82953f4b4e9cabd 100644 (file)
--- a/fs/ext2/xattr_user.c
+++ b/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';

diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c
index 99dc6a19a5adef81633dda3bae5c63b984ec0018..4a18653e831d903a945b5ea6ded0ac2b890d1d75 100644 (file)
--- a/fs/ext3/acl.c
+++ b/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;
 }

diff --git a/fs/ext3/xattr.c b/fs/ext3/xattr.c
index facea69eb7fb38c4bc07973b823b5f036617b6e6..b06cd4de68305c072368a1cdc5672ed7d949d68d 100644 (file)
--- a/fs/ext3/xattr.c
+++ b/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);

diff --git a/fs/ext3/xattr_security.c b/fs/ext3/xattr_security.c
index cf8cabf932fc8db0546b85f5a3cf28f53186665a..ddc1c41750e1403ce32c202129bb46f4bc813f06 100644 (file)
--- a/fs/ext3/xattr_security.c
+++ b/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';

diff --git a/fs/ext3/xattr_trusted.c b/fs/ext3/xattr_trusted.c
index 7e30c4e4c39c073fc6c5eca18316ef781fe4ba0f..f68bfd1cf519f6f55d24fcd5728e2d442ba18440 100644 (file)
--- a/fs/ext3/xattr_trusted.c
+++ b/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';

diff --git a/fs/ext3/xattr_user.c b/fs/ext3/xattr_user.c
index 9c06a348b7967c0786cf78aa94fd3496ada89228..e907cae7a07c34617983bf808125e1a526235801 100644 (file)
--- a/fs/ext3/xattr_user.c
+++ b/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';

diff --git a/fs/xattr.c b/fs/xattr.c
index dd803101121227dd038c0c47e6b21198434402dd..93dee70a1dbe0ef404b389dc6147a8f817b0b300 100644 (file)
--- a/fs/xattr.c
+++ b/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++;