]>
Commit | Line | Data |
---|---|---|
957fdf94 GKH |
1 | From cac36f707119b792b2396aed371d6b5cdc194890 Mon Sep 17 00:00:00 2001 |
2 | From: Jeff Mahoney <jeffm@suse.com> | |
3 | Date: Fri, 23 Apr 2010 13:17:37 -0400 | |
4 | Subject: reiserfs: fix permissions on .reiserfs_priv | |
5 | ||
6 | From: Jeff Mahoney <jeffm@suse.com> | |
7 | ||
8 | commit cac36f707119b792b2396aed371d6b5cdc194890 upstream. | |
9 | ||
10 | Commit 677c9b2e393a0cd203bd54e9c18b012b2c73305a ("reiserfs: remove | |
11 | privroot hiding in lookup") removed the magic from the lookup code to hide | |
12 | the .reiserfs_priv directory since it was getting loaded at mount-time | |
13 | instead. The intent was that the entry would be hidden from the user via | |
14 | a poisoned d_compare, but this was faulty. | |
15 | ||
16 | This introduced a security issue where unprivileged users could access and | |
17 | modify extended attributes or ACLs belonging to other users, including | |
18 | root. | |
19 | ||
20 | This patch resolves the issue by properly hiding .reiserfs_priv. This was | |
21 | the intent of the xattr poisoning code, but it appears to have never | |
22 | worked as expected. This is fixed by using d_revalidate instead of | |
23 | d_compare. | |
24 | ||
25 | This patch makes -oexpose_privroot a no-op. I'm fine leaving it this way. | |
26 | The effort involved in working out the corner cases wrt permissions and | |
27 | caching outweigh the benefit of the feature. | |
28 | ||
29 | Signed-off-by: Jeff Mahoney <jeffm@suse.com> | |
30 | Acked-by: Edward Shishkin <edward.shishkin@gmail.com> | |
31 | Reported-by: Matt McCutchen <matt@mattmccutchen.net> | |
32 | Tested-by: Matt McCutchen <matt@mattmccutchen.net> | |
33 | Cc: Frederic Weisbecker <fweisbec@gmail.com> | |
34 | Cc: Al Viro <viro@zeniv.linux.org.uk> | |
35 | Signed-off-by: Andrew Morton <akpm@linux-foundation.org> | |
36 | Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> | |
37 | Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> | |
38 | ||
39 | --- | |
40 | fs/reiserfs/dir.c | 2 -- | |
41 | fs/reiserfs/xattr.c | 17 ++++------------- | |
42 | 2 files changed, 4 insertions(+), 15 deletions(-) | |
43 | ||
44 | --- a/fs/reiserfs/dir.c | |
45 | +++ b/fs/reiserfs/dir.c | |
46 | @@ -45,8 +45,6 @@ static inline bool is_privroot_deh(struc | |
47 | struct reiserfs_de_head *deh) | |
48 | { | |
49 | struct dentry *privroot = REISERFS_SB(dir->d_sb)->priv_root; | |
50 | - if (reiserfs_expose_privroot(dir->d_sb)) | |
51 | - return 0; | |
52 | return (dir == dir->d_parent && privroot->d_inode && | |
53 | deh->deh_objectid == INODE_PKEY(privroot->d_inode)->k_objectid); | |
54 | } | |
55 | --- a/fs/reiserfs/xattr.c | |
56 | +++ b/fs/reiserfs/xattr.c | |
57 | @@ -976,21 +976,13 @@ int reiserfs_permission(struct inode *in | |
58 | return generic_permission(inode, mask, NULL); | |
59 | } | |
60 | ||
61 | -/* This will catch lookups from the fs root to .reiserfs_priv */ | |
62 | -static int | |
63 | -xattr_lookup_poison(struct dentry *dentry, struct qstr *q1, struct qstr *name) | |
64 | +static int xattr_hide_revalidate(struct dentry *dentry, struct nameidata *nd) | |
65 | { | |
66 | - struct dentry *priv_root = REISERFS_SB(dentry->d_sb)->priv_root; | |
67 | - if (container_of(q1, struct dentry, d_name) == priv_root) | |
68 | - return -ENOENT; | |
69 | - if (q1->len == name->len && | |
70 | - !memcmp(q1->name, name->name, name->len)) | |
71 | - return 0; | |
72 | - return 1; | |
73 | + return -EPERM; | |
74 | } | |
75 | ||
76 | static const struct dentry_operations xattr_lookup_poison_ops = { | |
77 | - .d_compare = xattr_lookup_poison, | |
78 | + .d_revalidate = xattr_hide_revalidate, | |
79 | }; | |
80 | ||
81 | int reiserfs_lookup_privroot(struct super_block *s) | |
82 | @@ -1004,8 +996,7 @@ int reiserfs_lookup_privroot(struct supe | |
83 | strlen(PRIVROOT_NAME)); | |
84 | if (!IS_ERR(dentry)) { | |
85 | REISERFS_SB(s)->priv_root = dentry; | |
86 | - if (!reiserfs_expose_privroot(s)) | |
87 | - s->s_root->d_op = &xattr_lookup_poison_ops; | |
88 | + dentry->d_op = &xattr_lookup_poison_ops; | |
89 | if (dentry->d_inode) | |
90 | dentry->d_inode->i_flags |= S_PRIVATE; | |
91 | } else |