[PATCH] use pid_alive in proc_pid_status

From: Manfred Spraul
Date: Sun Nov 28 2004 - 06:30:36 EST


Hi,

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.

What do you think? Are you aware of further instances where p->pid is still used to check if a thread is alive?

Signed-Off-By: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>

// $Header$
// Kernel Version:
// VERSION = 2
// PATCHLEVEL = 6
// SUBLEVEL = 10
// EXTRAVERSION =-rc2
--- 2.6/include/linux/pid.h 2004-10-23 09:58:17.000000000 +0200
+++ build-2.6/include/linux/pid.h 2004-11-28 12:07:55.514992845 +0100
@@ -52,4 +52,6 @@
hlist_unhashed(&(task)->pids[type].pid_chain)); \
} \

+extern int pid_alive(struct task_struct *p);
+
#endif /* _LINUX_PID_H */
--- 2.6/kernel/pid.c 2004-11-19 18:54:37.000000000 +0100
+++ build-2.6/kernel/pid.c 2004-11-28 12:09:07.464302391 +0100
@@ -247,6 +247,19 @@
attach_pid(leader, PIDTYPE_SID, leader->signal->session);
}

+/**
+ * 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.
+ */
+int pid_alive(struct task_struct *p)
+{
+ return p->pids[PIDTYPE_PID].nr != 0;
+}
+
/*
* The pid hash table is scaled according to the amount of memory in the
* machine. From a minimum of 16 slots up to 4096 slots at one gigabyte or
--- 2.6/fs/proc/base.c 2004-11-19 18:54:34.000000000 +0100
+++ build-2.6/fs/proc/base.c 2004-11-28 12:06:49.259448232 +0100
@@ -780,11 +780,6 @@
.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)
--- 2.6/fs/proc/array.c 2004-11-19 18:54:34.000000000 +0100
+++ build-2.6/fs/proc/array.c 2004-11-28 12:00:17.944726203 +0100
@@ -171,8 +171,8 @@
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);