Commit 10b1fbdb authored Nov 04, 2006 by Linus Torvalds

Make sure "user->sigpending" count is in sync



The previous commit (45c18b0b, aka "Fix
unlikely (but possible) race condition on task->user access") fixed a
potential oops due to __sigqueue_alloc() getting its "user" pointer out
of sync with switch_user(), and accessing a user pointer that had been
de-allocated on another CPU.

It still left another (much less serious) problem, where a concurrent
__sigqueue_alloc and swich_user could cause sigqueue_alloc to do signal
pending reference counting for a _different_ user than the one it then
actually ended up using.  No oops, but we'd end up with the wrong signal
accounting.

Another case of Oleg's eagle-eyes picking up the problem.

This is trivially fixed by just making sure we load whichever "user"
structure we decide to use (it doesn't matter _which_ one we pick, we
just need to pick one) just once.

Acked-by: 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>

parent 45c18b0b

kernel/signal.c

+11 −4

Original line number	Diff line number	Diff line
		@@ -267,18 +267,25 @@ static struct sigqueue __sigqueue_alloc(struct task_struct t, gfp_t flags,
		int override_rlimit)
		{
		struct sigqueue *q = NULL;
		struct user_struct *user;

		atomic_inc(&t->user->sigpending);
		/*
		* In order to avoid problems with "switch_user()", we want to make
		* sure that the compiler doesn't re-load "t->user"
		*/
		user = t->user;
		barrier();
		atomic_inc(&user->sigpending);
		if (override_rlimit \|\|
		atomic_read(&t->user->sigpending) <=
		atomic_read(&user->sigpending) <=
		t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
		q = kmem_cache_alloc(sigqueue_cachep, flags);
		if (unlikely(q == NULL)) {
		atomic_dec(&t->user->sigpending);
		atomic_dec(&user->sigpending);
		} else {
		INIT_LIST_HEAD(&q->list);
		q->flags = 0;
		q->user = get_uid(t->user);
		q->user = get_uid(user);
		}
		return(q);
		}