linux-kernel - Re: [PATCHv2] perf x86_64: Fix rsp register for system call fast path

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: <20121002161633.GF10605@somewhere>
Date:	Tue, 2 Oct 2012 18:16:47 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Paul Mackerras <paulus@...ba.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCHv2] perf x86_64: Fix rsp register for system call fast path

On Tue, Oct 02, 2012 at 06:06:26PM +0200, Jiri Olsa wrote:
> On Tue, Oct 02, 2012 at 05:49:26PM +0200, Frederic Weisbecker wrote:
> > On Tue, Oct 02, 2012 at 04:58:15PM +0200, Jiri Olsa wrote:
> > > diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> > > index 915b876..11d62ff 100644
> > > --- a/arch/x86/kernel/cpu/perf_event.c
> > > +++ b/arch/x86/kernel/cpu/perf_event.c
> > > @@ -34,6 +34,7 @@
> > >  #include <asm/timer.h>
> > >  #include <asm/desc.h>
> > >  #include <asm/ldt.h>
> > > +#include <asm/syscall.h>
> > >  
> > >  #include "perf_event.h"
> > >  
> > > @@ -1699,6 +1700,52 @@ void arch_perf_update_userpage(struct perf_event_mmap_page *userpg, u64 now)
> > >  	userpg->time_offset = this_cpu_read(cyc2ns_offset) - now;
> > >  }
> > >  
> > > +#ifdef CONFIG_X86_64
> > > +__weak int arch_sample_regs_user(struct pt_regs *oregs, struct pt_regs *regs)
> > > +{
> > > +	int kernel = !user_mode(regs);
> > > +
> > > +	if (kernel) {
> > > +		if (current->mm)
> > > +			regs = task_pt_regs(current);
> > > +		else
> > > +			regs = NULL;
> > > +	}
> > 
> > Shouldn't the above stay in generic code?
> 
> could be.. I guess I thought that having the regs retrieval
> plus the fixup at the same place feels better/compact ;)
> 
> but could change that if needed

Yeah please.

> > 
> > I'm trying to scratch my head to find a solution to detect the race and
> > bail out instead of recording erroneous values but I can't find one.
> > 
> > Anyway this is still better than what we have now.
> > 
> > Another solution could be to force syscall slow path and have some variable
> > set there that tells us we are in a syscall and every regs have been saved.
> > 
> > But we probably don't want to force syscall slow path...
> 
> I was trying something like that as well, but the one I sent looks
> far less hacky to me.. :)

Actually it's more hacky because it's less deterministic.
But it's more simple, and doesn't hurt performances.

Ok, let's start with that.
--
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/