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

Mika Penttilä
2005-10-05T17:36:25Z

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