proc_task_readdir() verifies that the result of get_proc_task()
is pid_alive() and thus its ->group_leader is fine too. However
this is not necessarily true after rcu_read_unlock(), we need
to recheck this after first_tid() does rcu_read_lock() again.
The race is subtle and unlikely, but still it is possible afaics.
To simplify lets ignore the "likely" case when tid != 0, f_version
can be cleared by proc_task_operations->llseek().
Suppose we have a main thread M and its subthread T. Suppose that
f_pos == 3, iow first_tid() should return T. Now suppose that the
following happens between rcu_read_unlock() and rcu_read_lock():
1. T execs and becomes the new leader. This removes M from
->thread_group but next_thread(M) is still T.
2. T creates another thread X which does exec as well, T
goes away.
3. X creates another subthread, this increments nr_threads.
4. first_tid() does next_thread(M) and returns the already
dead T.
Note that we need 2. and 3. only because of get_nr_threads() check,
and this check was supposed to be optimization only.
Note: I think that proc_task_readdir/first_tid interaction can be
simplified, but this needs another patch. proc_task_readdir() should
not play with ->group_leader at all. See the next patches.
Signed-off-by: Oleg Nesterov <o...@redhat.com>
---
fs/proc/base.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index dd51e50..c939c9f 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3186,10 +3186,13 @@ static struct task_struct *first_tid(struct task_struct
*leader,
goto found;
}
- /* If nr exceeds the number of threads there is nothing todo */
pos = NULL;
+ /* If nr exceeds the number of threads there is nothing todo */
if (nr && nr >= get_nr_threads(leader))
goto out;
+ /* It could be unhashed before we take rcu lock */
+ if (!pid_alive(leader))
+ goto out;
/* If we haven't found our starting place yet start
* with the leader and walk nr threads forward.
--
1.5.5.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
