Unionfs: xattr copyup fixes

Rewrote xattr copyup code more cleanly; documented it better; eliminate one
possible leak in error path; and ignore another impossible copyup-time error
which caused fanout invariant violations under memory-pressure conditions.

Don't use vmalloc when allocating xattr buffers, as the VFS no longer does
so (just use kmalloc).  Eliminate unionfs_xattr_free which is now just plain
kfree.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index eb5d5a0d50203a576c22ea17d352dfa1318f6eea..fb7a2de0ad099841cb69af627e14eb6a26399b2a 100644 (file)
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -34,25 +34,37 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,
        char *attr_value = NULL;
        char *name_list_orig = NULL;
 
+       /* query the actual size of the xattr list */
        list_size = vfs_listxattr(old_lower_dentry, NULL, 0);
-
        if (list_size <= 0) {
                err = list_size;
                goto out;
        }
 
+       /* allocate space for the actual list */
        name_list = unionfs_xattr_alloc(list_size + 1, XATTR_LIST_MAX);
        if (!name_list || IS_ERR(name_list)) {
                err = PTR_ERR(name_list);
                goto out;
        }
+
+       name_list_orig = name_list; /* save for kfree at end */
+
+       /* now get the actual xattr list of the source file */
        list_size = vfs_listxattr(old_lower_dentry, name_list, list_size);
+       if (list_size <= 0) {
+               err = list_size;
+               goto out;
+       }
+
+       /* allocate space to hold each xattr's value */
        attr_value = unionfs_xattr_alloc(XATTR_SIZE_MAX, XATTR_SIZE_MAX);
        if (!attr_value || IS_ERR(attr_value)) {
                err = PTR_ERR(name_list);
                goto out;
        }
-       name_list_orig = name_list;
+
+       /* in a loop, get and set each xattr from src to dst file */
        while (*name_list) {
                ssize_t size;
 
@@ -65,7 +77,6 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,
                        err = size;
                        goto out;
                }
-
                if (size > XATTR_SIZE_MAX) {
                        err = -E2BIG;
                        goto out;
@@ -73,20 +84,21 @@ static int copyup_xattrs(struct dentry *old_lower_dentry,
                /* Don't lock here since vfs_setxattr does it for us. */
                err = vfs_setxattr(new_lower_dentry, name_list, attr_value,
                                   size, 0);
-
                if (err < 0)
                        goto out;
                name_list += strlen(name_list) + 1;
        }
 out:
-       name_list = name_list_orig;
-
-       if (name_list)
-               unionfs_xattr_free(name_list, list_size + 1);
+       if (name_list_orig)
+               kfree(name_list_orig);
        if (attr_value)
-               unionfs_xattr_free(attr_value, XATTR_SIZE_MAX);
-       /* It is no big deal if this fails, we just roll with the punches. */
-       if (err == -ENOTSUPP || err == -EOPNOTSUPP)
+               kfree(attr_value);
+       /*
+        * Ignore if xattr isn't supported.  Also ignore EPERM because that
+        * requires CAP_SYS_ADMIN for security.* xattrs, but copyup happens
+        * as normal users.
+        */
+       if (err == -ENOTSUPP || err == -EOPNOTSUPP || err == -EPERM)
                err = 0;
        return err;
 }

diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index b4743939c89d41e6735c1c64d5c909d73ff51f32..ba0ff504cf0139333acdf28ec4b59c248b5bed4c 100644 (file)
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -342,7 +342,6 @@ extern struct dentry *unionfs_interpose(struct dentry *this_dentry,
 #ifdef CONFIG_UNION_FS_XATTR
 /* Extended attribute functions. */
 extern void *unionfs_xattr_alloc(size_t size, size_t limit);
-extern void unionfs_xattr_free(void *ptr, size_t size);
 
 extern ssize_t unionfs_getxattr(struct dentry *dentry, const char *name,
                                void *value, size_t size);

diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index d6f4d5366047bc8baee037f6f54b495d3ca3ecc0..ee7da13231d50656742012f8c0db7baa69ae5c00 100644 (file)
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -28,25 +28,13 @@ void *unionfs_xattr_alloc(size_t size, size_t limit)
 
        if (!size)              /* size request, no buffer is needed */
                return NULL;
-       else if (size <= PAGE_SIZE)
-               ptr = kmalloc(size, GFP_KERNEL);
-       else
-               ptr = vmalloc(size);
+
+       ptr = kmalloc(size, GFP_KERNEL);
        if (!ptr)
                return ERR_PTR(-ENOMEM);
        return ptr;
 }
 
-void unionfs_xattr_free(void *ptr, size_t size)
-{
-       if (!size)              /* size request, no buffer was needed */
-               return;
-       else if (size <= PAGE_SIZE)
-               kfree(ptr);
-       else
-               vfree(ptr);
-}
-
 /*
  * BKL held by caller.
  * dentry->d_inode->i_mutex locked