linux-kernel - Re: [PATCH] clear the frame pointer in copy

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] [day] [month] [year] [list]

Message-ID: <1340155695.7225.6.camel@ymzhang.sh.intel.com>
Date:	Wed, 20 Jun 2012 09:28:15 +0800
From:	Yanmin Zhang <yanmin_zhang@...ux.intel.com>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	"Su, Xuemin" <xuemin.su@...el.com>, akpm@...ux-foundation.org,
	mingo@...e.hu, a.p.zijlstra@...llo.nl, rusty@...tcorp.com.au,
	william.douglas@...el.com, tglx@...utronix.de, mingo@...hat.com,
	x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clear the frame pointer in copy_thread

On Tue, 2012-06-19 at 06:57 -0700, H. Peter Anvin wrote:
> It isn't clear to me this is the right thing to do this in the kernel.  A frame pointer isn't obligatory and doing this in user space seems more appropriate.
Peter,

Thanks for the kind comments. You are right.

Yanmin

> 
> "Su, Xuemin" <xuemin.su@...el.com> wrote:
> 
> >From: xueminsu <xuemin.su@...el.com>
> >
> >When creating a userspace thread, kernel copies all parent
> >thread’s registers to the child thread, including bp register
> >which is used to create stack frame. At this point, the bp
> >register is pointing to the parent thread’s last stack frame.
> >
> >So, after the child thread is created, its bp register points
> >to the parent thread’s stack area, and the value would be pushed
> >to its own stack to form its first stack frame when it calls its
> >first function.
> >
> >This would not be a problem if the child thread and the parent
> >thread do not share the VM or if they share the VM and have the
> >same stack. But if they share the VM and have different stacks,
> >this would cause an issue: when we try to print the child thread’s
> >stack trace, we would eventually walk into the parent thread’s
> >stack area, and sometimes we will occasionally prints out the
> >parent thread’s stack trace.
> >
> >Currently some version of libc clears the frame pointer just after
> >the thread is created, but this is not guaranteed.
> >
> >Signed-off-by: xueminsu <xuemin.su@...el.com>
> >---
> > arch/x86/kernel/process_32.c |    9 +++++++++
> > arch/x86/kernel/process_64.c |    9 +++++++++
> > 2 files changed, 18 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/x86/kernel/process_32.c
> >b/arch/x86/kernel/process_32.c
> >index 516fa18..b1b6d42 100644
> >--- a/arch/x86/kernel/process_32.c
> >+++ b/arch/x86/kernel/process_32.c
> >@@ -139,6 +139,15 @@ int copy_thread(unsigned long clone_flags,
> >unsigned long sp,
> > 	childregs->ax = 0;
> > 	childregs->sp = sp;
> > 
> >+	/*
> >+	 * If the child thread and the parent thread share the VM
> >+	 * but have different stack, the child thread's bp register
> >+	 * should be set to null, preventing it from pointing to
> >+	 * the parent thread's stack area.
> >+	 */
> >+	if ((sp != regs->sp) && (clone_flags & CLONE_VM))
> >+		childregs->bp = 0;
> >+
> > 	p->thread.sp = (unsigned long) childregs;
> > 	p->thread.sp0 = (unsigned long) (childregs+1);
> > 
> >diff --git a/arch/x86/kernel/process_64.c
> >b/arch/x86/kernel/process_64.c
> >index 61cdf7f..5acff83 100644
> >--- a/arch/x86/kernel/process_64.c
> >+++ b/arch/x86/kernel/process_64.c
> >@@ -163,6 +163,15 @@ int copy_thread(unsigned long clone_flags,
> >unsigned long sp,
> > 	else
> > 		childregs->sp = (unsigned long)childregs;
> > 
> >+	/*
> >+	 * If the child thread and the parent thread share the VM
> >+	 * but have different stack, the child thread's bp register
> >+	 * should be set to null, preventing it from pointing to
> >+	 * the parent thread's stack area.
> >+	 */
> >+	if ((sp != regs->sp) && (clone_flags & CLONE_VM))
> >+		childregs->bp = 0;
> >+
> > 	p->thread.sp = (unsigned long) childregs;
> > 	p->thread.sp0 = (unsigned long) (childregs+1);
> > 	p->thread.usersp = me->thread.usersp;
> >-- 
> >1.7.6
> 


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