diff options
author | Jeff Mahoney <jeffm@suse.com> | 2010-04-23 13:17:37 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2010-04-24 14:31:24 -0400 |
commit | cac36f707119b792b2396aed371d6b5cdc194890 (patch) | |
tree | ccb19a68ce1c1aa066d6d13dde51657cba906de6 /fs/reiserfs/xattr.c | |
parent | 33eaf788345c0311ab48ae62673c05f59fb09bb3 (diff) |
reiserfs: fix permissions on .reiserfs_priv
Commit 677c9b2e393a0cd203bd54e9c18b012b2c73305a ("reiserfs: remove
privroot hiding in lookup") 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 unprivileged 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@suse.com>
Acked-by: Edward Shishkin <edward.shishkin@gmail.com>
Reported-by: Matt McCutchen <matt@mattmccutchen.net>
Tested-by: Matt McCutchen <matt@mattmccutchen.net>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs/reiserfs/xattr.c')
-rw-r--r-- | fs/reiserfs/xattr.c | 17 |
1 files changed, 4 insertions, 13 deletions
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c index 4f9586bb7631..28f0a448f3c7 100644 --- a/fs/reiserfs/xattr.c +++ b/fs/reiserfs/xattr.c | |||
@@ -973,21 +973,13 @@ int reiserfs_permission(struct inode *inode, int mask) | |||
973 | return generic_permission(inode, mask, NULL); | 973 | return generic_permission(inode, mask, NULL); |
974 | } | 974 | } |
975 | 975 | ||
976 | /* This will catch lookups from the fs root to .reiserfs_priv */ | 976 | static int xattr_hide_revalidate(struct dentry *dentry, struct nameidata *nd) |
977 | static int | ||
978 | xattr_lookup_poison(struct dentry *dentry, struct qstr *q1, struct qstr *name) | ||
979 | { | 977 | { |
980 | struct dentry *priv_root = REISERFS_SB(dentry->d_sb)->priv_root; | 978 | return -EPERM; |
981 | if (container_of(q1, struct dentry, d_name) == priv_root) | ||
982 | return -ENOENT; | ||
983 | if (q1->len == name->len && | ||
984 | !memcmp(q1->name, name->name, name->len)) | ||
985 | return 0; | ||
986 | return 1; | ||
987 | } | 979 | } |
988 | 980 | ||
989 | static const struct dentry_operations xattr_lookup_poison_ops = { | 981 | static const struct dentry_operations xattr_lookup_poison_ops = { |
990 | .d_compare = xattr_lookup_poison, | 982 | .d_revalidate = xattr_hide_revalidate, |
991 | }; | 983 | }; |
992 | 984 | ||
993 | int reiserfs_lookup_privroot(struct super_block *s) | 985 | int reiserfs_lookup_privroot(struct super_block *s) |
@@ -1001,8 +993,7 @@ int reiserfs_lookup_privroot(struct super_block *s) | |||
1001 | strlen(PRIVROOT_NAME)); | 993 | strlen(PRIVROOT_NAME)); |
1002 | if (!IS_ERR(dentry)) { | 994 | if (!IS_ERR(dentry)) { |
1003 | REISERFS_SB(s)->priv_root = dentry; | 995 | REISERFS_SB(s)->priv_root = dentry; |
1004 | if (!reiserfs_expose_privroot(s)) | 996 | dentry->d_op = &xattr_lookup_poison_ops; |
1005 | s->s_root->d_op = &xattr_lookup_poison_ops; | ||
1006 | if (dentry->d_inode) | 997 | if (dentry->d_inode) |
1007 | dentry->d_inode->i_flags |= S_PRIVATE; | 998 | dentry->d_inode->i_flags |= S_PRIVATE; |
1008 | } else | 999 | } else |