x86_64,signal: Fix the set of saved segment registers

The comment in the signal code says that apps can save/restore other segments on their own. It's true that apps can *save* SS on their own, but there's no way for apps to restore it: SYSCALL effectively resets SS to __USER_DS, so any value that user code tries to load into SS gets lost on entry to sigreturn.

This recycles two padding bytes in the segment selector area for SS.

I suspect that 64-bit programs that try to run 16-bit code and uses signals will have a lot of trouble without this.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/include/asm/sigcontext.h      | 2 +-
 arch/x86/include/uapi/asm/sigcontext.h | 2 +-
 arch/x86/kernel/signal.c               | 8 +-------
 3 files changed, 3 insertions(+), 9 deletions(-)

arch/x86/include/asm/sigcontext.h +1 -1

index 9dfce4e..f910cdc 100644
@@ -59,7 +59,7 @@ struct sigcontext {
        unsigned short cs;
        unsigned short gs;
        unsigned short fs;
-       unsigned short __pad0;
+       unsigned short ss;
        unsigned long err;
        unsigned long trapno;
        unsigned long oldmask;

arch/x86/include/uapi/asm/sigcontext.h +1 -1

index d8b9f90..076b11f 100644
@@ -179,7 +179,7 @@ struct sigcontext {
        __u16 cs;
        __u16 gs;
        __u16 fs;
-       __u16 __pad0;
+       __u16 ss;
        __u64 err;
        __u64 trapno;
        __u64 oldmask;

arch/x86/kernel/signal.c +1 -8

index 2851d63..08d7a9e 100644
@@ -94,15 +94,8 @@ int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
                COPY(r15);
 #endif /* CONFIG_X86_64 */
 
-#ifdef CONFIG_X86_32
                COPY_SEG_CPL3(cs);
                COPY_SEG_CPL3(ss);
-#else /* !CONFIG_X86_32 */
-               /* Kernel saves and restores only the CS segment register on signals,
-                * which is the bare minimum needed to allow mixed 32/64-bit code.
-                * App's signal handler can save/restore other segments if needed. */
-               COPY_SEG_CPL3(cs);
-#endif /* CONFIG_X86_32 */
 
                get_user_ex(tmpflags, &sc->flags);
                regs->flags = (regs->flags & ~FIX_EFLAGS) | (tmpflags & FIX_EFLAGS);

@@ -164,6 +157,7 @@ int setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate,
                put_user_ex(regs->cs, &sc->cs);
                put_user_ex(0, &sc->gs);
                put_user_ex(0, &sc->fs);
+               put_user_ex(regs->ss, &sc->ss);
 #endif /* CONFIG_X86_32 */
 
                put_user_ex(fpstate, &sc->fpstate);
--

1.9.3

arch/x86/include/uapi/asm/sigcontext.h +2 -2

@@ -177,8 +177,8 @@ struct sigcontext {
 	__u64 rip;
 	__u64 eflags;		/* RFLAGS */
 	__u16 cs;
-	__u16 gs;
-	__u16 fs;
+	__u16 __pad2;		/* Was called gs, but was always zero. */
+	__u16 __pad1;		/* Was called fs, but was always zero. */
 	__u16 ss;
 	__u64 err;
 	__u64 trapno;

arch/x86/include/uapi/asm/sigcontext.h

Quoted 4 lines

arch/x86/include/uapi/asm/sigcontext.h +0 -0

index 076b11f..df9908b 100644

I don't think renaming fields in uapi/asm is acceptable. These are likely used by user programs and you'll break compiles.

Quoted 7 lines · Andi Kleen

arch/x86/include/uapi/asm/sigcontext.h

Quoted 4 lines

arch/x86/include/uapi/asm/sigcontext.h +0 -0

index 076b11f..df9908b 100644

I don't think renaming fields in uapi/asm is acceptable. These are likely used by user programs and you'll break compiles.

Hmm. That's a fair point. On the other hand, any user code that uses these fields explicitly may already be broken, since the current names of these fields rather strongly imply that they do something.

Is there any clear policy on minor API breaks in the UAPI headers that don't affect ABI?

arch/x86/include/uapi/asm/sigcontext.h +0 -0

index 076b11f..df9908b 100644

As far as I can tell, these fields have been set to zero on save and ignored on restore since Linux was imported into git. Rename them '__pad1' and '__pad2' to avoid confusion and to allow them to be recycled some day.

Quoted 4 lines · Andy Lutomirski

As far as I can tell, these fields have been set to zero on save and ignored on restore since Linux was imported into git. Rename them '__pad1' and '__pad2' to avoid confusion and to allow them to be recycled some day.

Shouldn't we actually try to save/restore gs/fs properly? Admittedly, changing them in the signal handler would be insane, but still.. See our context switching code with the whole segment selector vs base save/restore code. Hmm?

Linus

Quoted 4 lines · Andy Lutomirski

As far as I can tell, these fields have been set to zero on save and ignored on restore since Linux was imported into git. Rename them '__pad1' and '__pad2' to avoid confusion and to allow them to be recycled some day.

Shouldn't we actually try to save/restore gs/fs properly? Admittedly, changing them in the signal handler would be insane, but still.. See our context switching code with the whole segment selector vs base save/restore code. Hmm?

Linus

This seems like it's asking for trouble. I think wxe'd have to separately save the selectors and the base registers to avoid breaking something, especially once wrgsbase, etc are enabled.

Linus, for context, the other patch in this series saves and restores SS.

Without that, 64-bit sigreturn to a nondefault stack segment is basically impossible.

Quoted 2 lines

This seems like it's asking for trouble. I think wxe'd have to separately save the selectors and the base registers to avoid

breaking

Quoted 1 line

something, especially once wrgsbase, etc are enabled.

I agree, it would likely break existing code.

Quoted 5 lines

Linus, for context, the other patch in this series saves and restores SS.

Without that, 64-bit sigreturn to a nondefault stack segment is basically impossible.

Why would you ever want to do that?

-Andi

Because you are doing something weird (like Pin, for example) and take an asynchronous fault?

Quoted 1 line · H. Peter Anvin

Because you are doing something weird (like Pin, for example) and take an asynchronous fault?

But even for pin that would need executing 16 bit code, or really weird 32bit code. AFAIK for 32bit the only good use case was NX emulation (and old virtualization) which are both completely obsolete.

I don't think it's worth messing with the signal handlers for 16bit code. If there's any problem with saving/restoring state that emulator can always handle it by itself.

Quoted 6 lines · Andi Kleen

Quoted 1 line · H. Peter Anvin

Because you are doing something weird (like Pin, for example) and take an asynchronous fault?

But even for pin that would need executing 16 bit code, or really weird 32bit code. AFAIK for 32bit the only good use case was NX emulation (and old virtualization) which are both completely obsolete.

Nothing particularly weird is needed. Set a non-default stack segment (e.g. any 16-bit ss) and take *any* fault. This could be something asynchronous or even a normal synchronous fault. Return from the signal handler: boom.

We know that people use 16-bit stack segments: it's what prompted the whole espfix64 thing.

Quoted 5 lines

I don't think it's worth messing with the signal handlers for 16bit code. If there's any problem with saving/restoring state that emulator can always handle it by itself.

How?

I can think of at least two vaguely feasible ways. The program could ptrace itself, trap sigreturn, and fix ss. Or it could restore registers itself and return using far ret or iret. Both suck.