Bug 140523

Summary: [kernel] sparc {set,swap}context calls trash TLS register %g7
Product: Base System Reporter: Nathaniel Filardo <nwf>
Component: sparc64Assignee: freebsd-sparc64 (Nobody) <sparc64>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 9.0-CURRENT   
Hardware: Any   
OS: Any   

Description Nathaniel Filardo 2009-11-13 09:00:07 UTC
The FreeBSD sparc64 implementation of {set,swap}context() will trash the Thread Local Storage register (%g7), making migration of contexts across threads unsafe.  This breaks plan9port, at least.

The same problem existed in Linux; see http://sourceware.org/bugzilla/show_bug.cgi?id=6577 for my report there.

Fix: 

Don't restore %g7 from the stored context.
How-To-Repeat: Attempt to setcontext() in one pthread the result of getcontext() in another thread.  Suddenly pthread_self() will return as if running on the other thread.
Comment 1 marius 2009-11-14 16:17:11 UTC
Could you please test whether the following patch fixes your
problem?
http://people.freebsd.org/~marius/sparc64_mcontext_unroll_skip_g7.diff

Marius
Comment 2 Nathaniel Filardo 2009-11-15 02:52:01 UTC
On Sat, Nov 14, 2009 at 05:17:11PM +0100, Marius Strobl wrote:
> 
> Could you please test whether the following patch fixes your
> problem?
> http://people.freebsd.org/~marius/sparc64_mcontext_unroll_skip_g7.diff


Appears to work wonderfully.  Thanks.
--nwf;
Comment 3 dfilter service freebsd_committer freebsd_triage 2009-11-17 21:08:21 UTC
Author: marius
Date: Tue Nov 17 21:08:10 2009
New Revision: 199442
URL: http://svn.freebsd.org/changeset/base/199442

Log:
  Unroll copying of the registers in {g,s}et_mcontext() and limit it
  to the set actually restored by tl0_ret() instead of using the whole
  trapframe. Additionally skip %g7 as that register is used as the
  userland TLS pointer.
  
  PR:		140523
  MFC after:	1 week

Modified:
  head/sys/sparc64/sparc64/machdep.c

