linux-kernel - [PATCH] 9p: Don't use ATTR_* values from fs.h in userspace facing structs

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-next>] [day] [month] [year] [list]

Date:	Sat, 17 Dec 2011 17:07:02 +0200
From:	Sasha Levin <levinsasha928@...il.com>
To:	ericvh@...il.com, rminnich@...dia.gov, lucho@...kov.net,
	davem@...emloft.net, aneesh.kumar@...ux.vnet.ibm.com,
	jvrao@...ux.vnet.ibm.com, viro@...IV.linux.org.uk
Cc:	v9fs-developer@...ts.sourceforge.net, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, Sasha Levin <levinsasha928@...il.com>
Subject: [PATCH] 9p: Don't use ATTR_* values from fs.h in userspace facing  structs

struct p9_iattr_dotl is userspace facing, but the 'valid' field is documented
as follows:

	 * @valid: bitfield specifying which fields are valid
	 *         same as in struct iattr

Which means that the user has to know about kernel internal ATTR_* values.

On Fri, 2011-12-16 at 23:30 +0000, Al Viro wrote:
> They *are* kernel internal values and 9P is asking for trouble exposing
> them.  Translation: tomorrow we might reassign those as we bloody wish
> and any userland code that happens to rely on their values will break.
> At which point we'll handle complaints by pointing and laughing.
>
> It's a 9P bug; fix it there.  Turning random internal constants into a part
> of ABI is not going to work.

Signed-off-by: Sasha Levin <levinsasha928@...il.com>
---
 fs/9p/vfs_inode_dotl.c |   31 ++++++++++++++++++++++++++++++-
 include/net/9p/9p.h    |   18 ++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 0b5745e..a948214 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -523,6 +523,35 @@ v9fs_vfs_getattr_dotl(struct vfsmount *mnt, struct dentry *dentry,
 	return 0;
 }
 
+int v9fs_vfs_iattr_to_9p_valid(u32 ia_valid)
+{
+	u32 valid = 0, i;
+	static u32 attr_map[][2] = {
+		{ATTR_MODE,		P9_ATTR_MODE},
+		{ATTR_UID,		P9_ATTR_UID},
+		{ATTR_SIZE,		P9_ATTR_SIZE},
+		{ATTR_ATIME,		P9_ATTR_ATIME},
+		{ATTR_MTIME,		P9_ATTR_MTIME},
+		{ATTR_CTIME,		P9_ATTR_CTIME},
+		{ATTR_ATIME_SET,	P9_ATTR_ATIME_SET},
+		{ATTR_MTIME_SET,	P9_ATTR_MTIME_SET},
+		{ATTR_FORCE,		P9_ATTR_FORCE},
+		{ATTR_ATTR_FLAG,	P9_ATTR_ATTR_FLAG},
+		{ATTR_KILL_SUID,	P9_ATTR_KILL_SUID},
+		{ATTR_KILL_SGID,	P9_ATTR_KILL_SGID},
+		{ATTR_FILE,		P9_ATTR_FILE},
+		{ATTR_KILL_PRIV,	P9_ATTR_KILL_PRIV},
+		{ATTR_OPEN,		P9_ATTR_OPEN},
+		{ATTR_TIMES_SET,	P9_ATTR_TIMES_SET},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(attr_map); i++)
+		if (ia_valid & attr_map[i][0])
+			valid |= attr_map[i][1];
+
+	return valid;
+}
+
 /**
  * v9fs_vfs_setattr_dotl - set file metadata
  * @dentry: file whose metadata to set
@@ -543,7 +572,7 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
 	if (retval)
 		return retval;
 
-	p9attr.valid = iattr->ia_valid;
+	p9attr.valid = v9fs_vfs_iattr_to_9p_valid(iattr->ia_valid);
 	p9attr.mode = iattr->ia_mode;
 	p9attr.uid = iattr->ia_uid;
 	p9attr.gid = iattr->ia_gid;
diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
index 2d70b95..98b3f71 100644
--- a/include/net/9p/9p.h
+++ b/include/net/9p/9p.h
@@ -468,6 +468,24 @@ struct p9_stat_dotl {
 #define P9_STATS_BASIC		0x000007ffULL /* Mask for fields up to BLOCKS */
 #define P9_STATS_ALL		0x00003fffULL /* Mask for All fields above */
 
+#define P9_ATTR_MODE		(1 << 0)
+#define P9_ATTR_UID		(1 << 1)
+#define P9_ATTR_GID		(1 << 2)
+#define P9_ATTR_SIZE		(1 << 3)
+#define P9_ATTR_ATIME		(1 << 4)
+#define P9_ATTR_MTIME		(1 << 5)
+#define P9_ATTR_CTIME		(1 << 6)
+#define P9_ATTR_ATIME_SET	(1 << 7)
+#define P9_ATTR_MTIME_SET	(1 << 8)
+#define P9_ATTR_FORCE		(1 << 9) /* Not a change, but a change it */
+#define P9_ATTR_ATTR_FLAG	(1 << 10)
+#define P9_ATTR_KILL_SUID	(1 << 11)
+#define P9_ATTR_KILL_SGID	(1 << 12)
+#define P9_ATTR_FILE		(1 << 13)
+#define P9_ATTR_KILL_PRIV	(1 << 14)
+#define P9_ATTR_OPEN		(1 << 15) /* Truncating from open(O_TRUNC) */
+#define P9_ATTR_TIMES_SET	(1 << 16)
+
 /**
  * struct p9_iattr_dotl - P9 inode attribute for setattr
  * @valid: bitfield specifying which fields are valid
-- 
1.7.8

--
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/