[PATCH] Fix listxattr() for generic security attributes

[PATCH] Fix listxattr() for generic security attributes

[Daniel Drake]

[-- Attachment #1: Type: text/plain, Size: 949 bytes --]

Commit f549d6c18c0e8e6cf1bf0e7a47acc1daf7e2cec1 introduced a generic fallback 
for security xattrs, but appears to include a subtle bug.

Gentoo users with kernels with selinux compiled in, and coreutils compiled 
with acl support, noticed that they could not copy files on tmpfs using 'cp'.

cp (compiled with acl support) copies the file, lists the extended attributes 
on the old file, copies them all to the new file, and then exits. However the 
listxattr() calls were failing with this odd behaviour:

llistxattr("a.out", (nil), 0)           = 17
llistxattr("a.out", 0x7fffff8c6cb0, 17) = -1 ERANGE (Numerical result out of 
range)

I believe this is a simple problem in the logic used to check the buffer 
sizes; if the user sends a buffer the exact size of the data, then its ok :)

This patch solves the problem. Please apply for 2.6.15.
More info can be found at http://bugs.gentoo.org/113138

Signed-off-by: Daniel Drake <dsd@gentoo.org>

[-- Attachment #2: xattr-security-bounds-check.patch --]
[-- Type: text/x-patch, Size: 415 bytes --]

--- linux/fs/xattr.c.orig	2005-12-08 11:48:31.000000000 +0000
+++ linux/fs/xattr.c	2005-12-08 11:48:50.000000000 +0000
@@ -243,7 +243,7 @@ listxattr(struct dentry *d, char __user 
 		error = d->d_inode->i_op->listxattr(d, klist, size);
 	} else {
 		error = security_inode_listsecurity(d->d_inode, klist, size);
-		if (size && error >= size)
+		if (size && error > size)
 			error = -ERANGE;
 	}
 	if (error > 0) {

Re: [PATCH] Fix listxattr() for generic security attributes

[James Morris]

On Thu, 8 Dec 2005, Daniel Drake wrote:

> Commit f549d6c18c0e8e6cf1bf0e7a47acc1daf7e2cec1 introduced a generic fallback
> for security xattrs, but appears to include a subtle bug.
> 
> Gentoo users with kernels with selinux compiled in, and coreutils compiled
> with acl support, noticed that they could not copy files on tmpfs using 'cp'.
> 
> cp (compiled with acl support) copies the file, lists the extended attributes
> on the old file, copies them all to the new file, and then exits. However the
> listxattr() calls were failing with this odd behaviour:
> 
> llistxattr("a.out", (nil), 0)           = 17
> llistxattr("a.out", 0x7fffff8c6cb0, 17) = -1 ERANGE (Numerical result out of
> range)
> 
> I believe this is a simple problem in the logic used to check the buffer
> sizes; if the user sends a buffer the exact size of the data, then its ok :)
> 
> This patch solves the problem. Please apply for 2.6.15.
> More info can be found at http://bugs.gentoo.org/113138
> 
> Signed-off-by: Daniel Drake <dsd@gentoo.org>

Acked-by: James Morris <jmorris@namei.org>

-- 
James Morris
<jmorris@namei.org>

Re: [PATCH] Fix listxattr() for generic security attributes

[Stephen Smalley]

On Thu, 2005-12-08 at 16:13 +0000, Daniel Drake wrote:
> Commit f549d6c18c0e8e6cf1bf0e7a47acc1daf7e2cec1 introduced a generic fallback 
> for security xattrs, but appears to include a subtle bug.
> 
> Gentoo users with kernels with selinux compiled in, and coreutils compiled 
> with acl support, noticed that they could not copy files on tmpfs using 'cp'.
> 
> cp (compiled with acl support) copies the file, lists the extended attributes 
> on the old file, copies them all to the new file, and then exits. However the 
> listxattr() calls were failing with this odd behaviour:
> 
> llistxattr("a.out", (nil), 0)           = 17
> llistxattr("a.out", 0x7fffff8c6cb0, 17) = -1 ERANGE (Numerical result out of 
> range)
> 
> I believe this is a simple problem in the logic used to check the buffer 
> sizes; if the user sends a buffer the exact size of the data, then its ok :)
> 
> This patch solves the problem. Please apply for 2.6.15.
> More info can be found at http://bugs.gentoo.org/113138
> 
> Signed-off-by: Daniel Drake <dsd@gentoo.org>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

-- 
Stephen Smalley
National Security Agency