Re: [PATCH] x86_64 signal handling for 64-bit apps w/ mixed 32-bit code - trivial fix

["Mika Penttilä" <mika.penttila@kolumbus.fi>]

Bryan Ford wrote:

>The following trivial patch fixes a bug in signal handling on x86-64: the 
>kernel currently fails to save and restore the CS and SS segment registers on 
>user-mode signal handler dispatch/return, which makes it impossible for 
>64-bit applications to catch and handle signals properly that occur while 
>running 32-bit code fragments in compatibility mode.
>
>The proposed patch doesn't affect any performance-critical paths (e.g., 
>syscall or interrupt entry/exit), and merely involves a couple more moves 
>to/from user space on signal frame setup and sigreturn.  It also doesn't 
>affect the size or shape of the sigcontext at all, since there already was an
>(unused) slot for CS, and I've assigned the convenient __pad0 field as a slot 
>for SS.  The existing, unused slots for FS and GS remain unused for now, and 
>I don't see any urgent need to change that.  The only way this might break an 
>existing app is if the app tries to cons up its own signal frame (not 
>generated by the kernel) and pass it to sigreturn, but this is presumably a 
>no-no anyway.
>
>The patch is against linux-2.6.13.3.
>Author: Bryan Ford, baford@mit.edu
>No copyright claimed; public domain.
>  
>
>------------------------------------------------------------------------
>
>diff -ur o13/include/asm-x86_64/sigcontext.h linux-2.6.13.3/include/asm-x86_64/sigcontext.h
>--- o13/include/asm-x86_64/sigcontext.h	2005-10-03 19:27:35.000000000 -0400
>+++ linux-2.6.13.3/include/asm-x86_64/sigcontext.h	2005-10-05 12:06:59.000000000 -0400
>@@ -43,7 +43,7 @@
> 	unsigned short cs;
> 	unsigned short gs;
> 	unsigned short fs;
>-	unsigned short __pad0; 
>+	unsigned short ss; 
> 	unsigned long err;
> 	unsigned long trapno;
> 	unsigned long oldmask;
>diff -ur o13/arch/x86_64/kernel/signal.c linux-2.6.13.3/arch/x86_64/kernel/signal.c
>--- o13/arch/x86_64/kernel/signal.c	2005-10-03 19:27:35.000000000 -0400
>+++ linux-2.6.13.3/arch/x86_64/kernel/signal.c	2005-10-05 12:13:22.000000000 -0400
>@@ -110,6 +110,15 @@
> 	COPY(r14);
> 	COPY(r15);
> 
>+	/* Kernel saves and restores only CS and DS segments on signals,
>+	 * which are the bare essentials needed to allow mixed 32/64-bit code.
>+	 * App's signal handler can save/restore other segments if needed. */
>+	unsigned short cs, ss;
>+	err |= __get_user(cs, &sc->cs);
>+	err |= __get_user(ss, &sc->ss);
>+	regs->cs = cs | 3;	/* Force into user mode */
>+	regs->ss = ss | 3;
>+
> 	{
> 		unsigned int tmpflags;
> 		err |= __get_user(tmpflags, &sc->eflags);
>@@ -187,6 +196,8 @@
> {
> 	int err = 0;
> 
>+	err |= __put_user(regs->cs, &sc->cs);
>+	err |= __put_user(regs->ss, &sc->ss);
> 	err |= __put_user(0, &sc->gs);
> 	err |= __put_user(0, &sc->fs);
> 
>@@ -318,7 +329,15 @@
> 
> 	regs->rsp = (unsigned long)frame;
> 
>+	/* Set up segment registers to run signal handlers in 64-bit mode,
>+	   even if the handler happens to be interrupting 32-bit code. */
>+	regs->cs = __USER_CS;
>+	regs->ss = __USER_DS;
>+
>+	/* This, by contrast, has nothing to do with segment registers -
>+	   see include/asm-x86_64/uaccess.h for details. */
> 	set_fs(USER_DS);
>+
> 	regs->eflags &= ~TF_MASK;
> 	if (test_thread_flag(TIF_SINGLESTEP))
> 		ptrace_notify(SIGTRAP);
>  
>

What about the opposite? Are there things that would break if the app 
depends on compatibility mode signal handler?

--Mika