]> git.karo-electronics.de Git - karo-tx-linux.git/commit
proc: fix the potential use-after-free in first_tid()
authorOleg Nesterov <oleg@redhat.com>
Fri, 3 Jan 2014 03:10:28 +0000 (14:10 +1100)
committerStephen Rothwell <sfr@canb.auug.org.au>
Fri, 3 Jan 2014 03:10:28 +0000 (14:10 +1100)
commitefd654713c66d097cc463d7e66a364fca20cf5a6
tree69073da5ea3a9f13e3f0abcbef2b7e96a57ed7be
parent08d9b17a38bc1b6e29b477dedd2eddd8476ea351
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>
fs/proc/base.c