diff options
author | Alexey Dobriyan <adobriyan@sw.ru> | 2007-11-28 19:21:23 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2007-11-29 12:24:52 -0500 |
commit | c2319540cd7330fa9066e5b9b84d357a2c8631a2 (patch) | |
tree | e63a0aeae5a9951a9cbc705fdb48eecc7ec62110 | |
parent | a7839e960675b549f06209d18283d5cee2ce9261 (diff) |
proc: fix NULL ->i_fop oops
proc_kill_inodes() can clear ->i_fop in the middle of vfs_readdir resulting in
NULL dereference during "file->f_op->readdir(file, buf, filler)".
The solution is to remove proc_kill_inodes() completely:
a) we don't have tricky modules implementing their tricky readdir hooks which
could keeping this revoke from hell.
b) In a situation when module is gone but PDE still alive, standard
readdir will return only "." and "..", because pde->next was cleared by
remove_proc_entry().
c) the race proc_kill_inode() destined to prevent is not completely
fixed, just race window made smaller, because vfs_readdir() is run
without sb_lock held and without file_list_lock held. Effectively,
->i_fop is cleared at random moment, which can't fix properly anything.
BUG: unable to handle kernel NULL pointer dereference at virtual address 00000018
printing eip: c1061205 *pdpt = 0000000005b22001 *pde = 0000000000000000
Oops: 0000 [#1] PREEMPT SMP
Modules linked in: foo af_packet ipv6 cpufreq_ondemand loop serio_raw sr_mod k8temp cdrom hwmon amd_rng
Pid: 2033, comm: find Not tainted (2.6.24-rc1-b1d08ac064268d0ae2281e98bf5e82627e0f0c56 #2)
EIP: 0060:[<c1061205>] EFLAGS: 00010246 CPU: 0
EIP is at vfs_readdir+0x47/0x74
EAX: c6b6a780 EBX: 00000000 ECX: c1061040 EDX: c5decf94
ESI: c6b6a780 EDI: fffffffe EBP: c9797c54 ESP: c5decf78
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Process find (pid: 2033, ti=c5dec000 task=c64bba90 task.ti=c5dec000)
Stack: c5decf94 c1061040 fffffff7 0805ffbc 00000000 c6b6a780 c1061295 0805ffbc
00000000 00000400 00000000 00000004 0805ffbc 4588eff4 c5dec000 c10026ba
00000004 0805ffbc 00000400 0805ffbc 4588eff4 bfdc6c70 000000dc 0000007b
Call Trace:
[<c1061040>] filldir64+0x0/0xc5
[<c1061295>] sys_getdents64+0x63/0xa5
[<c10026ba>] sysenter_past_esp+0x5f/0x85
=======================
Code: 49 83 78 18 00 74 43 8d 6b 74 bf fe ff ff ff 89 e8 e8 b8 c0 12 00 f6 83 2c 01 00 00 10 75 22 8b 5e 10 8b 4c 24 04 89 f0 8b 14 24 <ff> 53 18 f6 46 1a 04 89 c7 75 0b 8b 56 0c 8b 46 08 e8 c8 66 00
EIP: [<c1061205>] vfs_readdir+0x47/0x74 SS:ESP 0068:c5decf78
hch: "Nice, getting rid of this is a very good step formwards.
Unfortunately we have another copy of this junk in
security/selinux/selinuxfs.c:sel_remove_entries() which would need the
same treatment."
Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
Acked-by: Christoph Hellwig <hch@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: James Morris <jmorris@namei.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | fs/proc/generic.c | 37 | ||||
-rw-r--r-- | fs/proc/internal.h | 2 | ||||
-rw-r--r-- | fs/proc/root.c | 2 |
3 files changed, 1 insertions, 40 deletions
diff --git a/fs/proc/generic.c b/fs/proc/generic.c index a9806bc21ec3..39f3d6519035 100644 --- a/fs/proc/generic.c +++ b/fs/proc/generic.c | |||
@@ -555,41 +555,6 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp | |||
555 | return 0; | 555 | return 0; |
556 | } | 556 | } |
557 | 557 | ||
558 | /* | ||
559 | * Kill an inode that got unregistered.. | ||
560 | */ | ||
561 | static void proc_kill_inodes(struct proc_dir_entry *de) | ||
562 | { | ||
563 | struct list_head *p; | ||
564 | struct super_block *sb; | ||
565 | |||
566 | /* | ||
567 | * Actually it's a partial revoke(). | ||
568 | */ | ||
569 | spin_lock(&sb_lock); | ||
570 | list_for_each_entry(sb, &proc_fs_type.fs_supers, s_instances) { | ||
571 | file_list_lock(); | ||
572 | list_for_each(p, &sb->s_files) { | ||
573 | struct file *filp = list_entry(p, struct file, | ||
574 | f_u.fu_list); | ||
575 | struct dentry *dentry = filp->f_path.dentry; | ||
576 | struct inode *inode; | ||
577 | const struct file_operations *fops; | ||
578 | |||
579 | if (dentry->d_op != &proc_dentry_operations) | ||
580 | continue; | ||
581 | inode = dentry->d_inode; | ||
582 | if (PDE(inode) != de) | ||
583 | continue; | ||
584 | fops = filp->f_op; | ||
585 | filp->f_op = NULL; | ||
586 | fops_put(fops); | ||
587 | } | ||
588 | file_list_unlock(); | ||
589 | } | ||
590 | spin_unlock(&sb_lock); | ||
591 | } | ||
592 | |||
593 | static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent, | 558 | static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent, |
594 | const char *name, | 559 | const char *name, |
595 | mode_t mode, | 560 | mode_t mode, |
@@ -764,8 +729,6 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent) | |||
764 | continue_removing: | 729 | continue_removing: |
765 | if (S_ISDIR(de->mode)) | 730 | if (S_ISDIR(de->mode)) |
766 | parent->nlink--; | 731 | parent->nlink--; |
767 | if (!S_ISREG(de->mode)) | ||
768 | proc_kill_inodes(de); | ||
769 | de->nlink = 0; | 732 | de->nlink = 0; |
770 | WARN_ON(de->subdir); | 733 | WARN_ON(de->subdir); |
771 | if (!atomic_read(&de->count)) | 734 | if (!atomic_read(&de->count)) |
diff --git a/fs/proc/internal.h b/fs/proc/internal.h index 1b2b6c6bb475..1820eb2ef762 100644 --- a/fs/proc/internal.h +++ b/fs/proc/internal.h | |||
@@ -78,5 +78,3 @@ static inline int proc_fd(struct inode *inode) | |||
78 | { | 78 | { |
79 | return PROC_I(inode)->fd; | 79 | return PROC_I(inode)->fd; |
80 | } | 80 | } |
81 | |||
82 | extern struct file_system_type proc_fs_type; | ||
diff --git a/fs/proc/root.c b/fs/proc/root.c index 1f86bb860e04..ec9cb3b6c93b 100644 --- a/fs/proc/root.c +++ b/fs/proc/root.c | |||
@@ -98,7 +98,7 @@ static void proc_kill_sb(struct super_block *sb) | |||
98 | put_pid_ns(ns); | 98 | put_pid_ns(ns); |
99 | } | 99 | } |
100 | 100 | ||
101 | struct file_system_type proc_fs_type = { | 101 | static struct file_system_type proc_fs_type = { |
102 | .name = "proc", | 102 | .name = "proc", |
103 | .get_sb = proc_get_sb, | 103 | .get_sb = proc_get_sb, |
104 | .kill_sb = proc_kill_sb, | 104 | .kill_sb = proc_kill_sb, |