Modified: head/sys/sparc64/sparc64/machdep.c
==============================================================================
--- head/sys/sparc64/sparc64/machdep.c	Tue Nov 17 21:00:02 2009	(r199441)
+++ head/sys/sparc64/sparc64/machdep.c	Tue Nov 17 21:08:10 2009	(r199442)
@@ -692,12 +692,39 @@ get_mcontext(struct thread *td, mcontext
 
 	tf = td->td_frame;
 	pcb = td->td_pcb;
-	bcopy(tf, mc, sizeof(*tf));
+	/*
+	 * Copy the registers which will be restored by tl0_ret() from the
+	 * trapframe.
+	 * Note that we skip %g7 which is used as the userland TLS register
+	 * and %wstate.
+	 */
+	mc->mc_flags = _MC_VERSION;
+	mc->mc_global[1] = tf->tf_global[1];
+	mc->mc_global[2] = tf->tf_global[2];
+	mc->mc_global[3] = tf->tf_global[3];
+	mc->mc_global[4] = tf->tf_global[4];
+	mc->mc_global[5] = tf->tf_global[5];
+	mc->mc_global[6] = tf->tf_global[6];
 	if (flags & GET_MC_CLEAR_RET) {
 		mc->mc_out[0] = 0;
 		mc->mc_out[1] = 0;
+	} else {
+		mc->mc_out[0] = tf->tf_out[0];
+		mc->mc_out[1] = tf->tf_out[1];
 	}
-	mc->mc_flags = _MC_VERSION;
+	mc->mc_out[2] = tf->tf_out[2];
+	mc->mc_out[3] = tf->tf_out[3];
+	mc->mc_out[4] = tf->tf_out[4];
+	mc->mc_out[5] = tf->tf_out[5];
+	mc->mc_out[6] = tf->tf_out[6];
+	mc->mc_out[7] = tf->tf_out[7];
+	mc->mc_fprs = tf->tf_fprs;
+	mc->mc_fsr = tf->tf_fsr;
+	mc->mc_gsr = tf->tf_gsr;
+	mc->mc_tnpc = tf->tf_tnpc;
+	mc->mc_tpc = tf->tf_tpc;
+	mc->mc_tstate = tf->tf_tstate;
+	mc->mc_y = tf->tf_y;
 	critical_enter();
 	if ((tf->tf_fprs & FPRS_FEF) != 0) {
 		savefpctx(pcb->pcb_ufp);
@@ -717,7 +744,6 @@ set_mcontext(struct thread *td, const mc
 {
 	struct trapframe *tf;
 	struct pcb *pcb;
-	uint64_t wstate;
 
 	if (!TSTATE_SECURE(mc->mc_tstate) ||
 	    (mc->mc_flags & ((1L << _MC_VERSION_BITS) - 1)) != _MC_VERSION)
@@ -726,9 +752,33 @@ set_mcontext(struct thread *td, const mc
 	pcb = td->td_pcb;
 	/* Make sure the windows are spilled first. */
 	flushw();
-	wstate = tf->tf_wstate;
-	bcopy(mc, tf, sizeof(*tf));
-	tf->tf_wstate = wstate;
+	/*
+	 * Copy the registers which will be restored by tl0_ret() to the
+	 * trapframe.
+	 * Note that we skip %g7 which is used as the userland TLS register
+	 * and %wstate.
+	 */
+	tf->tf_global[1] = mc->mc_global[1];
+	tf->tf_global[2] = mc->mc_global[2];
+	tf->tf_global[3] = mc->mc_global[3];
+	tf->tf_global[4] = mc->mc_global[4];
+	tf->tf_global[5] = mc->mc_global[5];
+	tf->tf_global[6] = mc->mc_global[6];
+	tf->tf_out[0] = mc->mc_out[0];
+	tf->tf_out[1] = mc->mc_out[1];
+	tf->tf_out[2] = mc->mc_out[2];
+	tf->tf_out[3] = mc->mc_out[3];
+	tf->tf_out[4] = mc->mc_out[4];
+	tf->tf_out[5] = mc->mc_out[5];
+	tf->tf_out[6] = mc->mc_out[6];
+	tf->tf_out[7] = mc->mc_out[7];
+	tf->tf_fprs = mc->mc_fprs;
+	tf->tf_fsr = mc->mc_fsr;
+	tf->tf_gsr = mc->mc_gsr;
+	tf->tf_tnpc = mc->mc_tnpc;
+	tf->tf_tpc = mc->mc_tpc;
+	tf->tf_tstate = mc->mc_tstate;
+	tf->tf_y = mc->mc_y;
 	if ((mc->mc_fprs & FPRS_FEF) != 0) {
 		tf->tf_fprs = 0;
 		bcopy(mc->mc_fp, pcb->pcb_ufp, sizeof(pcb->pcb_ufp));
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 4 dfilter service freebsd_committer freebsd_triage 2009-11-24 20:04:41 UTC
Author: marius
Date: Tue Nov 24 20:04:31 2009
New Revision: 199765
URL: http://svn.freebsd.org/changeset/base/199765

Log:
  MFC: r199442
  
  Unroll copying of the registers in {g,s}et_mcontext() and limit it
  to the set actually restored by tl0_ret() instead of using the whole
  trapframe. Additionally skip %g7 as that register is used as the
  userland TLS pointer.
  
  PR:		140523

Modified:
  stable/8/sys/sparc64/sparc64/machdep.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/xen/xenpci/   (props changed)

Modified: stable/8/sys/sparc64/sparc64/machdep.c
==============================================================================
--- stable/8/sys/sparc64/sparc64/machdep.c	Tue Nov 24 19:57:41 2009	(r199764)
+++ stable/8/sys/sparc64/sparc64/machdep.c	Tue Nov 24 20:04:31 2009	(r199765)
@@ -696,12 +696,39 @@ get_mcontext(struct thread *td, mcontext
 
 	tf = td->td_frame;
 	pcb = td->td_pcb;
-	bcopy(tf, mc, sizeof(*tf));
+	/*
+	 * Copy the registers which will be restored by tl0_ret() from the
+	 * trapframe.
+	 * Note that we skip %g7 which is used as the userland TLS register
+	 * and %wstate.
+	 */
+	mc->mc_flags = _MC_VERSION;
+	mc->mc_global[1] = tf->tf_global[1];
+	mc->mc_global[2] = tf->tf_global[2];
+	mc->mc_global[3] = tf->tf_global[3];
+	mc->mc_global[4] = tf->tf_global[4];
+	mc->mc_global[5] = tf->tf_global[5];
+	mc->mc_global[6] = tf->tf_global[6];
 	if (flags & GET_MC_CLEAR_RET) {
 		mc->mc_out[0] = 0;
 		mc->mc_out[1] = 0;
+	} else {
+		mc->mc_out[0] = tf->tf_out[0];
+		mc->mc_out[1] = tf->tf_out[1];
 	}
-	mc->mc_flags = _MC_VERSION;
+	mc->mc_out[2] = tf->tf_out[2];
+	mc->mc_out[3] = tf->tf_out[3];
+	mc->mc_out[4] = tf->tf_out[4];
+	mc->mc_out[5] = tf->tf_out[5];
+	mc->mc_out[6] = tf->tf_out[6];
+	mc->mc_out[7] = tf->tf_out[7];
+	mc->mc_fprs = tf->tf_fprs;
+	mc->mc_fsr = tf->tf_fsr;
+	mc->mc_gsr = tf->tf_gsr;
+	mc->mc_tnpc = tf->tf_tnpc;
+	mc->mc_tpc = tf->tf_tpc;
+	mc->mc_tstate = tf->tf_tstate;
+	mc->mc_y = tf->tf_y;
 	critical_enter();
 	if ((tf->tf_fprs & FPRS_FEF) != 0) {
 		savefpctx(pcb->pcb_ufp);
@@ -721,7 +748,6 @@ set_mcontext(struct thread *td, const mc
 {
 	struct trapframe *tf;
 	struct pcb *pcb;
-	uint64_t wstate;
 
 	if (!TSTATE_SECURE(mc->mc_tstate) ||
 	    (mc->mc_flags & ((1L << _MC_VERSION_BITS) - 1)) != _MC_VERSION)
@@ -730,9 +756,33 @@ set_mcontext(struct thread *td, const mc
 	pcb = td->td_pcb;
 	/* Make sure the windows are spilled first. */
 	flushw();
-	wstate = tf->tf_wstate;
-	bcopy(mc, tf, sizeof(*tf));
-	tf->tf_wstate = wstate;
+	/*
+	 * Copy the registers which will be restored by tl0_ret() to the
+	 * trapframe.
+	 * Note that we skip %g7 which is used as the userland TLS register
+	 * and %wstate.
+	 */
+	tf->tf_global[1] = mc->mc_global[1];
+	tf->tf_global[2] = mc->mc_global[2];
+	tf->tf_global[3] = mc->mc_global[3];
+	tf->tf_global[4] = mc->mc_global[4];
+	tf->tf_global[5] = mc->mc_global[5];
+	tf->tf_global[6] = mc->mc_global[6];
+	tf->tf_out[0] = mc->mc_out[0];
+	tf->tf_out[1] = mc->mc_out[1];
+	tf->tf_out[2] = mc->mc_out[2];
+	tf->tf_out[3] = mc->mc_out[3];
+	tf->tf_out[4] = mc->mc_out[4];
+	tf->tf_out[5] = mc->mc_out[5];
+	tf->tf_out[6] = mc->mc_out[6];
+	tf->tf_out[7] = mc->mc_out[7];
+	tf->tf_fprs = mc->mc_fprs;
+	tf->tf_fsr = mc->mc_fsr;
+	tf->tf_gsr = mc->mc_gsr;
+	tf->tf_tnpc = mc->mc_tnpc;
+	tf->tf_tpc = mc->mc_tpc;
+	tf->tf_tstate = mc->mc_tstate;
+	tf->tf_y = mc->mc_y;
 	if ((mc->mc_fprs & FPRS_FEF) != 0) {
 		tf->tf_fprs = 0;
 		bcopy(mc->mc_fp, pcb->pcb_ufp, sizeof(pcb->pcb_ufp));
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 5 dfilter service freebsd_committer freebsd_triage 2009-11-24 22:13:25 UTC
Author: marius
Date: Tue Nov 24 22:13:06 2009
New Revision: 199768
URL: http://svn.freebsd.org/changeset/base/199768

Log:
  MFC: r199442
  
  Unroll copying of the registers in {g,s}et_mcontext() and limit it
  to the set actually restored by tl0_ret() instead of using the whole
  trapframe. Additionally skip %g7 as that register is used as the
  userland TLS pointer.
  
  PR:		140523

Modified:
  stable/7/sys/sparc64/sparc64/machdep.c
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/sparc64/sparc64/machdep.c
==============================================================================
--- stable/7/sys/sparc64/sparc64/machdep.c	Tue Nov 24 21:06:41 2009	(r199767)
+++ stable/7/sys/sparc64/sparc64/machdep.c	Tue Nov 24 22:13:06 2009	(r199768)
@@ -695,12 +695,39 @@ get_mcontext(struct thread *td, mcontext
 
 	tf = td->td_frame;
 	pcb = td->td_pcb;
-	bcopy(tf, mc, sizeof(*tf));
+	/*
+	 * Copy the registers which will be restored by tl0_ret() from the
+	 * trapframe.
+	 * Note that we skip %g7 which is used as the userland TLS register
+	 * and %wstate.
+	 */
+	mc->mc_flags = _MC_VERSION;
+	mc->mc_global[1] = tf->tf_global[1];
+	mc->mc_global[2] = tf->tf_global[2];
+	mc->mc_global[3] = tf->tf_global[3];
+	mc->mc_global[4] = tf->tf_global[4];
+	mc->mc_global[5] = tf->tf_global[5];
+	mc->mc_global[6] = tf->tf_global[6];
 	if (flags & GET_MC_CLEAR_RET) {
 		mc->mc_out[0] = 0;
 		mc->mc_out[1] = 0;
+	} else {
+		mc->mc_out[0] = tf->tf_out[0];
+		mc->mc_out[1] = tf->tf_out[1];
 	}
-	mc->mc_flags = _MC_VERSION;
+	mc->mc_out[2] = tf->tf_out[2];
+	mc->mc_out[3] = tf->tf_out[3];
+	mc->mc_out[4] = tf->tf_out[4];
+	mc->mc_out[5] = tf->tf_out[5];
+	mc->mc_out[6] = tf->tf_out[6];
+	mc->mc_out[7] = tf->tf_out[7];
+	mc->mc_fprs = tf->tf_fprs;
+	mc->mc_fsr = tf->tf_fsr;
+	mc->mc_gsr = tf->tf_gsr;
+	mc->mc_tnpc = tf->tf_tnpc;
+	mc->mc_tpc = tf->tf_tpc;
+	mc->mc_tstate = tf->tf_tstate;
+	mc->mc_y = tf->tf_y;
 	critical_enter();
 	if ((tf->tf_fprs & FPRS_FEF) != 0) {
 		savefpctx(pcb->pcb_ufp);
@@ -720,7 +747,6 @@ set_mcontext(struct thread *td, const mc
 {
 	struct trapframe *tf;
 	struct pcb *pcb;
-	uint64_t wstate;
 
 	if (!TSTATE_SECURE(mc->mc_tstate) ||
 	    (mc->mc_flags & ((1L << _MC_VERSION_BITS) - 1)) != _MC_VERSION)
@@ -729,9 +755,33 @@ set_mcontext(struct thread *td, const mc
 	pcb = td->td_pcb;
 	/* Make sure the windows are spilled first. */
 	flushw();
-	wstate = tf->tf_wstate;
-	bcopy(mc, tf, sizeof(*tf));
-	tf->tf_wstate = wstate;
+	/*
+	 * Copy the registers which will be restored by tl0_ret() to the
+	 * trapframe.
+	 * Note that we skip %g7 which is used as the userland TLS register
+	 * and %wstate.
+	 */
+	tf->tf_global[1] = mc->mc_global[1];
+	tf->tf_global[2] = mc->mc_global[2];
+	tf->tf_global[3] = mc->mc_global[3];
+	tf->tf_global[4] = mc->mc_global[4];
+	tf->tf_global[5] = mc->mc_global[5];
+	tf->tf_global[6] = mc->mc_global[6];
+	tf->tf_out[0] = mc->mc_out[0];
+	tf->tf_out[1] = mc->mc_out[1];
+	tf->tf_out[2] = mc->mc_out[2];
+	tf->tf_out[3] = mc->mc_out[3];
+	tf->tf_out[4] = mc->mc_out[4];
+	tf->tf_out[5] = mc->mc_out[5];
+	tf->tf_out[6] = mc->mc_out[6];
+	tf->tf_out[7] = mc->mc_out[7];
+	tf->tf_fprs = mc->mc_fprs;
+	tf->tf_fsr = mc->mc_fsr;
+	tf->tf_gsr = mc->mc_gsr;
+	tf->tf_tnpc = mc->mc_tnpc;
+	tf->tf_tpc = mc->mc_tpc;
+	tf->tf_tstate = mc->mc_tstate;
+	tf->tf_y = mc->mc_y;
 	if ((mc->mc_fprs & FPRS_FEF) != 0) {
 		tf->tf_fprs = 0;
 		bcopy(mc->mc_fp, pcb->pcb_ufp, sizeof(pcb->pcb_ufp));
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 Marius Strobl freebsd_committer freebsd_triage 2009-11-24 22:25:00 UTC
State Changed
From-To: open->closed

Close, this bug has been fixed.