Skip to content

Commit

Permalink
posix-cpu-timers: Rcu_read_lock/unlock protect find_task_by_vpid call
Browse files Browse the repository at this point in the history
Commit 4221a99 "Add RCU check for
find_task_by_vpid()" introduced rcu_lockdep_assert to find_task_by_pid_ns.
Add rcu_read_lock/rcu_read_unlock to call find_task_by_vpid.

Tetsuo Handa wrote:
| Quoting from one of posts in that thead
| http://kerneltrap.org/mailarchive/linux-kernel/2010/2/8/4536388
|
|| Usually tasklist gives enough protection, but if copy_process() fails
|| it calls free_pid() lockless and does call_rcu(delayed_put_pid().
|| This means, without rcu lock find_pid_ns() can't scan the hash table
|| safely.

Thomas Gleixner wrote:
| We can remove the tasklist_lock while at it. rcu_read_lock is enough.

Patch also replaces thread_group_leader with has_group_leader_pid
in accordance to comment by Oleg Nesterov:

| ... thread_group_leader() check is not relaible without 
| tasklist. If we race with de_thread() find_task_by_vpid() can find
| the new leader before it updates its ->group_leader.
|
| perhaps it makes sense to change posix_cpu_timer_create() to use 
| has_group_leader_pid() instead, just to make this code not look racy
| and avoid adding new problems.


Signed-off-by: Sergey Senozhatsky <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stanislaw Gruszka <[email protected]>
Reviewed-by: Oleg Nesterov <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
  • Loading branch information
sergey-senozhatsky authored and KAGA-KOKO committed Nov 10, 2010
1 parent 814ce25 commit c0deae8
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions kernel/posix-cpu-timers.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ static int check_clock(const clockid_t which_clock)
if (pid == 0)
return 0;

read_lock(&tasklist_lock);
rcu_read_lock();
p = find_task_by_vpid(pid);
if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
same_thread_group(p, current) : thread_group_leader(p))) {
same_thread_group(p, current) : has_group_leader_pid(p))) {
error = -EINVAL;
}
read_unlock(&tasklist_lock);
rcu_read_unlock();

return error;
}
Expand Down Expand Up @@ -390,7 +390,7 @@ int posix_cpu_timer_create(struct k_itimer *new_timer)

INIT_LIST_HEAD(&new_timer->it.cpu.entry);

read_lock(&tasklist_lock);
rcu_read_lock();
if (CPUCLOCK_PERTHREAD(new_timer->it_clock)) {
if (pid == 0) {
p = current;
Expand All @@ -404,7 +404,7 @@ int posix_cpu_timer_create(struct k_itimer *new_timer)
p = current->group_leader;
} else {
p = find_task_by_vpid(pid);
if (p && !thread_group_leader(p))
if (p && !has_group_leader_pid(p))
p = NULL;
}
}
Expand All @@ -414,7 +414,7 @@ int posix_cpu_timer_create(struct k_itimer *new_timer)
} else {
ret = -EINVAL;
}
read_unlock(&tasklist_lock);
rcu_read_unlock();

return ret;
}
Expand Down

0 comments on commit c0deae8

Please sign in to comment.