[PATCH] reiserfs: Fix permissions on .reiserfs_priv

From: Jeff Mahoney
Date: Wed Apr 07 2010 - 19:18:58 EST


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Commit 677c9b2e393a0cd203bd54e9c18b012b2c73305a removed the magic
from the lookup code to hide the .reiserfs_priv directory since it
was getting loaded at mount-time instead. The intent was that the
entry would be hidden from the user via a poisoned d_compare, but
this was faulty.

This introduced a security issue where unpriviledged users could
access and modify extended attributes or ACLs belonging to other
users, including root.

This patch resolves the issue by properly hiding .reiserfs_priv. This
was the intent of the xattr poisoning code, but it appears to have
never worked as expected. This is fixed by using d_revalidate instead
of d_compare.

This patch makes -oexpose_privroot a no-op. I'm fine leaving it this
way. The effort involved in working out the corner cases wrt permissions
and caching outweigh the benefit of the feature.

Signed-off-by: Jeff Mahoney <jeffm@xxxxxxxx>
- ---

fs/reiserfs/dir.c | 2 --
fs/reiserfs/xattr.c | 17 ++++-------------
2 files changed, 4 insertions(+), 15 deletions(-)

- --- a/fs/reiserfs/dir.c
+++ b/fs/reiserfs/dir.c
@@ -45,8 +45,6 @@ static inline bool is_privroot_deh(struc
struct reiserfs_de_head *deh)
{
struct dentry *privroot = REISERFS_SB(dir->d_sb)->priv_root;
- - if (reiserfs_expose_privroot(dir->d_sb))
- - return 0;
return (dir == dir->d_parent && privroot->d_inode &&
deh->deh_objectid == INODE_PKEY(privroot->d_inode)->k_objectid);
}
- --- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -972,21 +972,13 @@ int reiserfs_permission(struct inode *in
return generic_permission(inode, mask, NULL);
}

- -/* This will catch lookups from the fs root to .reiserfs_priv */
- -static int
- -xattr_lookup_poison(struct dentry *dentry, struct qstr *q1, struct qstr *name)
+static int xattr_hide_revalidate(struct dentry *dentry, struct nameidata *nd)
{
- - struct dentry *priv_root = REISERFS_SB(dentry->d_sb)->priv_root;
- - if (container_of(q1, struct dentry, d_name) == priv_root)
- - return -ENOENT;
- - if (q1->len == name->len &&
- - !memcmp(q1->name, name->name, name->len))
- - return 0;
- - return 1;
+ return -EPERM;
}

static const struct dentry_operations xattr_lookup_poison_ops = {
- - .d_compare = xattr_lookup_poison,
+ .d_revalidate = xattr_hide_revalidate,
};

int reiserfs_lookup_privroot(struct super_block *s)
@@ -1000,8 +992,7 @@ int reiserfs_lookup_privroot(struct supe
strlen(PRIVROOT_NAME));
if (!IS_ERR(dentry)) {
REISERFS_SB(s)->priv_root = dentry;
- - if (!reiserfs_expose_privroot(s))
- - s->s_root->d_op = &xattr_lookup_poison_ops;
+ dentry->d_op = &xattr_lookup_poison_ops;
if (dentry->d_inode)
dentry->d_inode->i_flags |= S_PRIVATE;
} else
- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/

iEYEARECAAYFAku9EtoACgkQLPWxlyuTD7LxOACdHf0maAVkCXKqrsMk6XP5dNT8
XPIAoIUB9xlvRigutaWf1XBre/mzyBAk
=G+Lo
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/