summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOleg Nesterov <oleg@redhat.com>2014-01-23 15:55:36 -0800
committerLinus Torvalds <torvalds@linux-foundation.org>2014-01-23 16:37:01 -0800
commit940fe4793a219375c4713a17c61b843720807c9d (patch)
treea000faaf44f785c18e519e5baaaf7b97d54913ee
parent74e37200de8e9c4e09b70c21c3f13c2071e77457 (diff)
proc: fix the potential use-after-free in first_tid()
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 again after first_tid() does rcu_read_lock(). Otherwise leader->thread_group.next (used by next_thread()) can be invalid if the rcu grace period expires in between. 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 also that we need 2. and 3. only because of get_nr_threads() check, and this check was supposed to be optimization only. Signed-off-by: Oleg Nesterov <oleg@redhat.com> Cc: "Eric W. Biederman" <ebiederm@xmission.com> Cc: Michal Hocko <mhocko@suse.cz> Cc: Sameer Nanda <snanda@chromium.org> Cc: Sergey Dyasly <dserrg@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--fs/proc/base.c3
1 files changed, 3 insertions, 0 deletions
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 03c8d747be48..f223a56e613c 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3109,6 +3109,9 @@ static struct task_struct *first_tid(struct task_struct *leader,
pos = NULL;
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.