Bug 6577 - SPARC64 get/setcontext smashes TLS
Summary: SPARC64 get/setcontext smashes TLS
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Jakub Jelinek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-28 23:45 UTC by Nathaniel Filardo
Modified: 2014-07-03 11:28 UTC (History)
2 users (show)

See Also:
Host: sparc-linux-gnu
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nathaniel Filardo 2008-05-28 23:45:34 UTC
(I'm not sure that this is libc's fault so apologies in advance if this is 
noise and should be redirected.  Please don't hesitate to ask for additional 
information; I'm not sure what else I should provide.)

Doing a setcontext() on Linux/SPARC64 (kernel 2.6.26-rc4) using glibc 2.7 
(Debian build 2.7-11) also writes over %g7, which is used to store the TLS 
pointer for pthread_self() (see libc/nptl/sysdeps/sparc/tls.h).  This breaks 
plan9port's libthread but it will show up any time threads exchange or pool 
contexts.

I've worked around it by manually setting uc_mcontext.mc_gregs[REG_G7] to 
pthread_self() before calling setcontext; if this is the correct fix, it would 
seem that setcontext() should be doing that itself.

Thanks for your time.
Comment 1 Jakub Jelinek 2008-07-31 17:18:34 UTC
I'd say the kernel setcontext 0x16f trap shouldn't change %g7, Dave, do you
agree?  Of course glibc can store %g7 to the setcontext structure before doing
ta 0x6f too, but that means the context has to be modified by setcontext.
Comment 2 David S. Miller 2008-08-01 03:42:05 UTC
Subject: Re:  SPARC64 get/setcontext smashes TLS

From: "jakub at redhat dot com" <sourceware-bugzilla@sourceware.org>
Date: 31 Jul 2008 17:18:35 -0000

> I'd say the kernel setcontext 0x16f trap shouldn't change %g7, Dave, do you
> agree?  Of course glibc can store %g7 to the setcontext structure before doing
> ta 0x6f too, but that means the context has to be modified by setcontext.

I agree, therefore I'll commit the following patch upstream.

sparc64: Do not clobber %g7 in setcontext() trap.

That's the userland thread register, so we should never try to change
it like this.

Based upon glibc bug nptl/6577 and suggestions by Jakub Jelinek.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc64/kernel/signal.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/sparc64/kernel/signal.c b/arch/sparc64/kernel/signal.c
index d1b8445..ca5a6ae 100644
--- a/arch/sparc64/kernel/signal.c
+++ b/arch/sparc64/kernel/signal.c
@@ -2,7 +2,7 @@
  *  arch/sparc64/kernel/signal.c
  *
  *  Copyright (C) 1991, 1992  Linus Torvalds
- *  Copyright (C) 1995 David S. Miller (davem@caip.rutgers.edu)
+ *  Copyright (C) 1995, 2008 David S. Miller (davem@davemloft.net)
  *  Copyright (C) 1996 Miguel de Icaza (miguel@nuclecu.unam.mx)
  *  Copyright (C) 1997 Eddie C. Dost   (ecd@skynet.be)
  *  Copyright (C) 1997,1998 Jakub Jelinek   (jj@sunsite.mff.cuni.cz)
@@ -91,7 +91,9 @@ asmlinkage void sparc64_set_context(struct pt_regs *regs)
 	err |= __get_user(regs->u_regs[UREG_G4], (&(*grp)[MC_G4]));
 	err |= __get_user(regs->u_regs[UREG_G5], (&(*grp)[MC_G5]));
 	err |= __get_user(regs->u_regs[UREG_G6], (&(*grp)[MC_G6]));
-	err |= __get_user(regs->u_regs[UREG_G7], (&(*grp)[MC_G7]));
+
+	/* Skip %g7 as that's the thread register in userspace.  */
+
 	err |= __get_user(regs->u_regs[UREG_I0], (&(*grp)[MC_O0]));
 	err |= __get_user(regs->u_regs[UREG_I1], (&(*grp)[MC_O1]));
 	err |= __get_user(regs->u_regs[UREG_I2], (&(*grp)[MC_O2]));
Comment 3 Ulrich Drepper 2008-08-13 07:00:02 UTC
Not a libc problem, this is fixed in the kernel.