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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/