Commit b49a9e7e authored Jun 19, 2009 by Peter Zijlstra Committed by Ingo Molnar Jun 19, 2009

perf_counter: Close race in perf_lock_task_context()



perf_lock_task_context() is buggy because it can return a dead
context.

the RCU read lock in perf_lock_task_context() only guarantees
the memory won't get freed, it doesn't guarantee the object is
valid (in our case refcount > 0).

Therefore we can return a locked object that can get freed the
moment we release the rcu read lock.

perf_pin_task_context() then increases the refcount and does an
unlock on freed memory.

That increased refcount will cause a double free, in case it
started out with 0.

Ammend this by including the get_ctx() functionality in
perf_lock_task_context() (all users already did this later
anyway), and return a NULL context when the found one is
already dead.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
LKML-Reference: <new-submission>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

parent 0c871971

kernel/perf_counter.c

+5 −6

Original line number	Diff line number	Diff line
		@@ -175,6 +175,11 @@ perf_lock_task_context(struct task_struct task, unsigned long flags)
		spin_unlock_irqrestore(&ctx->lock, *flags);
		goto retry;
		}

		if (!atomic_inc_not_zero(&ctx->refcount)) {
		spin_unlock_irqrestore(&ctx->lock, *flags);
		ctx = NULL;
		}
		}
		rcu_read_unlock();
		return ctx;
		@@ -193,7 +198,6 @@ static struct perf_counter_context perf_pin_task_context(struct task_struct ta
		ctx = perf_lock_task_context(task, &flags);
		if (ctx) {
		++ctx->pin_count;
		get_ctx(ctx);
		spin_unlock_irqrestore(&ctx->lock, flags);
		}
		return ctx;
		@@ -1459,11 +1463,6 @@ static struct perf_counter_context *find_get_context(pid_t pid, int cpu)
		put_ctx(parent_ctx);
		ctx->parent_ctx = NULL; /* no longer a clone */
		}
		/*
		* Get an extra reference before dropping the lock so that
		* this context won't get freed if the task exits.
		*/
		get_ctx(ctx);
		spin_unlock_irqrestore(&ctx->lock, flags);
		}