linux-kernel - Re: [patch 1/21] x86, bts: fix race when bts tracer is removed

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 1 Apr 2009 13:34:38 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	"Metzger, Markus T" <markus.t.metzger@...el.com>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"hpa@...or.com" <hpa@...or.com>,
	"markus.t.metzger@...il.com" <markus.t.metzger@...il.com>,
	"roland@...hat.com" <roland@...hat.com>,
	"eranian@...glemail.com" <eranian@...glemail.com>,
	"Villacis, Juan" <juan.villacis@...el.com>,
	"ak@...ux.jf.intel.com" <ak@...ux.jf.intel.com>
Subject: Re: [patch 1/21] x86, bts: fix race when bts tracer is removed


* Metzger, Markus T <markus.t.metzger@...el.com> wrote:

> >-----Original Message-----
> >From: Oleg Nesterov [mailto:oleg@...hat.com]
> >Sent: Wednesday, April 01, 2009 1:48 AM
> >To: Metzger, Markus T
> >Cc: linux-kernel@...r.kernel.org; mingo@...e.hu; tglx@...utronix.de; hpa@...or.com;
> >markus.t.metzger@...il.com; roland@...hat.com; eranian@...glemail.com; Villacis, Juan;
> >ak@...ux.jf.intel.com
> >Subject: Re: [patch 1/21] x86, bts: fix race when bts tracer is removed
> >
> >On 03/31, Markus Metzger wrote:
> >>
> >> Read the tracer once during a context switch.
> >> ...
> >> @@ -1044,36 +1051,39 @@ void ds_switch_to(struct task_struct *pr
> >>  {
> >>  	struct ds_context *prev_ctx = prev->thread.ds_ctx;
> >>  	struct ds_context *next_ctx = next->thread.ds_ctx;
> >> +	unsigned long debugctlmsr = next->thread.debugctlmsr;
> >>
> >>  	if (prev_ctx) {
> >> +		struct bts_tracer *tracer = prev_ctx->bts_master;
> >> +
> >>  		update_debugctlmsr(0);
> >>
> >> -		if (prev_ctx->bts_master &&
> >> -		    (prev_ctx->bts_master->trace.ds.flags & BTS_TIMESTAMPS)) {
> >> +		if (tracer && (tracer->flags & BTS_TIMESTAMPS)) {
> >
> >In theory, we need barrier() after reading ->bts_master.
> >
> >(actually, I did see the bug reports when the compiler read the pointer
> > twice with the code like above).
> 
> I guess the same is true for prev_ctx, next_ctx, and debugctlmsr, then.
> 
> Ingo,
> would it be OK to resend this one patch with the barrier()s added?

Sure - but note that i have put the series on hold until you get 
broad Ack's from Oleg for the ptrace bits. Please fix the review 
feedback from Oleg and propagate his acks into the commit logs as 
well. Oleg is finding bugs we missed in the past so his review work 
is very valuable.

Also - minor patch submission technicality observation: currently 
each of your mails goes into a separate discussion thread, making it 
hard to review them as a group.

The preferred way to send such series is to use "git format-patch" + 
"git send-email". (That will give a nice 0/21 mail and a properly 
threaded discussion with proper References header lines.)

Thanks,

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