Skip to content

Commit

Permalink
Fix unlikely (but possible) race condition on task->user access
Browse files Browse the repository at this point in the history
There's a possible race condition when doing a "switch_uid()" from one
user to another, which could race with another thread doing a signal
allocation and looking at the old thread ->user pointer as it is freed.

This explains an oops reported by Lukasz Trabinski:
	http://permalink.gmane.org/gmane.linux.kernel/462241

We fix this by delaying the (reference-counted) freeing of the user
structure until the thread signal handler lock has been released, so
that we know that the signal allocation has either seen the new value or
has properly incremented the reference count of the old one.

Race identified by Oleg Nesterov.

Cc: Lukasz Trabinski <lukasz@wsisiz.edu.pl>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Cc: Andrew Morton <akpm@osdl.org>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
  • Loading branch information
Linus Torvalds committed Nov 4, 2006
1 parent 80491eb commit 45c18b0
Showing 1 changed file with 11 additions and 0 deletions.
11 changes: 11 additions & 0 deletions kernel/user.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,17 @@ void switch_uid(struct user_struct *new_user)
atomic_dec(&old_user->processes);
switch_uid_keyring(new_user);
current->user = new_user;

/*
* We need to synchronize with __sigqueue_alloc()
* doing a get_uid(p->user).. If that saw the old
* user value, we need to wait until it has exited
* its critical region before we can free the old
* structure.
*/
smp_mb();
spin_unlock_wait(&current->sighand->siglock);

free_uid(old_user);
suid_keys(current);
}
Expand Down

0 comments on commit 45c18b0

Please sign in to comment.