Re: oops in proc_pid_stat() on task->real_parent?

[Andrew Morton]

Dave Hansen <dave@xxxxxxxx> wrote:
>
> I got this OOPS while running a relatively intensive PostgreSQL load on
>  my dual-CPU PIII with 2.6.9, while simultaneously running top.  Sorry,
>  no serial console, but I do have a screenshot:
> 
>  	http://sprucegoose.sr71.net/~dave/oops/dsc02631.jpg
> 
>  $ addr2line -e vmlinux-2.6.9 c017920e

yup, we fixed that one.


From: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>

proc_pid_status dereferences pointers in the task structure even if the
task is already dead.  This is probably the reason for the oops described
in

http://bugme.osdl.org/show_bug.cgi?id=3812

The attached patch removes the pointer dereferences by using pid_alive()
for testing that the task structure contents is still valid before
dereferencing them.  The task structure itself is guaranteed to be valid -
we hold a reference count.

Signed-Off-By: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
---

 25-akpm/fs/proc/array.c       |    4 ++--
 25-akpm/fs/proc/base.c        |    5 -----
 25-akpm/include/linux/sched.h |   13 +++++++++++++
 3 files changed, 15 insertions(+), 7 deletions(-)

diff -puN fs/proc/array.c~use-pid_alive-in-proc_pid_status fs/proc/array.c
--- 25/fs/proc/array.c~use-pid_alive-in-proc_pid_status	2004-12-02 23:31:57.000000000 -0800
+++ 25-akpm/fs/proc/array.c	2004-12-02 23:31:57.000000000 -0800
@@ -171,8 +171,8 @@ static inline char * task_state(struct t
 		get_task_state(p),
 		(p->sleep_avg/1024)*100/(1020000000/1024),
 	       	p->tgid,
-		p->pid, p->pid ? p->group_leader->real_parent->tgid : 0,
-		p->pid && p->ptrace ? p->parent->pid : 0,
+		p->pid, pid_alive(p) ? p->group_leader->real_parent->tgid : 0,
+		pid_alive(p) && p->ptrace ? p->parent->pid : 0,
 		p->uid, p->euid, p->suid, p->fsuid,
 		p->gid, p->egid, p->sgid, p->fsgid);
 	read_unlock(&tasklist_lock);
diff -puN fs/proc/base.c~use-pid_alive-in-proc_pid_status fs/proc/base.c
--- 25/fs/proc/base.c~use-pid_alive-in-proc_pid_status	2004-12-02 23:31:57.000000000 -0800
+++ 25-akpm/fs/proc/base.c	2004-12-02 23:31:57.000000000 -0800
@@ -780,11 +780,6 @@ static struct inode_operations proc_pid_
 	.follow_link	= proc_pid_follow_link
 };
 
-static inline int pid_alive(struct task_struct *p)
-{
-	return p->pids[PIDTYPE_PID].nr != 0;
-}
-
 #define NUMBUF 10
 
 static int proc_readfd(struct file * filp, void * dirent, filldir_t filldir)
diff -puN include/linux/sched.h~use-pid_alive-in-proc_pid_status include/linux/sched.h
--- 25/include/linux/sched.h~use-pid_alive-in-proc_pid_status	2004-12-02 23:31:57.000000000 -0800
+++ 25-akpm/include/linux/sched.h	2004-12-02 23:31:57.000000000 -0800
@@ -671,6 +671,19 @@ static inline pid_t process_group(struct
 	return tsk->signal->pgrp;
 }
 
+/**
+ * pid_alive - check that a task structure is not stale
+ * @p: Task structure to be checked.
+ *
+ * Test if a process is not yet dead (at most zombie state)
+ * If pid_alive fails, then pointers within the task structure
+ * can be stale and must not be dereferenced.
+ */
+static inline int pid_alive(struct task_struct *p)
+{
+	return p->pids[PIDTYPE_PID].nr != 0;
+}
+
 extern void free_task(struct task_struct *tsk);
 extern void __put_task_struct(struct task_struct *tsk);
 #define get_task_struct(tsk) do { atomic_inc(&(tsk)->usage); } while(0)
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/