[PATCH] simplify/fix first_tid()

first_tid:

	/* If nr exceeds the number of threads there is nothing todo */
	if (nr) {
		if (nr >= get_nr_threads(leader))
			goto done;
	}

This is not reliable: sub-threads can exit after this check, so the
'for' loop below can overlap and proc_task_readdir() can return an
already filldir'ed dirents.

	for (; pos && pid_alive(pos); pos = next_thread(pos)) {
		if (--nr > 0)
			continue;

Off-by-one error, will return 'leader' when nr == 1.

This patch tries to fix these problems and simplify the code.

Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
This commit is contained in:
Oleg Nesterov 2006-06-26 00:26:01 -07:00 committed by Linus Torvalds
parent cc288738c9
commit a872ff0cb2

View file

@ -2227,38 +2227,34 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
static struct task_struct *first_tid(struct task_struct *leader,
int tid, int nr)
{
struct task_struct *pos = NULL;
struct task_struct *pos;
rcu_read_lock();
/* Attempt to start with the pid of a thread */
if (tid && (nr > 0)) {
pos = find_task_by_pid(tid);
if (pos && (pos->group_leader != leader))
pos = NULL;
if (pos)
nr = 0;
if (pos && (pos->group_leader == leader))
goto found;
}
/* If nr exceeds the number of threads there is nothing todo */
if (nr) {
if (nr >= get_nr_threads(leader))
goto done;
}
/* If we haven't found our starting place yet start with the
* leader and walk nr threads forward.
*/
if (!pos && (nr >= 0))
pos = leader;
for (; pos && pid_alive(pos); pos = next_thread(pos)) {
if (--nr > 0)
continue;
get_task_struct(pos);
goto done;
}
pos = NULL;
done:
if (nr && nr >= get_nr_threads(leader))
goto out;
/* If we haven't found our starting place yet start
* with the leader and walk nr threads forward.
*/
for (pos = leader; nr > 0; --nr) {
pos = next_thread(pos);
if (pos == leader) {
pos = NULL;
goto out;
}
}
found:
get_task_struct(pos);
out:
rcu_read_unlock();
return pos;
}