linux-kernel - [PATCH] perf_counter: Check task on counter read IPI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 14 Aug 2009 15:39:10 +1000
From:	Paul Mackerras <paulus@...ba.org>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	linux-kernel@...r.kernel.org
Subject: [PATCH] perf_counter: Check task on counter read IPI

In general, code in perf_counter.c that is called through an IPI
checks, for per-task counters, that the counter's task is still the
current task.  This is to handle the race condition where the cpu
switches from the task we want to another task in the interval between
sending the IPI and the IPI arriving and being handled on the target
CPU.

For some reason, __perf_counter_read is missing this check, yet there
is no reason why the race condition can't occur.  This adds a check
that the current task is the one we want.  If it isn't, we just
return.  In that case the counter->count value should be up to date,
since it will have been updated when the counter was scheduled out,
which must have happened since the IPI was sent.

Signed-off-by: Paul Mackerras <paulus@...ba.org>
---
I don't have an example of an actual failure due to this race, but it
seems obvious that it could occur and we need to guard against it, so
I think this should go in .31.

 kernel/perf_counter.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index 534e20d..b8fe739 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -1503,10 +1503,21 @@ static void perf_counter_enable_on_exec(struct task_struct *task)
  */
 static void __perf_counter_read(void *info)
 {
+	struct perf_cpu_context *cpuctx = &__get_cpu_var(perf_cpu_context);
 	struct perf_counter *counter = info;
 	struct perf_counter_context *ctx = counter->ctx;
 	unsigned long flags;
 
+	/*
+	 * If this is a task context, we need to check whether it is
+	 * the current task context of this cpu.  If not it has been
+	 * scheduled out before the smp call arrived.  In that case
+	 * counter->count would have been updated to a recent sample
+	 * when the counter was scheduled out.
+	 */
+	if (ctx->task && cpuctx->task_ctx != ctx)
+		return;
+
 	local_irq_save(flags);
 	if (ctx->is_active)
 		update_context_time(ctx);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/