diff options
author | Alexey Dobriyan <adobriyan@sw.ru> | 2008-02-08 07:18:37 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2008-02-08 12:22:24 -0500 |
commit | 2d3a4e3666325a9709cc8ea2e88151394e8f20fc (patch) | |
tree | c3e75289de8c824f5e2bb2e97d4b6853d48da3f3 /fs/proc | |
parent | c6caeb7c4544608e8ae62731334661fc396c7f85 (diff) |
proc: fix ->open'less usage due to ->proc_fops flip
Typical PDE creation code looks like:
pde = create_proc_entry("foo", 0, NULL);
if (pde)
pde->proc_fops = &foo_proc_fops;
Notice that PDE is first created, only then ->proc_fops is set up to
final value. This is a problem because right after creation
a) PDE is fully visible in /proc , and
b) ->proc_fops are proc_file_operations which do not have ->open callback. So, it's
possible to ->read without ->open (see one class of oopses below).
The fix is new API called proc_create() which makes sure ->proc_fops are
set up before gluing PDE to main tree. Typical new code looks like:
pde = proc_create("foo", 0, NULL, &foo_proc_fops);
if (!pde)
return -ENOMEM;
Fix most networking users for a start.
In the long run, create_proc_entry() for regular files will go.
BUG: unable to handle kernel NULL pointer dereference at virtual address 00000024
printing eip: c1188c1b *pdpt = 000000002929e001 *pde = 0000000000000000
Oops: 0002 [#1] PREEMPT SMP DEBUG_PAGEALLOC
last sysfs file: /sys/block/sda/sda1/dev
Modules linked in: foo af_packet ipv6 cpufreq_ondemand loop serio_raw psmouse k8temp hwmon sr_mod cdrom
Pid: 24679, comm: cat Not tainted (2.6.24-rc3-mm1 #2)
EIP: 0060:[<c1188c1b>] EFLAGS: 00210002 CPU: 0
EIP is at mutex_lock_nested+0x75/0x25d
EAX: 000006fe EBX: fffffffb ECX: 00001000 EDX: e9340570
ESI: 00000020 EDI: 00200246 EBP: e9340570 ESP: e8ea1ef8
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Process cat (pid: 24679, ti=E8EA1000 task=E9340570 task.ti=E8EA1000)
Stack: 00000000 c106f7ce e8ee05b4 00000000 00000001 458003d0 f6fb6f20 fffffffb
00000000 c106f7aa 00001000 c106f7ce 08ae9000 f6db53f0 00000020 00200246
00000000 00000002 00000000 00200246 00200246 e8ee05a0 fffffffb e8ee0550
Call Trace:
[<c106f7ce>] seq_read+0x24/0x28a
[<c106f7aa>] seq_read+0x0/0x28a
[<c106f7ce>] seq_read+0x24/0x28a
[<c106f7aa>] seq_read+0x0/0x28a
[<c10818b8>] proc_reg_read+0x60/0x73
[<c1081858>] proc_reg_read+0x0/0x73
[<c105a34f>] vfs_read+0x6c/0x8b
[<c105a6f3>] sys_read+0x3c/0x63
[<c10025f2>] sysenter_past_esp+0x5f/0xa5
[<c10697a7>] destroy_inode+0x24/0x33
=======================
INFO: lockdep is turned off.
Code: 75 21 68 e1 1a 19 c1 68 87 00 00 00 68 b8 e8 1f c1 68 25 73 1f c1 e8 84 06 e9 ff e8 52 b8 e7 ff 83 c4 10 9c 5f fa e8 28 89 ea ff <f0> fe 4e 04 79 0a f3 90 80 7e 04 00 7e f8 eb f0 39 76 34 74 33
EIP: [<c1188c1b>] mutex_lock_nested+0x75/0x25d SS:ESP 0068:e8ea1ef8
[akpm@linux-foundation.org: coding-style fixes]
Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs/proc')
-rw-r--r-- | fs/proc/generic.c | 40 | ||||
-rw-r--r-- | fs/proc/proc_net.c | 7 | ||||
-rw-r--r-- | fs/proc/root.c | 1 |
3 files changed, 38 insertions, 10 deletions
diff --git a/fs/proc/generic.c b/fs/proc/generic.c index b9dd3628d43a..68971e66cd41 100644 --- a/fs/proc/generic.c +++ b/fs/proc/generic.c | |||
@@ -562,7 +562,7 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp | |||
562 | return 0; | 562 | return 0; |
563 | } | 563 | } |
564 | 564 | ||
565 | static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent, | 565 | static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent, |
566 | const char *name, | 566 | const char *name, |
567 | mode_t mode, | 567 | mode_t mode, |
568 | nlink_t nlink) | 568 | nlink_t nlink) |
@@ -605,7 +605,7 @@ struct proc_dir_entry *proc_symlink(const char *name, | |||
605 | { | 605 | { |
606 | struct proc_dir_entry *ent; | 606 | struct proc_dir_entry *ent; |
607 | 607 | ||
608 | ent = proc_create(&parent,name, | 608 | ent = __proc_create(&parent, name, |
609 | (S_IFLNK | S_IRUGO | S_IWUGO | S_IXUGO),1); | 609 | (S_IFLNK | S_IRUGO | S_IWUGO | S_IXUGO),1); |
610 | 610 | ||
611 | if (ent) { | 611 | if (ent) { |
@@ -630,7 +630,7 @@ struct proc_dir_entry *proc_mkdir_mode(const char *name, mode_t mode, | |||
630 | { | 630 | { |
631 | struct proc_dir_entry *ent; | 631 | struct proc_dir_entry *ent; |
632 | 632 | ||
633 | ent = proc_create(&parent, name, S_IFDIR | mode, 2); | 633 | ent = __proc_create(&parent, name, S_IFDIR | mode, 2); |
634 | if (ent) { | 634 | if (ent) { |
635 | if (proc_register(parent, ent) < 0) { | 635 | if (proc_register(parent, ent) < 0) { |
636 | kfree(ent); | 636 | kfree(ent); |
@@ -664,7 +664,7 @@ struct proc_dir_entry *create_proc_entry(const char *name, mode_t mode, | |||
664 | nlink = 1; | 664 | nlink = 1; |
665 | } | 665 | } |
666 | 666 | ||
667 | ent = proc_create(&parent,name,mode,nlink); | 667 | ent = __proc_create(&parent, name, mode, nlink); |
668 | if (ent) { | 668 | if (ent) { |
669 | if (proc_register(parent, ent) < 0) { | 669 | if (proc_register(parent, ent) < 0) { |
670 | kfree(ent); | 670 | kfree(ent); |
@@ -674,6 +674,38 @@ struct proc_dir_entry *create_proc_entry(const char *name, mode_t mode, | |||
674 | return ent; | 674 | return ent; |
675 | } | 675 | } |
676 | 676 | ||
677 | struct proc_dir_entry *proc_create(const char *name, mode_t mode, | ||
678 | struct proc_dir_entry *parent, | ||
679 | const struct file_operations *proc_fops) | ||
680 | { | ||
681 | struct proc_dir_entry *pde; | ||
682 | nlink_t nlink; | ||
683 | |||
684 | if (S_ISDIR(mode)) { | ||
685 | if ((mode & S_IALLUGO) == 0) | ||
686 | mode |= S_IRUGO | S_IXUGO; | ||
687 | nlink = 2; | ||
688 | } else { | ||
689 | if ((mode & S_IFMT) == 0) | ||
690 | mode |= S_IFREG; | ||
691 | if ((mode & S_IALLUGO) == 0) | ||
692 | mode |= S_IRUGO; | ||
693 | nlink = 1; | ||
694 | } | ||
695 | |||
696 | pde = __proc_create(&parent, name, mode, nlink); | ||
697 | if (!pde) | ||
698 | goto out; | ||
699 | pde->proc_fops = proc_fops; | ||
700 | if (proc_register(parent, pde) < 0) | ||
701 | goto out_free; | ||
702 | return pde; | ||
703 | out_free: | ||
704 | kfree(pde); | ||
705 | out: | ||
706 | return NULL; | ||
707 | } | ||
708 | |||
677 | void free_proc_entry(struct proc_dir_entry *de) | 709 | void free_proc_entry(struct proc_dir_entry *de) |
678 | { | 710 | { |
679 | unsigned int ino = de->low_ino; | 711 | unsigned int ino = de->low_ino; |
diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c index 4823c9677fac..14e9b5aaf863 100644 --- a/fs/proc/proc_net.c +++ b/fs/proc/proc_net.c | |||
@@ -67,12 +67,7 @@ EXPORT_SYMBOL_GPL(seq_release_net); | |||
67 | struct proc_dir_entry *proc_net_fops_create(struct net *net, | 67 | struct proc_dir_entry *proc_net_fops_create(struct net *net, |
68 | const char *name, mode_t mode, const struct file_operations *fops) | 68 | const char *name, mode_t mode, const struct file_operations *fops) |
69 | { | 69 | { |
70 | struct proc_dir_entry *res; | 70 | return proc_create(name, mode, net->proc_net, fops); |
71 | |||
72 | res = create_proc_entry(name, mode, net->proc_net); | ||
73 | if (res) | ||
74 | res->proc_fops = fops; | ||
75 | return res; | ||
76 | } | 71 | } |
77 | EXPORT_SYMBOL_GPL(proc_net_fops_create); | 72 | EXPORT_SYMBOL_GPL(proc_net_fops_create); |
78 | 73 | ||
diff --git a/fs/proc/root.c b/fs/proc/root.c index 81f99e691f99..ef0fb57fc9ef 100644 --- a/fs/proc/root.c +++ b/fs/proc/root.c | |||
@@ -232,6 +232,7 @@ void pid_ns_release_proc(struct pid_namespace *ns) | |||
232 | EXPORT_SYMBOL(proc_symlink); | 232 | EXPORT_SYMBOL(proc_symlink); |
233 | EXPORT_SYMBOL(proc_mkdir); | 233 | EXPORT_SYMBOL(proc_mkdir); |
234 | EXPORT_SYMBOL(create_proc_entry); | 234 | EXPORT_SYMBOL(create_proc_entry); |
235 | EXPORT_SYMBOL(proc_create); | ||
235 | EXPORT_SYMBOL(remove_proc_entry); | 236 | EXPORT_SYMBOL(remove_proc_entry); |
236 | EXPORT_SYMBOL(proc_root); | 237 | EXPORT_SYMBOL(proc_root); |
237 | EXPORT_SYMBOL(proc_root_fs); | 238 | EXPORT_SYMBOL(proc_root_fs); |