| 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
| ||
|
Message-ID: <20140305080018.4be698f7@gandalf.local.home>
Date: Wed, 5 Mar 2014 08:00:18 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: mingo@...nel.org, hpa@...or.com, paulus@...ba.org,
linux-kernel@...r.kernel.org, acme@...stprotocols.net,
seiji.aguchi@....com, jolsa@...hat.com, vincent.weaver@...ne.edu,
tglx@...utronix.de, hpa@...ux.intel.com,
linux-tip-commits@...r.kernel.org
Subject: Re: [tip:x86/urgent] x86, trace: Fix CR2 corruption when tracing
page faults
On Wed, 5 Mar 2014 13:36:35 +0100
Peter Zijlstra <peterz@...radead.org> wrote:
> On Wed, Mar 05, 2014 at 01:25:35PM +0100, Peter Zijlstra wrote:
> > On Wed, Mar 05, 2014 at 07:20:22AM -0500, Steven Rostedt wrote:
> > > On Wed, 5 Mar 2014 12:14:15 +0100
> > > Peter Zijlstra <peterz@...radead.org> wrote:
> > >
> > > Please no! I used tracing of the do_page_fault function all the time.
> > > It is very useful.
> >
> > Why do you trace do_page_fault and not the __do_page_fault() function?
> > do_page_fault() is a minimal wrapper which doesn't actually do all that
> > much?
>
> Here, something like so; then you can still trace your do_page_fault().
>
> It also fixes up trace_page_fault_entries() to not re-read the cr2.
>
> ---
> arch/x86/include/asm/traps.h | 2 +-
> arch/x86/kernel/entry_32.S | 2 +-
> arch/x86/kernel/entry_64.S | 2 +-
> arch/x86/kernel/kvm.c | 2 +-
> arch/x86/mm/fault.c | 21 ++++++++++-----------
> 5 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
> index 58d66fe06b61..1280f72deea8 100644
> --- a/arch/x86/include/asm/traps.h
> +++ b/arch/x86/include/asm/traps.h
> @@ -71,7 +71,7 @@ dotraplinkage void do_double_fault(struct pt_regs *, long);
> asmlinkage __kprobes struct pt_regs *sync_regs(struct pt_regs *);
> #endif
> dotraplinkage void do_general_protection(struct pt_regs *, long);
> -dotraplinkage void do_page_fault(struct pt_regs *, unsigned long);
> +dotraplinkage void normal_do_page_fault(struct pt_regs *, unsigned long);
Why not call it "do_do_page_fault()" ;-)
> #ifdef CONFIG_TRACING
> dotraplinkage void trace_do_page_fault(struct pt_regs *, unsigned long);
> #endif
> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> index a2a4f4697889..9a9f64755da8 100644
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -1257,7 +1257,7 @@ END(trace_page_fault)
> ENTRY(page_fault)
> RING0_EC_FRAME
> ASM_CLAC
> - pushl_cfi $do_page_fault
> + pushl_cfi $normal_do_page_fault
> ALIGN
> error_code:
> /* the function address is in %gs's slot on the stack */
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 1e96c3628bf2..7d49812741ac 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -1491,7 +1491,7 @@ zeroentry xen_int3 do_int3
> errorentry xen_stack_segment do_stack_segment
> #endif
> errorentry general_protection do_general_protection
> -trace_errorentry page_fault do_page_fault
> +trace_errorentry page_fault normal_do_page_fault
> #ifdef CONFIG_KVM_GUEST
> errorentry async_page_fault do_async_page_fault
> #endif
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 713f1b3bad52..9e7db22ec437 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -259,7 +259,7 @@ do_async_page_fault(struct pt_regs *regs, unsigned long error_code)
>
> switch (kvm_read_and_reset_pf_reason()) {
> default:
> - do_page_fault(regs, error_code);
> + normal_do_page_fault(regs, error_code);
> break;
> case KVM_PV_REASON_PAGE_NOT_PRESENT:
> /* page is swapped out by the host. */
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index e7fa28bf3262..576cfc3d0086 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -1022,8 +1022,7 @@ static inline bool smap_violation(int error_code, struct pt_regs *regs)
> * routines.
> */
> static void __kprobes
Still requires the "noinline" above
-- Steve
> -__do_page_fault(struct pt_regs *regs, unsigned long error_code,
> - unsigned long address)
> +do_page_fault(struct pt_regs *regs, unsigned long error_code, unsigned long address)
> {
> struct vm_area_struct *vma;
> struct task_struct *tsk;
> @@ -1245,28 +1244,28 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
> up_read(&mm->mmap_sem);
> }
>
> -dotraplinkage void __kprobes
> -do_page_fault(struct pt_regs *regs, unsigned long error_code)
> +dotraplinkage void __kprobes notrace
> +normal_do_page_fault(struct pt_regs *regs, unsigned long error_code)
> {
> enum ctx_state prev_state;
> /* Get the faulting address: */
> unsigned long address = read_cr2();
>
> prev_state = exception_enter();
> - __do_page_fault(regs, error_code, address);
> + do_page_fault(regs, error_code, address);
> exception_exit(prev_state);
> }
>
> -static void trace_page_fault_entries(struct pt_regs *regs,
> +static void trace_page_fault_entries(unsigned long address, struct pt_regs *regs,
> unsigned long error_code)
> {
> if (user_mode(regs))
> - trace_page_fault_user(read_cr2(), regs, error_code);
> + trace_page_fault_user(address, regs, error_code);
> else
> - trace_page_fault_kernel(read_cr2(), regs, error_code);
> + trace_page_fault_kernel(address, regs, error_code);
> }
>
> -dotraplinkage void __kprobes
> +dotraplinkage void __kprobes notrace
> trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
> {
> enum ctx_state prev_state;
> @@ -1279,7 +1278,7 @@ trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
> unsigned long address = read_cr2();
>
> prev_state = exception_enter();
> - trace_page_fault_entries(regs, error_code);
> - __do_page_fault(regs, error_code, address);
> + trace_page_fault_entries(address, regs, error_code);
> + do_page_fault(regs, error_code, address);
> exception_exit(prev_state);
> }
--
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/
Powered by blists - more mailing lists