diff options
author | Alexey Dobriyan <adobriyan@gmail.com> | 2009-03-25 15:48:06 -0400 |
---|---|---|
committer | Alexey Dobriyan <adobriyan@gmail.com> | 2009-03-30 17:14:44 -0400 |
commit | 99b76233803beab302123d243eea9e41149804f3 (patch) | |
tree | 398178210fe66845ccd6fa4258ba762a87e023ad /arch | |
parent | 3dec7f59c370c7b58184d63293c3dc984d475840 (diff) |
proc 2/2: remove struct proc_dir_entry::owner
Setting ->owner as done currently (pde->owner = THIS_MODULE) is racy
as correctly noted at bug #12454. Someone can lookup entry with NULL
->owner, thus not pinning enything, and release it later resulting
in module refcount underflow.
We can keep ->owner and supply it at registration time like ->proc_fops
and ->data.
But this leaves ->owner as easy-manipulative field (just one C assignment)
and somebody will forget to unpin previous/pin current module when
switching ->owner. ->proc_fops is declared as "const" which should give
some thoughts.
->read_proc/->write_proc were just fixed to not require ->owner for
protection.
rmmod'ed directories will be empty and return "." and ".." -- no harm.
And directories with tricky enough readdir and lookup shouldn't be modular.
We definitely don't want such modular code.
Removing ->owner will also make PDE smaller.
So, let's nuke it.
Kudos to Jeff Layton for reminding about this, let's say, oversight.
http://bugzilla.kernel.org/show_bug.cgi?id=12454
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Diffstat (limited to 'arch')
-rw-r--r-- | arch/alpha/kernel/srm_env.c | 5 | ||||
-rw-r--r-- | arch/blackfin/mm/sram-alloc.c | 1 | ||||
-rw-r--r-- | arch/ia64/kernel/palinfo.c | 2 | ||||
-rw-r--r-- | arch/ia64/sn/kernel/sn2/prominfo_proc.c | 9 | ||||
-rw-r--r-- | arch/powerpc/kernel/rtas_flash.c | 1 | ||||
-rw-r--r-- | arch/sparc/kernel/led.c | 1 | ||||
-rw-r--r-- | arch/x86/kernel/cpu/mtrr/if.c | 10 |
7 files changed, 3 insertions, 26 deletions
diff --git a/arch/alpha/kernel/srm_env.c b/arch/alpha/kernel/srm_env.c index 78ad7cd1bbd6..d12af472e1c0 100644 --- a/arch/alpha/kernel/srm_env.c +++ b/arch/alpha/kernel/srm_env.c | |||
@@ -218,7 +218,6 @@ srm_env_init(void) | |||
218 | BASE_DIR); | 218 | BASE_DIR); |
219 | goto cleanup; | 219 | goto cleanup; |
220 | } | 220 | } |
221 | base_dir->owner = THIS_MODULE; | ||
222 | 221 | ||
223 | /* | 222 | /* |
224 | * Create per-name subdirectory | 223 | * Create per-name subdirectory |
@@ -229,7 +228,6 @@ srm_env_init(void) | |||
229 | BASE_DIR, NAMED_DIR); | 228 | BASE_DIR, NAMED_DIR); |
230 | goto cleanup; | 229 | goto cleanup; |
231 | } | 230 | } |
232 | named_dir->owner = THIS_MODULE; | ||
233 | 231 | ||
234 | /* | 232 | /* |
235 | * Create per-number subdirectory | 233 | * Create per-number subdirectory |
@@ -241,7 +239,6 @@ srm_env_init(void) | |||
241 | goto cleanup; | 239 | goto cleanup; |
242 | 240 | ||
243 | } | 241 | } |
244 | numbered_dir->owner = THIS_MODULE; | ||
245 | 242 | ||
246 | /* | 243 | /* |
247 | * Create all named nodes | 244 | * Create all named nodes |
@@ -254,7 +251,6 @@ srm_env_init(void) | |||
254 | goto cleanup; | 251 | goto cleanup; |
255 | 252 | ||
256 | entry->proc_entry->data = (void *) entry; | 253 | entry->proc_entry->data = (void *) entry; |
257 | entry->proc_entry->owner = THIS_MODULE; | ||
258 | entry->proc_entry->read_proc = srm_env_read; | 254 | entry->proc_entry->read_proc = srm_env_read; |
259 | entry->proc_entry->write_proc = srm_env_write; | 255 | entry->proc_entry->write_proc = srm_env_write; |
260 | 256 | ||
@@ -275,7 +271,6 @@ srm_env_init(void) | |||
275 | 271 | ||
276 | entry->id = var_num; | 272 | entry->id = var_num; |
277 | entry->proc_entry->data = (void *) entry; | 273 | entry->proc_entry->data = (void *) entry; |
278 | entry->proc_entry->owner = THIS_MODULE; | ||
279 | entry->proc_entry->read_proc = srm_env_read; | 274 | entry->proc_entry->read_proc = srm_env_read; |
280 | entry->proc_entry->write_proc = srm_env_write; | 275 | entry->proc_entry->write_proc = srm_env_write; |
281 | } | 276 | } |
diff --git a/arch/blackfin/mm/sram-alloc.c b/arch/blackfin/mm/sram-alloc.c index 834cab7438a8..530d1393a232 100644 --- a/arch/blackfin/mm/sram-alloc.c +++ b/arch/blackfin/mm/sram-alloc.c | |||
@@ -854,7 +854,6 @@ static int __init sram_proc_init(void) | |||
854 | printk(KERN_WARNING "unable to create /proc/sram\n"); | 854 | printk(KERN_WARNING "unable to create /proc/sram\n"); |
855 | return -1; | 855 | return -1; |
856 | } | 856 | } |
857 | ptr->owner = THIS_MODULE; | ||
858 | ptr->read_proc = sram_proc_read; | 857 | ptr->read_proc = sram_proc_read; |
859 | return 0; | 858 | return 0; |
860 | } | 859 | } |
diff --git a/arch/ia64/kernel/palinfo.c b/arch/ia64/kernel/palinfo.c index e5c57f413ca2..a4f19c70aadd 100644 --- a/arch/ia64/kernel/palinfo.c +++ b/arch/ia64/kernel/palinfo.c | |||
@@ -1002,8 +1002,6 @@ create_palinfo_proc_entries(unsigned int cpu) | |||
1002 | *pdir = create_proc_read_entry( | 1002 | *pdir = create_proc_read_entry( |
1003 | palinfo_entries[j].name, 0, cpu_dir, | 1003 | palinfo_entries[j].name, 0, cpu_dir, |
1004 | palinfo_read_entry, (void *)f.value); | 1004 | palinfo_read_entry, (void *)f.value); |
1005 | if (*pdir) | ||
1006 | (*pdir)->owner = THIS_MODULE; | ||
1007 | pdir++; | 1005 | pdir++; |
1008 | } | 1006 | } |
1009 | } | 1007 | } |
diff --git a/arch/ia64/sn/kernel/sn2/prominfo_proc.c b/arch/ia64/sn/kernel/sn2/prominfo_proc.c index 4dcce3d0e04c..e63328818643 100644 --- a/arch/ia64/sn/kernel/sn2/prominfo_proc.c +++ b/arch/ia64/sn/kernel/sn2/prominfo_proc.c | |||
@@ -225,7 +225,6 @@ static struct proc_dir_entry *sgi_prominfo_entry; | |||
225 | int __init prominfo_init(void) | 225 | int __init prominfo_init(void) |
226 | { | 226 | { |
227 | struct proc_dir_entry **entp; | 227 | struct proc_dir_entry **entp; |
228 | struct proc_dir_entry *p; | ||
229 | cnodeid_t cnodeid; | 228 | cnodeid_t cnodeid; |
230 | unsigned long nasid; | 229 | unsigned long nasid; |
231 | int size; | 230 | int size; |
@@ -246,14 +245,10 @@ int __init prominfo_init(void) | |||
246 | sprintf(name, "node%d", cnodeid); | 245 | sprintf(name, "node%d", cnodeid); |
247 | *entp = proc_mkdir(name, sgi_prominfo_entry); | 246 | *entp = proc_mkdir(name, sgi_prominfo_entry); |
248 | nasid = cnodeid_to_nasid(cnodeid); | 247 | nasid = cnodeid_to_nasid(cnodeid); |
249 | p = create_proc_read_entry("fit", 0, *entp, read_fit_entry, | 248 | create_proc_read_entry("fit", 0, *entp, read_fit_entry, |
250 | (void *)nasid); | 249 | (void *)nasid); |
251 | if (p) | 250 | create_proc_read_entry("version", 0, *entp, |
252 | p->owner = THIS_MODULE; | ||
253 | p = create_proc_read_entry("version", 0, *entp, | ||
254 | read_version_entry, (void *)nasid); | 251 | read_version_entry, (void *)nasid); |
255 | if (p) | ||
256 | p->owner = THIS_MODULE; | ||
257 | entp++; | 252 | entp++; |
258 | } | 253 | } |
259 | 254 | ||
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c index 149cb112cd1a..13011a96a977 100644 --- a/arch/powerpc/kernel/rtas_flash.c +++ b/arch/powerpc/kernel/rtas_flash.c | |||
@@ -669,7 +669,6 @@ static void remove_flash_pde(struct proc_dir_entry *dp) | |||
669 | { | 669 | { |
670 | if (dp) { | 670 | if (dp) { |
671 | kfree(dp->data); | 671 | kfree(dp->data); |
672 | dp->owner = NULL; | ||
673 | remove_proc_entry(dp->name, dp->parent); | 672 | remove_proc_entry(dp->name, dp->parent); |
674 | } | 673 | } |
675 | } | 674 | } |
diff --git a/arch/sparc/kernel/led.c b/arch/sparc/kernel/led.c index adaaed4ea2fb..00d034ea2164 100644 --- a/arch/sparc/kernel/led.c +++ b/arch/sparc/kernel/led.c | |||
@@ -126,7 +126,6 @@ static int __init led_init(void) | |||
126 | led = proc_create("led", 0, NULL, &led_proc_fops); | 126 | led = proc_create("led", 0, NULL, &led_proc_fops); |
127 | if (!led) | 127 | if (!led) |
128 | return -ENOMEM; | 128 | return -ENOMEM; |
129 | led->owner = THIS_MODULE; | ||
130 | 129 | ||
131 | printk(KERN_INFO | 130 | printk(KERN_INFO |
132 | "led: version %s, Lars Kotthoff <metalhead@metalhead.ws>\n", | 131 | "led: version %s, Lars Kotthoff <metalhead@metalhead.ws>\n", |
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c index 4c4214690dd1..fb73a52913a4 100644 --- a/arch/x86/kernel/cpu/mtrr/if.c +++ b/arch/x86/kernel/cpu/mtrr/if.c | |||
@@ -377,10 +377,6 @@ static const struct file_operations mtrr_fops = { | |||
377 | .release = mtrr_close, | 377 | .release = mtrr_close, |
378 | }; | 378 | }; |
379 | 379 | ||
380 | |||
381 | static struct proc_dir_entry *proc_root_mtrr; | ||
382 | |||
383 | |||
384 | static int mtrr_seq_show(struct seq_file *seq, void *offset) | 380 | static int mtrr_seq_show(struct seq_file *seq, void *offset) |
385 | { | 381 | { |
386 | char factor; | 382 | char factor; |
@@ -423,11 +419,7 @@ static int __init mtrr_if_init(void) | |||
423 | (!cpu_has(c, X86_FEATURE_CENTAUR_MCR))) | 419 | (!cpu_has(c, X86_FEATURE_CENTAUR_MCR))) |
424 | return -ENODEV; | 420 | return -ENODEV; |
425 | 421 | ||
426 | proc_root_mtrr = | 422 | proc_create("mtrr", S_IWUSR | S_IRUGO, NULL, &mtrr_fops); |
427 | proc_create("mtrr", S_IWUSR | S_IRUGO, NULL, &mtrr_fops); | ||
428 | |||
429 | if (proc_root_mtrr) | ||
430 | proc_root_mtrr->owner = THIS_MODULE; | ||
431 | return 0; | 423 | return 0; |
432 | } | 424 | } |
433 | 425 | ||