Patchwork [3.5.y.z,extended,stable] Patch "reiserfs: fix problems with chowning setuid file w/ xattrs" has been added to staging queue

login
register
mail settings
Submitter Luis Henriques
Date June 3, 2013, 1:35 p.m.
Message ID <1370266557-2425-1-git-send-email-luis.henriques@canonical.com>
Download mbox | patch
Permalink /patch/248272/
State New
Headers show

Comments

Luis Henriques - June 3, 2013, 1:35 p.m.
This is a note to let you know that I have just added a patch titled

    reiserfs: fix problems with chowning setuid file w/ xattrs

to the linux-3.5.y-queue branch of the 3.5.y.z extended stable tree 
which can be found at:

 http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.5.y-queue

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.5.y.z tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Luis

------

From 35b037ff074a8463d405cae4adc44024f0cfae3e Mon Sep 17 00:00:00 2001
From: Jeff Mahoney <jeffm@suse.com>
Date: Fri, 31 May 2013 15:54:17 -0400
Subject: [PATCH] reiserfs: fix problems with chowning setuid file w/ xattrs

commit 4a8570112b76a63ad21cfcbe2783f98f7fd5ba1b upstream.

reiserfs_chown_xattrs() takes the iattr struct passed into ->setattr
and uses it to iterate over all the attrs associated with a file to change
ownership of xattrs (and transfer quota associated with the xattr files).

When the setuid bit is cleared during chown, ATTR_MODE and iattr->ia_mode
are passed to all the xattrs as well. This means that the xattr directory
will have S_IFREG added to its mode bits.

This has been prevented in practice by a missing IS_PRIVATE check
in reiserfs_acl_chmod, which caused a double-lock to occur while holding
the write lock. Since the file system was completely locked up, the
writeout of the corrupted mode never happened.

This patch temporarily clears everything but ATTR_UID|ATTR_GID for the
calls to reiserfs_setattr and adds the missing IS_PRIVATE check.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 fs/reiserfs/xattr.c     | 14 +++++++++++++-
 fs/reiserfs/xattr_acl.c |  3 +++
 2 files changed, 16 insertions(+), 1 deletion(-)

--
1.8.1.2

Patch

diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 048d990..cff0888 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -318,7 +318,19 @@  static int delete_one_xattr(struct dentry *dentry, void *data)
 static int chown_one_xattr(struct dentry *dentry, void *data)
 {
 	struct iattr *attrs = data;
-	return reiserfs_setattr(dentry, attrs);
+	int ia_valid = attrs->ia_valid;
+	int err;
+
+	/*
+	 * We only want the ownership bits. Otherwise, we'll do
+	 * things like change a directory to a regular file if
+	 * ATTR_MODE is set.
+	 */
+	attrs->ia_valid &= (ATTR_UID|ATTR_GID);
+	err = reiserfs_setattr(dentry, attrs);
+	attrs->ia_valid = ia_valid;
+
+	return err;
 }

 /* No i_mutex, but the inode is unconnected. */
diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c
index 44474f9..84fe30c 100644
--- a/fs/reiserfs/xattr_acl.c
+++ b/fs/reiserfs/xattr_acl.c
@@ -429,6 +429,9 @@  int reiserfs_acl_chmod(struct inode *inode)
 	int depth;
 	int error;

+	if (IS_PRIVATE(inode))
+		return 0;
+
 	if (S_ISLNK(inode->i_mode))
 		return -EOPNOTSUPP;