linux-kernel - Re: [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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1250497016.5241.1675.camel@twins>
Date:	Mon, 17 Aug 2009 10:16:56 +0200
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Paul Mackerras <paulus@...ba.org>
Cc:	Ingo Molnar <mingo@...e.hu>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf_counter: Check task on counter read IPI

On Fri, 2009-08-14 at 15:39 +1000, Paul Mackerras wrote:
> 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.

Hmm, right.

However those other sites have retry loops in the caller, but callers of
__perf_counter_read() do not. Granted, I'm not sure what they should
retry on exactly, but this patch trades an invalid update to a missing
update.

While I think the balance tips towards favouring the missing update, its
not really much of an improvement.

I guess we could keep a sequence count with the update and loop until it
gets increased or something?


--
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/