diff options
author | Al Viro <viro@zeniv.linux.org.uk> | 2011-03-08 01:25:28 -0500 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2011-03-08 02:22:27 -0500 |
commit | dfef6dcd35cb4a251f6322ca9b2c06f0bb1aa1f4 (patch) | |
tree | 65e8a25d4ed913658db35c4b97ab0a021c2124eb | |
parent | 1858efd471624ecb37e6b5462cab8076f47d1cee (diff) |
unfuck proc_sysctl ->d_compare()
a) struct inode is not going to be freed under ->d_compare();
however, the thing PROC_I(inode)->sysctl points to just might.
Fortunately, it's enough to make freeing that sucker delayed,
provided that we don't step on its ->unregistering, clear
the pointer to it in PROC_I(inode) before dropping the reference
and check if it's NULL in ->d_compare().
b) I'm not sure that we *can* walk into NULL inode here (we recheck
dentry->seq between verifying that it's still hashed / fetching
dentry->d_inode and passing it to ->d_compare() and there's no
negative hashed dentries in /proc/sys/*), but if we can walk into
that, we really should not have ->d_compare() return 0 on it!
Said that, I really suspect that this check can be simply killed.
Nick?
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
-rw-r--r-- | fs/proc/inode.c | 8 | ||||
-rw-r--r-- | fs/proc/proc_sysctl.c | 7 | ||||
-rw-r--r-- | include/linux/sysctl.h | 14 | ||||
-rw-r--r-- | kernel/sysctl.c | 15 |
4 files changed, 31 insertions, 13 deletions
diff --git a/fs/proc/inode.c b/fs/proc/inode.c index 176ce4cda68a..d6a7ca1fdac5 100644 --- a/fs/proc/inode.c +++ b/fs/proc/inode.c | |||
@@ -27,6 +27,7 @@ | |||
27 | static void proc_evict_inode(struct inode *inode) | 27 | static void proc_evict_inode(struct inode *inode) |
28 | { | 28 | { |
29 | struct proc_dir_entry *de; | 29 | struct proc_dir_entry *de; |
30 | struct ctl_table_header *head; | ||
30 | 31 | ||
31 | truncate_inode_pages(&inode->i_data, 0); | 32 | truncate_inode_pages(&inode->i_data, 0); |
32 | end_writeback(inode); | 33 | end_writeback(inode); |
@@ -38,8 +39,11 @@ static void proc_evict_inode(struct inode *inode) | |||
38 | de = PROC_I(inode)->pde; | 39 | de = PROC_I(inode)->pde; |
39 | if (de) | 40 | if (de) |
40 | pde_put(de); | 41 | pde_put(de); |
41 | if (PROC_I(inode)->sysctl) | 42 | head = PROC_I(inode)->sysctl; |
42 | sysctl_head_put(PROC_I(inode)->sysctl); | 43 | if (head) { |
44 | rcu_assign_pointer(PROC_I(inode)->sysctl, NULL); | ||
45 | sysctl_head_put(head); | ||
46 | } | ||
43 | } | 47 | } |
44 | 48 | ||
45 | struct vfsmount *proc_mnt; | 49 | struct vfsmount *proc_mnt; |
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 09a1f92a34ef..8eb2522111c5 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c | |||
@@ -408,15 +408,18 @@ static int proc_sys_compare(const struct dentry *parent, | |||
408 | const struct dentry *dentry, const struct inode *inode, | 408 | const struct dentry *dentry, const struct inode *inode, |
409 | unsigned int len, const char *str, const struct qstr *name) | 409 | unsigned int len, const char *str, const struct qstr *name) |
410 | { | 410 | { |
411 | struct ctl_table_header *head; | ||
411 | /* Although proc doesn't have negative dentries, rcu-walk means | 412 | /* Although proc doesn't have negative dentries, rcu-walk means |
412 | * that inode here can be NULL */ | 413 | * that inode here can be NULL */ |
414 | /* AV: can it, indeed? */ | ||
413 | if (!inode) | 415 | if (!inode) |
414 | return 0; | 416 | return 1; |
415 | if (name->len != len) | 417 | if (name->len != len) |
416 | return 1; | 418 | return 1; |
417 | if (memcmp(name->name, str, len)) | 419 | if (memcmp(name->name, str, len)) |
418 | return 1; | 420 | return 1; |
419 | return !sysctl_is_seen(PROC_I(inode)->sysctl); | 421 | head = rcu_dereference(PROC_I(inode)->sysctl); |
422 | return !head || !sysctl_is_seen(head); | ||
420 | } | 423 | } |
421 | 424 | ||
422 | static const struct dentry_operations proc_sys_dentry_operations = { | 425 | static const struct dentry_operations proc_sys_dentry_operations = { |
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 7bb5cb64f3b8..bb7c2b086fa4 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h | |||
@@ -25,6 +25,7 @@ | |||
25 | #include <linux/kernel.h> | 25 | #include <linux/kernel.h> |
26 | #include <linux/types.h> | 26 | #include <linux/types.h> |
27 | #include <linux/compiler.h> | 27 | #include <linux/compiler.h> |
28 | #include <linux/rcupdate.h> | ||
28 | 29 | ||
29 | struct completion; | 30 | struct completion; |
30 | 31 | ||
@@ -1037,10 +1038,15 @@ struct ctl_table_root { | |||
1037 | struct ctl_table trees. */ | 1038 | struct ctl_table trees. */ |
1038 | struct ctl_table_header | 1039 | struct ctl_table_header |
1039 | { | 1040 | { |
1040 | struct ctl_table *ctl_table; | 1041 | union { |
1041 | struct list_head ctl_entry; | 1042 | struct { |
1042 | int used; | 1043 | struct ctl_table *ctl_table; |
1043 | int count; | 1044 | struct list_head ctl_entry; |
1045 | int used; | ||
1046 | int count; | ||
1047 | }; | ||
1048 | struct rcu_head rcu; | ||
1049 | }; | ||
1044 | struct completion *unregistering; | 1050 | struct completion *unregistering; |
1045 | struct ctl_table *ctl_table_arg; | 1051 | struct ctl_table *ctl_table_arg; |
1046 | struct ctl_table_root *root; | 1052 | struct ctl_table_root *root; |
diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 0f1bd83db985..4eed0af5d144 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c | |||
@@ -194,9 +194,9 @@ static int sysrq_sysctl_handler(ctl_table *table, int write, | |||
194 | static struct ctl_table root_table[]; | 194 | static struct ctl_table root_table[]; |
195 | static struct ctl_table_root sysctl_table_root; | 195 | static struct ctl_table_root sysctl_table_root; |
196 | static struct ctl_table_header root_table_header = { | 196 | static struct ctl_table_header root_table_header = { |
197 | .count = 1, | 197 | {{.count = 1, |
198 | .ctl_table = root_table, | 198 | .ctl_table = root_table, |
199 | .ctl_entry = LIST_HEAD_INIT(sysctl_table_root.default_set.list), | 199 | .ctl_entry = LIST_HEAD_INIT(sysctl_table_root.default_set.list),}}, |
200 | .root = &sysctl_table_root, | 200 | .root = &sysctl_table_root, |
201 | .set = &sysctl_table_root.default_set, | 201 | .set = &sysctl_table_root.default_set, |
202 | }; | 202 | }; |
@@ -1567,11 +1567,16 @@ void sysctl_head_get(struct ctl_table_header *head) | |||
1567 | spin_unlock(&sysctl_lock); | 1567 | spin_unlock(&sysctl_lock); |
1568 | } | 1568 | } |
1569 | 1569 | ||
1570 | static void free_head(struct rcu_head *rcu) | ||
1571 | { | ||
1572 | kfree(container_of(rcu, struct ctl_table_header, rcu)); | ||
1573 | } | ||
1574 | |||
1570 | void sysctl_head_put(struct ctl_table_header *head) | 1575 | void sysctl_head_put(struct ctl_table_header *head) |
1571 | { | 1576 | { |
1572 | spin_lock(&sysctl_lock); | 1577 | spin_lock(&sysctl_lock); |
1573 | if (!--head->count) | 1578 | if (!--head->count) |
1574 | kfree(head); | 1579 | call_rcu(&head->rcu, free_head); |
1575 | spin_unlock(&sysctl_lock); | 1580 | spin_unlock(&sysctl_lock); |
1576 | } | 1581 | } |
1577 | 1582 | ||
@@ -1948,10 +1953,10 @@ void unregister_sysctl_table(struct ctl_table_header * header) | |||
1948 | start_unregistering(header); | 1953 | start_unregistering(header); |
1949 | if (!--header->parent->count) { | 1954 | if (!--header->parent->count) { |
1950 | WARN_ON(1); | 1955 | WARN_ON(1); |
1951 | kfree(header->parent); | 1956 | call_rcu(&header->parent->rcu, free_head); |
1952 | } | 1957 | } |
1953 | if (!--header->count) | 1958 | if (!--header->count) |
1954 | kfree(header); | 1959 | call_rcu(&header->rcu, free_head); |
1955 | spin_unlock(&sysctl_lock); | 1960 | spin_unlock(&sysctl_lock); |
1956 | } | 1961 | } |
1957 | 1962 | ||