[PATCH] With size > XATTR_SIZE_MAX, getxattr(2) always returns E2BIG

From: Andreas Gruenbacher <agruen@suse.de>

The getxattr (listxattr) syscall returns E2BIG if the buffer passed to them
is bigger than XATTR_SIZE_MAX (XATTR_LIST_MAX), no matter what buffer size is
actually required.  Here is a fix.  It also removes the xattr_alloc and
xattr_free functions which are not of much use anymore.

diff --git a/fs/xattr.c b/fs/xattr.c
index 175d3e817619dbdbc434365d06dd47374a981d63..10adae3f722e53b0524438c664b6a4de440955b6 100644 (file)
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -15,40 +15,6 @@
 #include <linux/security.h>
 #include <asm/uaccess.h>
 
-/*
- * Extended attribute memory allocation wrappers, originally
- * based on the Intermezzo PRESTO_ALLOC/PRESTO_FREE macros.
- * Values larger than a page are uncommon - extended attributes
- * are supposed to be small chunks of metadata, and it is quite
- * unusual to have very many extended attributes, so lists tend
- * to be quite short as well.  The 64K upper limit is derived
- * from the extended attribute size limit used by XFS.
- * Intentionally allow zero @size for value/list size requests.
- */
-static void *
-xattr_alloc(size_t size, size_t limit)
-{
-       void *ptr;
-
-       if (size > limit)
-               return ERR_PTR(-E2BIG);
-
-       if (!size)      /* size request, no buffer is needed */
-               return NULL;
-
-       ptr = kmalloc((unsigned long) size, GFP_KERNEL);
-       if (!ptr)
-               return ERR_PTR(-ENOMEM);
-       return ptr;
-}
-
-static void
-xattr_free(void *ptr, size_t size)
-{
-       if (size)       /* for a size request, no buffer was needed */
-               kfree(ptr);
-}
-
 /*
  * Extended attribute SET operations
  */
@@ -57,7 +23,7 @@ setxattr(struct dentry *d, char __user *name, void __user *value,
         size_t size, int flags)
 {
        int error;
-       void *kvalue;
+       void *kvalue = NULL;
        char kname[XATTR_NAME_MAX + 1];
 
        if (flags & ~(XATTR_CREATE|XATTR_REPLACE))
@@ -69,13 +35,16 @@ setxattr(struct dentry *d, char __user *name, void __user *value,
        if (error < 0)
                return error;
 
-       kvalue = xattr_alloc(size, XATTR_SIZE_MAX);
-       if (IS_ERR(kvalue))
-               return PTR_ERR(kvalue);
-
-       if (size > 0 && copy_from_user(kvalue, value, size)) {
-               xattr_free(kvalue, size);
-               return -EFAULT;
+       if (size) {
+               if (size > XATTR_SIZE_MAX)
+                       return -E2BIG;
+               kvalue = kmalloc(size, GFP_KERNEL);
+               if (!kvalue)
+                       return -ENOMEM;
+               if (copy_from_user(kvalue, value, size)) {
+                       kfree(kvalue);
+                       return -EFAULT;
+               }
        }
 
        error = -EOPNOTSUPP;
@@ -90,7 +59,8 @@ setxattr(struct dentry *d, char __user *name, void __user *value,
 out:
                up(&d->d_inode->i_sem);
        }
-       xattr_free(kvalue, size);
+       if (kvalue)
+               kfree(kvalue);
        return error;
 }
 
@@ -146,7 +116,7 @@ static ssize_t
 getxattr(struct dentry *d, char __user *name, void __user *value, size_t size)
 {
        ssize_t error;
-       void *kvalue;
+       void *kvalue = NULL;
        char kname[XATTR_NAME_MAX + 1];
 
        error = strncpy_from_user(kname, name, sizeof(kname));
@@ -155,9 +125,13 @@ getxattr(struct dentry *d, char __user *name, void __user *value, size_t size)
        if (error < 0)
                return error;
 
-       kvalue = xattr_alloc(size, XATTR_SIZE_MAX);
-       if (IS_ERR(kvalue))
-               return PTR_ERR(kvalue);
+       if (size) {
+               if (size > XATTR_SIZE_MAX)
+                       size = XATTR_SIZE_MAX;
+               kvalue = kmalloc(size, GFP_KERNEL);
+               if (!kvalue)
+                       return -ENOMEM;
+       }
 
        error = -EOPNOTSUPP;
        if (d->d_inode->i_op && d->d_inode->i_op->getxattr) {
@@ -165,13 +139,18 @@ getxattr(struct dentry *d, char __user *name, void __user *value, size_t size)
                if (error)
                        goto out;
                error = d->d_inode->i_op->getxattr(d, kname, kvalue, size);
+               if (error > 0) {
+                       if (copy_to_user(value, kvalue, error))
+                               error = -EFAULT;
+               } else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
+                       /* The file system tried to returned a value bigger
+                          than XATTR_SIZE_MAX bytes. Not possible. */
+                       error = -E2BIG;
+               }
        }
-
-       if (kvalue && error > 0)
-               if (copy_to_user(value, kvalue, error))
-                       error = -EFAULT;
 out:
-       xattr_free(kvalue, size);
+       if (kvalue)
+               kfree(kvalue);
        return error;
 }
 
@@ -226,11 +205,15 @@ static ssize_t
 listxattr(struct dentry *d, char __user *list, size_t size)
 {
        ssize_t error;
-       char *klist;
-
-       klist = (char *)xattr_alloc(size, XATTR_LIST_MAX);
-       if (IS_ERR(klist))
-               return PTR_ERR(klist);
+       char *klist = NULL;
+
+       if (size) {
+               if (size > XATTR_LIST_MAX)
+                       size = XATTR_LIST_MAX;
+               klist = kmalloc(size, GFP_KERNEL);
+               if (!klist)
+                       return -ENOMEM;
+       }
 
        error = -EOPNOTSUPP;
        if (d->d_inode->i_op && d->d_inode->i_op->listxattr) {
@@ -238,13 +221,18 @@ listxattr(struct dentry *d, char __user *list, size_t size)
                if (error)
                        goto out;
                error = d->d_inode->i_op->listxattr(d, klist, size);
+               if (error > 0) {
+                       if (copy_to_user(list, klist, error))
+                               error = -EFAULT;
+               } else if (error == -ERANGE && size >= XATTR_LIST_MAX) {
+                       /* The file system tried to returned a list bigger
+                          than XATTR_LIST_MAX bytes. Not possible. */
+                       error = -E2BIG;
+               }
        }
-
-       if (klist && error > 0)
-               if (copy_to_user(list, klist, error))
-                       error = -EFAULT;
 out:
-       xattr_free(klist, size);
+       if (klist)
+               kfree(klist);
        return error;
 }