linux-kernel - [PATCH] sysfs: sysfs_setattr set iattrs unconditionally

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m1d40llaln.fsf_-_@fess.ebiederm.org>
Date:	Wed, 03 Feb 2010 23:32:36 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Greg KH <gregkh@...e.de>
Cc:	Jean Delvare <khali@...ux-fr.org>,
	LKML <linux-kernel@...r.kernel.org>, Tejun Heo <tj@...nel.org>,
	Serge Hallyn <serue@...ibm.com>, stable@...nel.org
Subject: [PATCH] sysfs: sysfs_setattr set iattrs unconditionally


There is currently a bug in sysfs_setattr where the first time
we set the attributes on a sysfs file we allocate backing store
but do not set the backing store attributes.  Resulting in overly
restrictive permissions on sysfs files.

This bug is masked by the fact we that we do update the vfs inode
so it will only show up after the vfs inode has been evicted
causing surprise sysfs attribute changes.

The fix is to simply modify the code so that it always executes
when we update the sysfs attributes, as we did in 2.6.31 and earlier.

This is a backported version of my similiar change for 2.6.33-rc6
against sysfs_sd_setattr, for the stable 2.6.32 tree.

Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>
---
 fs/sysfs/inode.c |   47 +++++++++++++++++++++++------------------------
 1 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index e28cecf..02a022a 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -94,30 +94,29 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
 		if (!sd_attrs)
 			return -ENOMEM;
 		sd->s_iattr = sd_attrs;
-	} else {
-		/* attributes were changed at least once in past */
-		iattrs = &sd_attrs->ia_iattr;
-
-		if (ia_valid & ATTR_UID)
-			iattrs->ia_uid = iattr->ia_uid;
-		if (ia_valid & ATTR_GID)
-			iattrs->ia_gid = iattr->ia_gid;
-		if (ia_valid & ATTR_ATIME)
-			iattrs->ia_atime = timespec_trunc(iattr->ia_atime,
-					inode->i_sb->s_time_gran);
-		if (ia_valid & ATTR_MTIME)
-			iattrs->ia_mtime = timespec_trunc(iattr->ia_mtime,
-					inode->i_sb->s_time_gran);
-		if (ia_valid & ATTR_CTIME)
-			iattrs->ia_ctime = timespec_trunc(iattr->ia_ctime,
-					inode->i_sb->s_time_gran);
-		if (ia_valid & ATTR_MODE) {
-			umode_t mode = iattr->ia_mode;
-
-			if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
-				mode &= ~S_ISGID;
-			iattrs->ia_mode = sd->s_mode = mode;
-		}
+	}
+	/* attributes were changed at least once in past */
+	iattrs = &sd_attrs->ia_iattr;
+
+	if (ia_valid & ATTR_UID)
+		iattrs->ia_uid = iattr->ia_uid;
+	if (ia_valid & ATTR_GID)
+		iattrs->ia_gid = iattr->ia_gid;
+	if (ia_valid & ATTR_ATIME)
+		iattrs->ia_atime = timespec_trunc(iattr->ia_atime,
+			inode->i_sb->s_time_gran);
+	if (ia_valid & ATTR_MTIME)
+		iattrs->ia_mtime = timespec_trunc(iattr->ia_mtime,
+			inode->i_sb->s_time_gran);
+	if (ia_valid & ATTR_CTIME)
+		iattrs->ia_ctime = timespec_trunc(iattr->ia_ctime,
+			inode->i_sb->s_time_gran);
+	if (ia_valid & ATTR_MODE) {
+		umode_t mode = iattr->ia_mode;
+
+		if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
+			mode &= ~S_ISGID;
+		iattrs->ia_mode = sd->s_mode = mode;
 	}
 	return error;
 }
-- 
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/