x86: sigcontext SS fixes, take 2

RFC Superseded Andy Lutomirski 5 participants 26 messages Oct 13, 2015 – Oct 18, 2015

Subsystems (2)

x86 Kselftest

lore

Share & tools

lore

Andy Lutomirski

author

2015-10-13 01:04 UTC #0 lore raw

cover x86: sigcontext SS fixes, take 2

This is take 2 at fixing x86 64-bit signals wrt SS. After a lot of thought, this is not controlled by any flags -- I would much prefer to avoid opt-in behavior. Instead, it just tries hard to avoid triggering the cases that break DOSEMU.

Stas, what do you think? Could you test this? It applies on top of tip:x86/asm. You can also find it at
https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/sigcontext&id=fd69bc4e6095d6a7cf2a0f03e69bace025505132

With this applied, all of the x86 selftests pass on x86_64. That wasn't the case before -- ldt_gdt_64 was broken.

This is a bit risky, and another option would be to do nothing at all. Then we'd disable the problematic self-tests (sigh), and DOSEMU and similar tools will be stuck using gross hacks even on new kernels.

Andy Lutomirski (4):
x86/signal/64: Add a comment about sigcontext->fs and gs x86/signal/64: Fix SS if needed when delivering a 64-bit signal x86/signal/64: Re-add support for SS in the 64-bit signal context selftests/x86: Add tests for UC_SIGCONTEXT_SS and UC_STRICT_RESTORE_SS

 arch/x86/include/asm/desc_defs.h        |  23 +++
 arch/x86/include/asm/sigcontext.h       |   2 +-
 arch/x86/include/asm/sighandling.h      |   1 -
 arch/x86/include/uapi/asm/sigcontext.h  |  23 ++-
 arch/x86/include/uapi/asm/ucontext.h    |  41 +++++-
 arch/x86/kernel/signal.c                | 115 ++++++++++++---
 tools/testing/selftests/x86/Makefile    |   4 +-
 tools/testing/selftests/x86/sigreturn.c | 240 ++++++++++++++++++++++++++++----
 8 files changed, 389 insertions(+), 60 deletions(-)

Signature · 1 line

2.4.3

Andy Lutomirski

author

2015-10-13 01:04 UTC #1 lore raw

RFC 1/4 x86/signal/64: Add a comment about sigcontext->fs and gs

These fields have a strange history. This tries to document it.

This borrows from 9a036b93a344 ("x86/signal/64: Remove 'fs' and 'gs' from sigcontext"), which was reverted by ed596cde9425 ("Revert x86 sigcontext cleanups").

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

---
 arch/x86/include/uapi/asm/sigcontext.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

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

index 40836a9a7250..827c6ed0e26a 100644
@@ -177,6 +177,24 @@ struct sigcontext {
 	__u64 rip;
 	__u64 eflags;		/* RFLAGS */
 	__u16 cs;
+
+	/*
+	 * Prior to 2.5.64 ("[PATCH] x86-64 updates for 2.5.64-bk3"),
+	 * Linux saved and restored fs and gs in these slots.  This
+	 * was counterproductive, as fsbase and gsbase were never
+	 * saved, so arch_prctl was presumably unreliable.
+	 *
+	 * If these slots are ever needed for any other purpose, there
+	 * is some risk that very old 64-bit binaries could get
+	 * confused.  I doubt that many such binaries still work,
+	 * though, since the same patch in 2.5.64 also removed the
+	 * 64-bit set_thread_area syscall, so it appears that there is
+	 * no TLS API that works in both pre- and post-2.5.64 kernels.
+	 *
+	 * There is at least one additional concern if these slots are
+	 * recycled for another purpose: some DOSEMU versions stash fs
+	 * and gs in these slots manually.
+	 */
 	__u16 gs;
 	__u16 fs;
 	__u16 __pad0;

Signature · 1 line

2.4.3

Andy Lutomirski

author

2015-10-13 01:04 UTC #25 lore raw

RFC 4/4 selftests/x86: Add tests for UC_SIGCONTEXT_SS and UC_STRICT_RESTORE_SS

In reply to Andy Lutomirski

This tests the two ABI-preserving cases that DOSEMU cares about, and it also explicitly tests the new UC_SIGCONTEXT_SS and UC_STRICT_RESTORE_SS flags.

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

---
 tools/testing/selftests/x86/sigreturn.c | 240 ++++++++++++++++++++++++++++----
 1 file changed, 212 insertions(+), 28 deletions(-)

tools/testing/selftests/x86/sigreturn.c +212 ~~-28~~

index b5aa1bab7416..43e840470e32 100644
@@ -55,6 +55,47 @@
 #include <sys/user.h>
 
 /*
+ * Copied from asm/ucontext.h, as asm/ucontext.h conflicts badly with the glibc
+ * headers.
+ */
+#ifdef __x86_64__
+/*
+ * UC_SAVED_SS will be set when delivering 64-bit or x32 signals on
+ * kernels that save SS in the sigcontext.  Kernels that set UC_SAVED_SS
+ * allow signal handlers to set UC_RESTORE_SS; if UC_RESTORE_SS is set,
+ * then sigreturn will restore SS.
+ *
+ * For compatibility with old programs, the kernel will *not* set
+ * UC_RESTORE_SS when delivering signals.
+ */
+#define UC_SIGCONTEXT_SS       0x2
+#define UC_STRICT_RESTORE_SS   0x4
+#endif
+
+/* Access rights as returned by LAR */
+#define AR_TYPE_RODATA		(0 * (1 << 9))
+#define AR_TYPE_RWDATA		(1 * (1 << 9))
+#define AR_TYPE_RODATA_EXPDOWN	(2 * (1 << 9))
+#define AR_TYPE_RWDATA_EXPDOWN	(3 * (1 << 9))
+#define AR_TYPE_XOCODE		(4 * (1 << 9))
+#define AR_TYPE_XRCODE		(5 * (1 << 9))
+#define AR_TYPE_XOCODE_CONF	(6 * (1 << 9))
+#define AR_TYPE_XRCODE_CONF	(7 * (1 << 9))
+#define AR_TYPE_MASK		(7 * (1 << 9))
+
+#define AR_DPL0			(0 * (1 << 13))
+#define AR_DPL3			(3 * (1 << 13))
+#define AR_DPL_MASK		(3 * (1 << 13))
+
+#define AR_A			(1 << 8)	/* A means "accessed" */
+#define AR_S			(1 << 12)	/* S means "not system" */
+#define AR_P			(1 << 15)	/* P means "present" */
+#define AR_AVL			(1 << 20)	/* AVL does nothing */
+#define AR_L			(1 << 21)	/* L means "long mode" */
+#define AR_DB			(1 << 22)	/* D or B, depending on type */
+#define AR_G			(1 << 23)	/* G means "limit in pages" */
+
+/*
  * In principle, this test can run on Linux emulation layers (e.g.
  * Illumos "LX branded zones").  Solaris-based kernels reserve LDT
  * entries 0-5 for their own internal purposes, so start our LDT

@@ -267,6 +308,9 @@ static gregset_t initial_regs, requested_regs, resulting_regs;
 /* Instructions for the SIGUSR1 handler. */
 static volatile unsigned short sig_cs, sig_ss;
 static volatile sig_atomic_t sig_trapped, sig_err, sig_trapno;
+#ifdef __x86_64__
+static volatile sig_atomic_t sig_corrupt_final_ss;
+#endif
 
 /* Abstractions for some 32-bit vs 64-bit differences. */
 #ifdef __x86_64__

@@ -305,9 +349,105 @@ static greg_t *csptr(ucontext_t *ctx)
 }
 #endif
 
+/*
+ * Checks a given selector for its code bitness or returns -1 if it's not
+ * a usable code segment selector.
+ */
+int cs_bitness(unsigned short cs)
+{
+	uint32_t valid = 0, ar;
+	asm ("lar %[cs], %[ar]\n\t"
+	     "jnz 1f\n\t"
+	     "mov $1, %[valid]\n\t"
+	     "1:"
+	     : [ar] "=r" (ar), [valid] "+rm" (valid)
+	     : [cs] "r" (cs));
+
+	if (!valid)
+		return -1;
+
+	bool db = (ar & (1 << 22));
+	bool l = (ar & (1 << 21));
+
+	if (!(ar & (1<<11)))
+	    return -1;	/* Not code. */
+
+	if (l && !db)
+		return 64;
+	else if (!l && db)
+		return 32;
+	else if (!l && !db)
+		return 16;
+	else
+		return -1;	/* Unknown bitness. */
+}
+
+/*
+ * Checks a given selector for its code bitness or returns -1 if it's not
+ * a usable code segment selector.
+ */
+bool is_valid_ss(unsigned short cs)
+{
+	uint32_t valid = 0, ar;
+	asm ("lar %[cs], %[ar]\n\t"
+	     "jnz 1f\n\t"
+	     "mov $1, %[valid]\n\t"
+	     "1:"
+	     : [ar] "=r" (ar), [valid] "+rm" (valid)
+	     : [cs] "r" (cs));
+
+	if (!valid)
+		return false;
+
+	if ((ar & AR_TYPE_MASK) != AR_TYPE_RWDATA &&
+	    (ar & AR_TYPE_MASK) != AR_TYPE_RWDATA_EXPDOWN)
+		return false;
+
+	return (ar & AR_P);
+}
+
 /* Number of errors in the current test case. */
 static volatile sig_atomic_t nerrs;
 
+static void validate_signal_ss(int sig, ucontext_t *ctx)
+{
+#ifdef __x86_64__
+	bool was_64bit = (cs_bitness(*csptr(ctx)) == 64);
+
+	if (!(ctx->uc_flags & UC_SIGCONTEXT_SS)) {
+		printf("[FAIL]\tUC_SIGCONTEXT_SS was not set\n");
+		nerrs++;
+
+		/*
+		 * This happens on Linux 4.1.  The rest will fail, too, so
+		 * return now to reduce the noise.
+		 */
+		return;
+	}
+
+	/* UC_STRICT_RESTORE_SS is set iff we came from 64-bit mode. */
+	if (!!(ctx->uc_flags & UC_STRICT_RESTORE_SS) != was_64bit) {
+		printf("[FAIL]\tUC_STRICT_RESTORE_SS was wrong in signal %d\n",
+		       sig);
+		nerrs++;
+	}
+
+	if (is_valid_ss(*ssptr(ctx))) {
+		/*
+		 * DOSEMU was written before 64-bit sigcontext had SS, and
+		 * it tries to figure out the signal source SS by looking at
+		 * the physical register.  Make sure that keeps working.
+		 */
+		unsigned short hw_ss;
+		asm ("mov %%ss, %0" : "=rm" (hw_ss));
+		if (hw_ss != *ssptr(ctx)) {
+			printf("[FAIL]\tHW SS didn't match saved SS\n");
+			nerrs++;
+		}
+	}
+#endif
+}
+
 /*
  * SIGUSR1 handler.  Sets CS and SS as requested and points IP to the
  * int3 trampoline.  Sets SP to a large known value so that we can see

@@ -317,6 +457,8 @@ static void sigusr1(int sig, siginfo_t *info, void *ctx_void)
 {
 	ucontext_t *ctx = (ucontext_t*)ctx_void;
 
+	validate_signal_ss(sig, ctx);
+
 	memcpy(&initial_regs, &ctx->uc_mcontext.gregs, sizeof(gregset_t));
 
 	*csptr(ctx) = sig_cs;

@@ -334,13 +476,16 @@ static void sigusr1(int sig, siginfo_t *info, void *ctx_void)
 }
 
 /*
- * Called after a successful sigreturn.  Restores our state so that
- * the original raise(SIGUSR1) returns.
+ * Called after a successful sigreturn (via int3) or from a failed
+ * sigreturn (directly by kernel).  Restores our state so that the
+ * original raise(SIGUSR1) returns.
  */
 static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
 {
 	ucontext_t *ctx = (ucontext_t*)ctx_void;
 
+	validate_signal_ss(sig, ctx);
+
 	sig_err = ctx->uc_mcontext.gregs[REG_ERR];
 	sig_trapno = ctx->uc_mcontext.gregs[REG_TRAPNO];

@@ -358,41 +503,62 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
 	memcpy(&resulting_regs, &ctx->uc_mcontext.gregs, sizeof(gregset_t));
 	memcpy(&ctx->uc_mcontext.gregs, &initial_regs, sizeof(gregset_t));
 
+#ifdef __x86_64__
+	if (sig_corrupt_final_ss) {
+		if (ctx->uc_flags & UC_STRICT_RESTORE_SS) {
+			printf("[FAIL]\tUC_STRICT_RESTORE_SS was set inappropriately\n");
+			nerrs++;
+		} else {
+			/*
+			 * DOSEMU transitions from 32-bit to 64-bit mode by
+			 * adjusting sigcontext, and it requires that this work
+			 * even if the saved SS is bogus.
+			 */
+			printf("\tCorrupting SS on return to 64-bit mode\n");
+			*ssptr(ctx) = 0;
+		}
+	}
+#endif
+
 	sig_trapped = sig;
 }
 
-/*
- * Checks a given selector for its code bitness or returns -1 if it's not
- * a usable code segment selector.
- */
-int cs_bitness(unsigned short cs)
+#ifdef __x86_64__
+/* Tests recovery if !UC_STRICT_RESTORE_SS */
+static void sigusr2(int sig, siginfo_t *info, void *ctx_void)
 {
-	uint32_t valid = 0, ar;
-	asm ("lar %[cs], %[ar]\n\t"
-	     "jnz 1f\n\t"
-	     "mov $1, %[valid]\n\t"
-	     "1:"
-	     : [ar] "=r" (ar), [valid] "+rm" (valid)
-	     : [cs] "r" (cs));
+	ucontext_t *ctx = (ucontext_t*)ctx_void;
 
-	if (!valid)
-		return -1;
+	if (!(ctx->uc_flags & UC_STRICT_RESTORE_SS)) {
+		printf("[FAIL]\traise(2) didn't set UC_STRICT_RESTORE_SS\n");
+		nerrs++;
+		return;  /* We can't do the rest. */
+	}
 
-	bool db = (ar & (1 << 22));
-	bool l = (ar & (1 << 21));
+	ctx->uc_flags &= ~UC_STRICT_RESTORE_SS;
+	*ssptr(ctx) = 0;
 
-	if (!(ar & (1<<11)))
-	    return -1;	/* Not code. */
+	/* Return.  The kernel should recover without sending another signal. */
+}
 
-	if (l && !db)
-		return 64;
-	else if (!l && db)
-		return 32;
-	else if (!l && !db)
-		return 16;
-	else
-		return -1;	/* Unknown bitness. */
+static int test_nonstrict_ss(void)
+{
+	clearhandler(SIGUSR1);
+	clearhandler(SIGTRAP);
+	clearhandler(SIGSEGV);
+	clearhandler(SIGILL);
+	sethandler(SIGUSR2, sigusr2, 0);
+
+	nerrs = 0;
+
+	printf("[RUN]\tClear UC_STRICT_RESTORE_SS and corrupt SS\n");
+	raise(SIGUSR2);
+	if (!nerrs)
+		printf("[OK]\tIt worked\n");
+
+	return nerrs;
 }
+#endif
 
 /* Finds a usable code segment of the requested bitness. */
 int find_cs(int bitness)

@@ -576,6 +742,12 @@ static int test_bad_iret(int cs_bits, unsigned short ss, int force_cs)
 		       errdesc, strsignal(sig_trapped));
 		return 0;
 	} else {
+		/*
+		 * This also implicitly tests UC_STRICT_RESTORE_SS:
+		 * We check that these signals set UC_STRICT_RESTORE_SS and,
+		 * if UC_STRICT_RESTORE_SS doesn't cause strict behavior,
+		 * then we won't get SIGSEGV.
+		 */
 		printf("[FAIL]\tDid not get SIGSEGV\n");
 		return 1;
 	}

@@ -632,6 +804,14 @@ int main()
 						    GDT3(gdt_data16_idx));
 	}
 
+#ifdef __x86_64__
+	/* Nasty ABI case: check SS corruption handling. */
+	sig_corrupt_final_ss = 1;
+	total_nerrs += test_valid_sigreturn(32, false, -1);
+	total_nerrs += test_valid_sigreturn(32, true, -1);
+	sig_corrupt_final_ss = 0;
+#endif
+
 	/*
 	 * We're done testing valid sigreturn cases.  Now we test states
 	 * for which sigreturn itself will succeed but the subsequent

@@ -680,5 +860,9 @@ int main()
 	if (gdt_npdata32_idx)
 		test_bad_iret(32, GDT3(gdt_npdata32_idx), -1);
 
+#ifdef __x86_64__
+	total_nerrs += test_nonstrict_ss();
+#endif
+
 	return total_nerrs ? 1 : 0;
 }

Signature · 1 line

2.4.3

Andy Lutomirski

author

2015-10-13 01:04 UTC #3 lore raw

RFC 3/4 x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Andy Lutomirski

This is a second attempt to make the improvements from c6f2062935c8 ("x86/signal/64: Fix SS handling for signals delivered to 64-bit programs"), which was reverted by 51adbfbba5c6 ("x86/signal/64: Add support for SS in the 64-bit signal context").

This adds two new uc_flags flags. UC_SIGCONTEXT_SS will be set for all 64-bit signals (including x32). It indicates that the saved SS field is valid and that the kernel supports the new behavior.

The goal is to fix a problems with signal handling in 64-bit tasks:
SS wasn't saved in the 64-bit signal context, making it awkward to determine what SS was at the time of signal delivery and making it impossible to return to a non-flat SS (as calling sigreturn clobbers SS).

This also made it extremely difficult for 64-bit tasks to return to fully-defined 16-bit contexts, because only the kernel can easily do espfix64, but sigreturn was unable to load or even restore SS:ESP. (DOSEMU has a monstrous hack to partially work around this.)

If we could go back in time, the correct fix would be to make 64-bit signals work just like 32-bit signals with respect to SS: save it in signal context, reset it when delivering a signal, and restore it in sigreturn.

Unfortunately, doing that (as I tried originally) breaks DOSEMU:
DOSEMU wouldn't reset the signal context's SS when clearing the LDT and changing the saved CS to 64-bit mode, since it predates the SS context field existing in the first place.

This patch is a bit more complicated, and it tries to balance a bunch of goals. It makes signal delivery due to a bogus SS reliable without having to set any flags (64-bit signals will always be delivered to a valid SS), and it makes most cases of changing ucontext->ss during signal handling work as expected.

I do this by special-casing the interesting case. On sigreturn, ucontext->ss will be honored by default, unless the ucontext was created from scratch by an old program and had a 64-bit CS (unfortunately, CRIU can do this) or was the result of changing a 32-bit signal context to 64-bit without resetting SS (as DOSEMU does).

For the benefit of new 64-bit software that uses segmentation (new versions of DOSEMU might), the new behavior can be detected with a new ucontext flag UC_SIGCONTEXT_SS.

To avoid compilation issues, __pad0 is left as an alias for ss in ucontext.

The nitty-gritty details are documented in the header file.

Cc Stas Sergeev <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Cyrill Gorcunov <[email protected]>

Cc Pavel Emelyanov <[email protected]>

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

---
 arch/x86/include/asm/sigcontext.h      |  2 +-
 arch/x86/include/asm/sighandling.h     |  1 -
 arch/x86/include/uapi/asm/sigcontext.h |  5 ++-
 arch/x86/include/uapi/asm/ucontext.h   | 41 +++++++++++++++++++---
 arch/x86/kernel/signal.c               | 64 +++++++++++++++++++++++-----------
 tools/testing/selftests/x86/Makefile   |  4 +--
 6 files changed, 87 insertions(+), 30 deletions(-)

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

index 9dfce4e0417d..9789402b99b6 100644
@@ -59,7 +59,7 @@ struct sigcontext {
 	unsigned short cs;
 	unsigned short gs;
 	unsigned short fs;
-	unsigned short __pad0;
+	unsigned short ss;	/* Only restored if UC_RESTORE_SS is set. */
 	unsigned long err;
 	unsigned long trapno;
 	unsigned long oldmask;

arch/x86/include/asm/sighandling.h +0 -1

index 89db46752a8f..452c88b8ad06 100644
@@ -13,7 +13,6 @@
 			 X86_EFLAGS_CF | X86_EFLAGS_RF)
 
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
-int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc);
 int setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate,
 		     struct pt_regs *regs, unsigned long mask);

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

index 827c6ed0e26a..ad02ae519179 100644
@@ -197,7 +197,10 @@ struct sigcontext {
 	 */
 	__u16 gs;
 	__u16 fs;
-	__u16 __pad0;
+	union {
+		__u16 ss;	/* If UC_SAVED_SS */
+		__u16 __pad0;	/* If !UC_SAVED_SS */
+	};
 	__u64 err;
 	__u64 trapno;
 	__u64 oldmask;

arch/x86/include/uapi/asm/ucontext.h +36 -5

47 lines not on this page · raw message

arch/x86/kernel/signal.c +44 ~~-20~~

144 lines not on this page · raw message

tools/testing/selftests/x86/Makefile +2 -2

index 389701f59940..c2221786d835 100644
@@ -4,8 +4,8 @@ include ../lib.mk
 
 .PHONY: all all_32 all_64 warn_32bit_failure clean
 
-TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs ldt_gdt syscall_nt ptrace_syscall
-TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault sigreturn test_syscall_vdso unwind_vdso
+TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs ldt_gdt syscall_nt ptrace_syscall sigreturn
+TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso
 
 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
 BINARIES_32 := $(TARGETS_C_32BIT_ALL:%=%_32)

Signature · 1 line

2.4.3

Andy Lutomirski

author

2015-10-13 01:05 UTC #2 lore raw

RFC 2/4 x86/signal/64: Fix SS if needed when delivering a 64-bit signal

In reply to Andy Lutomirski

Signals are always delivered to 64-bit tasks with CS set to a long mode segment. In long mode, SS doesn't matter, as long as it's a present writable segment.

If SS starts out invalid (this can happen if the signal was caused by an IRET fault or was delivered on the way out of set_thread_area or modify_ldt), then IRET to the signal handler can fail, eventually killing the task.

The straightforward fix would be to simply reset SS when delivering a signal. That breaks DOSEMU, though: 64-bit builds of DOSEMU rely on SS being set to the faulting SS when signals are delivered.

As a compromise, this patch leaves SS alone so long as it's valid.

The net effect should be that the behavior of successfully delivered signals is unchanged. Some signals that would previously have failed to be delivered will now be delivered successfully.

This has no effect for x32 or 32-bit tasks: their signal handlers were already called with SS == __USER_DS.

(On Xen, there's a slight hole: if a task sets SS to a writable
*kernel* data segment, then we will fail to identify it as invalid and we'll still kill the task. If anyone cares, this could be fixed with a new paravirt hook.)

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

---
 arch/x86/include/asm/desc_defs.h | 23 ++++++++++++++++++
 arch/x86/kernel/signal.c         | 51 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 72 insertions(+), 2 deletions(-)

arch/x86/include/asm/desc_defs.h +23 -0

index 278441f39856..00971705a16d 100644
@@ -98,4 +98,27 @@ struct desc_ptr {
 
 #endif /* !__ASSEMBLY__ */
 
+/* Access rights as returned by LAR */
+#define AR_TYPE_RODATA		(0 * (1 << 9))
+#define AR_TYPE_RWDATA		(1 * (1 << 9))
+#define AR_TYPE_RODATA_EXPDOWN	(2 * (1 << 9))
+#define AR_TYPE_RWDATA_EXPDOWN	(3 * (1 << 9))
+#define AR_TYPE_XOCODE		(4 * (1 << 9))
+#define AR_TYPE_XRCODE		(5 * (1 << 9))
+#define AR_TYPE_XOCODE_CONF	(6 * (1 << 9))
+#define AR_TYPE_XRCODE_CONF	(7 * (1 << 9))
+#define AR_TYPE_MASK		(7 * (1 << 9))
+
+#define AR_DPL0			(0 * (1 << 13))
+#define AR_DPL3			(3 * (1 << 13))
+#define AR_DPL_MASK		(3 * (1 << 13))
+
+#define AR_A			(1 << 8)	/* A means "accessed" */
+#define AR_S			(1 << 12)	/* S means "not system" */
+#define AR_P			(1 << 15)	/* P means "present" */
+#define AR_AVL			(1 << 20)	/* AVL does nothing */
+#define AR_L			(1 << 21)	/* L means "long mode" */
+#define AR_DB			(1 << 22)	/* D or B, depending on type */
+#define AR_G			(1 << 23)	/* G means "limit in pages" */
+
 #endif /* _ASM_X86_DESC_DEFS_H */

arch/x86/kernel/signal.c +49 -2

65 lines not on this page · raw message

Signature · 1 line

2.4.3

Stas Sergeev

2015-10-13 16:01 UTC #4 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Andy Lutomirski

13.10.2015 04:04, Andy Lutomirski пишет:

Quoted 4 lines

+ * UC_SIGCONTEXT_SS will be set when delivering 64-bit or x32 signals on
+ * kernels that save SS in the sigcontext. All kernels that set
+ * UC_SIGCONTEXT_SS will correctly restore at least the low 32 bits of esp
+ * regardless of SS (i.e. they implement espfix).

Is this comment relevant? I think neither signal delivery nor sigreturn were affected by esp corruption, or were they? I guess you suggest to use that flag as the detection
for espfix, but I don't think this is relevant: you may need to know about espfix also outside of a signal handler. In fact, I don't think espfix needs any run-time detection, because then the stack fault will simply not happen, and that's all. I think it is a matter of compile-time detection only.

Quoted 10 lines

+ *
+ * Kernels that set UC_SIGCONTEXT_SS will also set UC_STRICT_RESTORE_SS
+ * when delivering a signal that came from 64-bit code.
+ *
+ * Sigreturn modifies its behavior depending on the UC_STRICT_RESTORE_SS
+ * flag. If UC_STRICT_RESTORE_SS is set, then the SS value in the
+ * signal context is restored verbatim. If UC_STRICT_RESTORE_SS is not
+ * set, the CS value in the signal context refers to a 64-bit code
+ * segment, and the signal context's SS value is invalid, it will be
+ * replaced by an flat 32-bit selector.

Quoted 8 lines

+
+ * This behavior serves two purposes. It ensures that older programs
+ * that are unaware of the signal context's SS slot and either construct
+ * a signal context from scratch or that catch signals from segmented
+ * contexts and change CS to a 64-bit selector won't crash due to a bad
+ * SS value. It also ensures that signal handlers that do not modify
+ * the signal context at all return back to the exact CS and SS state
+ * that they came from.

Do you need a second flag for this?
IIRC non-restoring was needed because:
1. dosemu saves SS to different place
2. If you save it yourself, dosemu can invalidate it, but not replace with the right one because of 1.
IMHO to solve this, you need _either_ the second flag or the heuristic, but not both.

With new flag:
Just don't set it by default, and the new progs will set it themselves. Old progs are unaffected.
When it is set, SS should always be restored.
I prefer this approach.

With heuristic:
Save SS yourself on delivery, and, if it happens invalid on sigreturn - replace it with better one.
Old progs are unaffected because they use iret anyway, and that iret happens _after_ sigreturn.
New progs will never leave invalid SS in the right sigcontext slot.

So why have you choose to have both the new flag UC_STRICT_RESTORE_SS and the heuristic?

Quoted 2 lines

This is a bit risky, and another option would be to do nothing at all.

Andy, could you please stop pretending there are no other solutions? You do not have to like them. You do not have to implement them. But your continuous re-assertions that they do not exist, make me feel a bit uncomfortable after I spelled them many times.

Quoted 1 line

Stas, what do you think? Could you test this?

I think I'll get to testing this only at a week-end.
In a mean time, the question about a safety of leaving LDT SS in 64bit mode still makes me wonder. Perhaps, instead of re-iterating this here, you can describe this all in the patch comments? Namely:
- How will LDT SS interact with nested signals
- with syscalls
- with siglongjmp()
- with another thread. Do we have a per-thread or a per-process LDT these days? If LDT is per-process, my question is what will happen if another thread invalidates an LDT entry while we are in 64bit mode. If LDT is per-thread, there is no such question.

Quoted 4 lines

If SS starts out invalid (this can happen if the signal was caused by an IRET fault or was delivered on the way out of set_thread_area or modify_ldt), then IRET to the signal handler can fail, eventually killing the task.

Is this signal-pecific? I.e. the return from IRQs happens via iret too. So if we are running with invalid SS in 64bit mode, can the iret from IRQ also cause the problem?

On an off-topic: there was recently a patch from you that disables vm86() by mmap_min_addr. I've found that dosemu, when started as root, could override mmap_min_addr. I guess this will no longer work, right? Not a big regression, just something to know and document.

Ingo Molnar

2015-10-14 15:02 UTC #5 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

Quoted 5 lines · Stas Sergeev

On an off-topic: there was recently a patch from you that disables vm86() by mmap_min_addr. I've found that dosemu, when started as root, could override mmap_min_addr. I guess this will no longer work, right? Not a big regression, just something to know and document.

So I think it should still work, because we check for mmap_min_addr in the system call itself:

static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) {
...

        err = security_mmap_addr(0);
        if (err) {
                /*

So if dosemu first tweaks mmap_min_addr, the syscall should succeed.

Thanks,

Ingo

Stas Sergeev

2015-10-14 15:18 UTC #6 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

14.10.2015 18:01, Ingo Molnar пишет:

Quoted 12 lines · Stas Sergeev

Quoted 6 lines
On an off-topic: there was recently a patch from you that disables vm86() by mmap_min_addr. I've found that dosemu, when started as root, could override mmap_min_addr. I guess this will no longer work, right? Not a big regression, just something to know and document.
So I think it should still work, because we check for mmap_min_addr in the system call itself:

Quoted 8 lines

static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) {
...
        err = security_mmap_addr(0);
        if (err) {
                /*
So if dosemu first tweaks mmap_min_addr, the syscall should succeed.

No, it doesn't tweak it.
It just seems that root is allowed to do mmap(0, MAP_FIXED) _regardless_ of mmap_min_addr. But it would be crazy to run vm86() as root to also bypass the check (dosemu drops privs earlier), so I guess this trick will stop working.

Cyrill Gorcunov

2015-10-14 16:40 UTC #20 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Andy Lutomirski

On Mon, Oct 12, 2015 at 06:04:07PM -0700, Andy Lutomirski wrote:
...

Quoted 8 lines

For the benefit of new 64-bit software that uses segmentation (new versions of DOSEMU might), the new behavior can be detected with a new ucontext flag UC_SIGCONTEXT_SS.
To avoid compilation issues, __pad0 is left as an alias for ss in ucontext.

Quoted 7 lines

The nitty-gritty details are documented in the header file.
Cc Stas Sergeev <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Cyrill Gorcunov <[email protected]>
Cc Pavel Emelyanov <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>

Andy, so for old criu versions (prior the 1.5.1 which is Mar 2015, in next versions we already write proper ss into the images) we've been providing __pad = 0, which is ss in a new meaning, and the kernel will overwrite it with @user-ds after this series, correct? This should work for us. Stas, mind to refresh my memory, which ss value doesmu setups here?

Andy Lutomirski

2015-10-14 16:41 UTC #7 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Stas Sergeev

Quoted 8 lines · Stas Sergeev

13.10.2015 04:04, Andy Lutomirski пишет:
Quoted 4 lines
+ * UC_SIGCONTEXT_SS will be set when delivering 64-bit or x32 signals on
+ * kernels that save SS in the sigcontext. All kernels that set
+ * UC_SIGCONTEXT_SS will correctly restore at least the low 32 bits of esp
+ * regardless of SS (i.e. they implement espfix).
Is this comment relevant? I think neither signal delivery nor sigreturn were affected by esp corruption, or were they?

Every IRET from the kernel to 16-bit mode was affected. That includes interrupt return, non-signaling page fault return, and all signal returns. So this could be a hint that, if you see a stack fault (#SS or page fault) that you don't need to try to work around the corruption issue.

Quoted 6 lines

I guess you suggest to use that flag as the detection
for espfix, but I don't think this is relevant: you may need to know about espfix also outside of a signal handler. In fact, I don't think espfix needs any run-time detection, because then the stack fault will simply not happen, and that's all. I think it is a matter of compile-time detection only.

True. In any event, there's no code involved - this is just an observation that all kernels with the new flag have espfix64.

Quoted 27 lines

Quoted 11 lines
+ *
+ * Kernels that set UC_SIGCONTEXT_SS will also set UC_STRICT_RESTORE_SS
+ * when delivering a signal that came from 64-bit code.
+ *
+ * Sigreturn modifies its behavior depending on the UC_STRICT_RESTORE_SS
+ * flag. If UC_STRICT_RESTORE_SS is set, then the SS value in the
+ * signal context is restored verbatim. If UC_STRICT_RESTORE_SS is not
+ * set, the CS value in the signal context refers to a 64-bit code
+ * segment, and the signal context's SS value is invalid, it will be
+ * replaced by an flat 32-bit selector.
Quoted 8 lines
+
+ * This behavior serves two purposes. It ensures that older programs
+ * that are unaware of the signal context's SS slot and either construct
+ * a signal context from scratch or that catch signals from segmented
+ * contexts and change CS to a 64-bit selector won't crash due to a bad
+ * SS value. It also ensures that signal handlers that do not modify
+ * the signal context at all return back to the exact CS and SS state
+ * that they came from.
Do you need a second flag for this?
IIRC non-restoring was needed because:
1. dosemu saves SS to different place
2. If you save it yourself, dosemu can invalidate it, but not replace with the right one because of 1.
IMHO to solve this, you need _either_ the second flag or the heuristic, but not both.

Quoted 6 lines

With new flag:
Just don't set it by default, and the new progs will set it themselves. Old progs are unaffected.
When it is set, SS should always be restored.
I prefer this approach.

The down side is that, if we do it that way, returning from a signal due to a bad SS will silently fix it, possibly with bad effects. That seems suboptimal. Most of the code is handling the flag on return, anyway -- setting it is straightforward.

Quoted 3 lines

With heuristic:
Save SS yourself on delivery, and, if it happens invalid on sigreturn - replace it with better one.

Then new DOSEMU will have to set the flag to get the expected behavior, which seems unfortunate. It also makes tests like sigreturn_64 much harder to write, which is unfortunate because, if there's an exploitable bug, attackers will still exploit it by some more roundabout means.

Quoted 2 lines

Old progs are unaffected because they use iret anyway, and that iret happens _after_ sigreturn.

Old progs = just DOSEMU here, I think.

Quoted 12 lines

New progs will never leave invalid SS in the right sigcontext slot.
So why have you choose to have both the new flag UC_STRICT_RESTORE_SS and the heuristic?
Quoted 2 lines
This is a bit risky, and another option would be to do nothing at all.
Andy, could you please stop pretending there are no other solutions? You do not have to like them. You do not have to implement them. But your continuous re-assertions that they do not exist, make me feel a bit uncomfortable after I spelled them many times.

Quoted 6 lines

Quoted 1 line
Stas, what do you think? Could you test this?
I think I'll get to testing this only at a week-end.
In a mean time, the question about a safety of leaving LDT SS in 64bit mode still makes me wonder. Perhaps, instead of re-iterating this here, you can describe this all in the patch comments? Namely:
- How will LDT SS interact with nested signals

The kernel doesn't think about nested signals. If the inner signal is delivered while SS is in the LDT, the kernel will try to keep it as is and will stick whatever was in SS when the signal happened in the inner saved context. On return to the outer signal, it'll restore it following the UC_STRICT_RESTORE_SS rules.

Quoted 1 line

- with syscalls

64-bit syscalls change SS to some default flat value as a side-effect. (Actually, IIRC, 64-bit syscalls change it specifically to __USER_DS, but, on Xen, 64-bit fast syscall returns may silently flip it to a different flat selector.)

Quoted 1 line

- with siglongjmp()

siglongjmp is a glibc thing. It should work the same way it always did. If it internally does a syscall (sigprocmask or whatever), that will override SS.

Quoted 4 lines

- with another thread. Do we have a per-thread or a per-process LDT these days? If LDT is per-process, my question is what will happen if another thread invalidates an LDT entry while we are in 64bit mode. If LDT is per-thread, there is no such question.

The LDT is per-process. If you have some SS value loaded and another thread invalidates it, then you get a signal delivered. On 64-bit kernels, it's the same SIGSEGV you'd get if you tried to directly load the bad SS value using IRET from user mode and, on 32-bit kernels, it's SIGILL. On kernels before 4.2 and that don't have the fix backported (IIRC), the signal may be non-deterministically deferred.

Quoted 9 lines

Quoted 5 lines
If SS starts out invalid (this can happen if the signal was caused by an IRET fault or was delivered on the way out of set_thread_area or modify_ldt), then IRET to the signal handler can fail, eventually killing the task.
Is this signal-pecific? I.e. the return from IRQs happens via iret too. So if we are running with invalid SS in 64bit mode, can the iret from IRQ also cause the problem?

On new kernels, you can't run with invalid SS under any conditions. On old kernels, you could, but only due to a modify_ldt race, and, if you did that, then you could get non-determinstically killed.

Quoted 6 lines

On an off-topic: there was recently a patch from you that disables vm86() by mmap_min_addr. I've found that dosemu, when started as root, could override mmap_min_addr. I guess this will no longer work, right? Not a big regression, just something to know and document.

As root, mmap_min_addr isn't enforced. Calling mmap and then dropping privileges would still keep the old mappings around. We could potentially rig it so that calling vm86 and then dropping privileges allows you to keep using vm86 even after dropping privileges.

--Andy

Andy Lutomirski

2015-10-14 16:43 UTC #21 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Cyrill Gorcunov

Quoted 18 lines · Cyrill Gorcunov

On Mon, Oct 12, 2015 at 06:04:07PM -0700, Andy Lutomirski wrote:
...
Quoted 8 lines
For the benefit of new 64-bit software that uses segmentation (new versions of DOSEMU might), the new behavior can be detected with a new ucontext flag UC_SIGCONTEXT_SS.
To avoid compilation issues, __pad0 is left as an alias for ss in ucontext.
Quoted 7 lines
The nitty-gritty details are documented in the header file.
Cc Stas Sergeev <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Cyrill Gorcunov <[email protected]>
Cc Pavel Emelyanov <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>

Quoted 6 lines

Andy, so for old criu versions (prior the 1.5.1 which is Mar 2015, in next versions we already write proper ss into the images) we've been providing __pad = 0, which is ss in a new meaning, and the kernel will overwrite it with @user-ds after this series, correct? This should work for us. Stas, mind to refresh my memory, which ss value doesmu setups here?

That's the intent.

If you write __pad = 0, don't set UC_STRICT_RESTORE_SS, and leave cs set to a 64-bit value, then the kernel will detect that 0 is not a valid SS and will fix it for you.

If you do write UC_STRICT_RESTORE_SS (e.g. if you saved on a new kernel and you restore the saved uc_flags), then you'll get a new signal delivered.

If you're restoring a 32-bit or 16-bit context, then none of the above applies, but I doubt that CRIU supports that anyway.

--Andy

Stas Sergeev

2015-10-14 16:57 UTC #23 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Cyrill Gorcunov

14.10.2015 19:40, Cyrill Gorcunov пишет:

Quoted 18 lines · Andy Lutomirski

...
Quoted 8 lines
For the benefit of new 64-bit software that uses segmentation (new versions of DOSEMU might), the new behavior can be detected with a new ucontext flag UC_SIGCONTEXT_SS.
To avoid compilation issues, __pad0 is left as an alias for ss in ucontext.
Quoted 7 lines
The nitty-gritty details are documented in the header file.
Cc Stas Sergeev <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Cyrill Gorcunov <[email protected]>
Cc Pavel Emelyanov <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>

Quoted 6 lines

Andy, so for old criu versions (prior the 1.5.1 which is Mar 2015, in next versions we already write proper ss into the images) we've been providing __pad = 0, which is ss in a new meaning, and the kernel will overwrite it with @user-ds after this series, correct? This should work for us. Stas, mind to refresh my memory, which ss value doesmu setups here?

Nothing.
Older dosemus didn't care about touching __pad0, so whatever kernel saves there, is still there, even when dosemu needs another value.
The problem starts to happen IIRC when dosemu invalidates the LDT entry that was previously saved by the kernel as an SS. IIRC this was causing the SIGSEGV right from sigreturn(). It is actually a bit annoying to have such bad code in kernel only for the sake of the older dosemu.

Cyrill Gorcunov

2015-10-14 16:57 UTC #22 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Andy Lutomirski

Quoted 10 lines · Andy Lutomirski

That's the intent.
If you write __pad = 0, don't set UC_STRICT_RESTORE_SS, and leave cs set to a 64-bit value, then the kernel will detect that 0 is not a valid SS and will fix it for you.
If you do write UC_STRICT_RESTORE_SS (e.g. if you saved on a new kernel and you restore the saved uc_flags), then you'll get a new signal delivered.

Yeah, I just wanna be sure I understood the patch correctly. Thanks!

Quoted 2 lines

If you're restoring a 32-bit or 16-bit context, then none of the above applies, but I doubt that CRIU supports that anyway.

True. I didn't implemented compat mode for criu yet, it's in todo.

Cyrill Gorcunov

2015-10-14 17:01 UTC #24 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Stas Sergeev

On Wed, Oct 14, 2015 at 07:57:02PM +0300, Stas Sergeev wrote:
...

Quoted 12 lines

Quoted 2 lines
correct? This should work for us. Stas, mind to refresh my memory, which ss value doesmu setups here?
Nothing.
Older dosemus didn't care about touching __pad0, so whatever kernel saves there, is still there, even when dosemu needs another value.
The problem starts to happen IIRC when dosemu invalidates the LDT entry that was previously saved by the kernel as an SS. IIRC this was causing the SIGSEGV right from sigreturn(). It is actually a bit annoying to have such bad code in kernel only for the sake of the older dosemu.

I see. Thanks a huge for info!

Stas Sergeev

2015-10-14 17:41 UTC #8 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

In reply to Andy Lutomirski

14.10.2015 19:40, Andy Lutomirski пишет:

Quoted 10 lines

Quoted 10 lines
Quoted 10 lines
+ *
+ * Kernels that set UC_SIGCONTEXT_SS will also set UC_STRICT_RESTORE_SS
+ * when delivering a signal that came from 64-bit code.
+ *
+ * Sigreturn modifies its behavior depending on the UC_STRICT_RESTORE_SS
+ * flag. If UC_STRICT_RESTORE_SS is set, then the SS value in the
+ * signal context is restored verbatim. If UC_STRICT_RESTORE_SS is not
+ * set, the CS value in the signal context refers to a 64-bit code
+ * segment, and the signal context's SS value is invalid, it will be
+ * replaced by an flat 32-bit selector.

Is this correct?
It says "64-bit code segment will use the 32-bit SS". I guess you mean 64-bit SS instead of a 32-bit?
Also it doesn't seem to be saying what happens if CS is 32-bit and SS is invalid (the flag is not set).

Quoted 14 lines

Quoted 7 lines
Quoted 2 lines
This is a bit risky, and another option would be to do nothing at all.
Andy, could you please stop pretending there are no other solutions? You do not have to like them. You do not have to implement them. But your continuous re-assertions that they do not exist, make me feel a bit uncomfortable after I spelled them many times.
Quoted 6 lines
Quoted 1 line
Stas, what do you think? Could you test this?
I think I'll get to testing this only at a week-end.
In a mean time, the question about a safety of leaving LDT SS in 64bit mode still makes me wonder. Perhaps, instead of re-iterating this here, you can describe this all in the patch comments? Namely:
- How will LDT SS interact with nested signals

Quoted 5 lines

The kernel doesn't think about nested signals. If the inner signal is delivered while SS is in the LDT, the kernel will try to keep it as is and will stick whatever was in SS when the signal happened in the inner saved context. On return to the outer signal, it'll restore it following the UC_STRICT_RESTORE_SS rules.

Good.

Quoted 6 lines

Quoted 1 line
- with syscalls
64-bit syscalls change SS to some default flat value as a side-effect. (Actually, IIRC, 64-bit syscalls change it specifically to __USER_DS, but, on Xen, 64-bit fast syscall returns may silently flip it to a different flat selector.)

Do we need this?
Maybe it should stop doing so?

Quoted 5 lines

Quoted 1 line
- with siglongjmp()
siglongjmp is a glibc thing. It should work the same way it always did. If it internally does a syscall (sigprocmask or whatever), that will override SS.

IMHO this side-effect needs to be documented somewhere.
I was scared about using it because I thought SS could be left bad. Why I think it IS the kernel's problem is because in an ideal world the sighandler should not run with LDT SS at all, so there will be no fear about a bad SS after siglongjmp(). And if the sigprocmask() will sometime stop validating SS, this can lead to surprises.

Quoted 10 lines

Quoted 8 lines
Quoted 4 lines
If SS starts out invalid (this can happen if the signal was caused by an IRET fault or was delivered on the way out of set_thread_area or modify_ldt), then IRET to the signal handler can fail, eventually killing the task.
Is this signal-pecific? I.e. the return from IRQs happens via iret too. So if we are running with invalid SS in 64bit mode, can the iret from IRQ also cause the problem?
On new kernels, you can't run with invalid SS under any conditions.

Good.

Quoted 10 lines

Quoted 5 lines
On an off-topic: there was recently a patch from you that disables vm86() by mmap_min_addr. I've found that dosemu, when started as root, could override mmap_min_addr. I guess this will no longer work, right? Not a big regression, just something to know and document.
As root, mmap_min_addr isn't enforced. Calling mmap and then dropping privileges would still keep the old mappings around. We could potentially rig it so that calling vm86 and then dropping privileges allows you to keep using vm86 even after dropping privileges.

Well, there is a special vm86() entry that is served just for checking its presence, so maybe this could indeed be done. Not that I find this very important. If you code up such a patch, I'll see about changing dosemu2 accordingly, but don't rush on this too much. :)

Andy Lutomirski

2015-10-14 18:06 UTC #9 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

Quoted 11 lines · Stas Sergeev

14.10.2015 19:40, Andy Lutomirski пишет:
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
+ *
+ * Kernels that set UC_SIGCONTEXT_SS will also set UC_STRICT_RESTORE_SS
+ * when delivering a signal that came from 64-bit code.
+ *
+ * Sigreturn modifies its behavior depending on the UC_STRICT_RESTORE_SS
+ * flag. If UC_STRICT_RESTORE_SS is set, then the SS value in the
+ * signal context is restored verbatim. If UC_STRICT_RESTORE_SS is not
+ * set, the CS value in the signal context refers to a 64-bit code
+ * segment, and the signal context's SS value is invalid, it will be
+ * replaced by an flat 32-bit selector.

Quoted 3 lines

Is this correct?
It says "64-bit code segment will use the 32-bit SS". I guess you mean 64-bit SS instead of a 32-bit?

There is no such thing as a 64-bit SS. The case this is guarding against is:

void handler(...) {
ctx->cs = [some 64-bit value];
modify_ldt(zap the old SS);
return;
}

Old DOSEMU does this IIUC. It's trying to switch back to 64-bit mode, and the value of SS that gets loaded into the SS selector doesn't matter, but something *valid* needs to be loaded. (Remember the weird ISA design: the SS descriptor is basically irrelevant in 64-bit mode, but it still has to be valid.) On old kernels, this works, because sigreturn zaps SS unconditionally. On new kernels, it'll be interpreted as an attempt to change CS and restore the old SS, but that SS is no longer valid. The fixup is to avoid sending a new signal and to instead do what DOSEMU expected.

Quoted 2 lines

Also it doesn't seem to be saying what happens if CS is 32-bit and SS is invalid (the flag is not set).

A new signal will be delivered. sigreturn doesn't modify its behavior in this case -- it does the default thing, which is to honor the SS in the saved context. So it will actually try to use that saved SS value, which will fail, causing SIGSEGV.

Quoted 22 lines

Quoted 15 lines
Quoted 8 lines
Quoted 3 lines
This is a bit risky, and another option would be to do nothing at all.
Andy, could you please stop pretending there are no other solutions? You do not have to like them. You do not have to implement them. But your continuous re-assertions that they do not exist, make me feel a bit uncomfortable after I spelled them many times.
Quoted 6 lines
Quoted 1 line
Stas, what do you think? Could you test this?
I think I'll get to testing this only at a week-end.
In a mean time, the question about a safety of leaving LDT SS in 64bit mode still makes me wonder. Perhaps, instead of re-iterating this here, you can describe this all in the patch comments? Namely:
- How will LDT SS interact with nested signals
Quoted 5 lines
The kernel doesn't think about nested signals. If the inner signal is delivered while SS is in the LDT, the kernel will try to keep it as is and will stick whatever was in SS when the signal happened in the inner saved context. On return to the outer signal, it'll restore it following the UC_STRICT_RESTORE_SS rules.
Good.

Quoted 9 lines

Quoted 6 lines
Quoted 1 line
- with syscalls
64-bit syscalls change SS to some default flat value as a side-effect. (Actually, IIRC, 64-bit syscalls change it specifically to __USER_DS, but, on Xen, 64-bit fast syscall returns may silently flip it to a different flat selector.)
Do we need this?
Maybe it should stop doing so?

It's a performance trick IIUC. I don't know enough about Xen's innards to know whether this could be cleaned up without incurring nasty overheads. As a guess, Xen doesn't want to change the MSR controlling the SYSRET selector layout when switching guests, so it uses a default value that doesn't match Linux's. Linux mostly ignores this, and it only really matters if user code cares which flat selector gets loaded.

This shouldn't have much effect on segmented programs, as they don't use SYSRET in segmented contexts.

Quoted 10 lines

Quoted 5 lines
Quoted 1 line
- with siglongjmp()
siglongjmp is a glibc thing. It should work the same way it always did. If it internally does a syscall (sigprocmask or whatever), that will override SS.
IMHO this side-effect needs to be documented somewhere.
I was scared about using it because I thought SS could be left bad. Why I think it IS the kernel's problem is because in an ideal world the sighandler should not run with LDT SS at all, so there will be no fear about a bad SS after siglongjmp().

I agree, but that ship sailed quite a few years ago :(

Quoted 2 lines

And if the sigprocmask() will
sometime stop validating SS, this can lead to surprises.

Not possible without ISA changes. The SYSCALL instruction itself forgets the old SS value.

In any event, there's not much the kernel can do about this. You could ask the glibc people to document some well-defined behavior in their man pages.

--Andy

Stas Sergeev

2015-10-14 18:34 UTC #10 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

14.10.2015 21:06, Andy Lutomirski пишет:

Quoted 6 lines

Quoted 2 lines
Also it doesn't seem to be saying what happens if CS is 32-bit and SS is invalid (the flag is not set).
A new signal will be delivered. sigreturn doesn't modify its behavior in this case -- it does the default thing, which is to honor the SS in the saved context.

Hmm, no, it didn't do this in the past for sure. It simply ignored SS, no matter to what mode it returns.

Quoted 2 lines

So it will actually try to use that saved SS
value, which will fail, causing SIGSEGV.

So it seems this logic assumes that when dosemu returns to 32bit, the previous SS is always still valid, am I right with the understanding? I.e. the one that kernel have saved on a signal delivery (because old dosemu does not overwrite it).
If it is so, I'd say this assumption is very risky and will likely not hold. But maybe I am missing the point.

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 1 line
- with siglongjmp()
siglongjmp is a glibc thing. It should work the same way it always did. If it internally does a syscall (sigprocmask or whatever), that will override SS.

Quoted 7 lines

Quoted 5 lines
IMHO this side-effect needs to be documented somewhere.
I was scared about using it because I thought SS could be left bad. Why I think it IS the kernel's problem is because in an ideal world the sighandler should not run with LDT SS at all, so there will be no fear about a bad SS after siglongjmp().
I agree, but that ship sailed quite a few years ago :(

Please, once again you are claiming there were no solutions proposed in that area. :(
If you didn't repeatedly ignore the SA_hyz solution, there will be the chance to do exactly that. But whatever.

Andy Lutomirski

2015-10-14 18:53 UTC #11 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

Quoted 10 lines · Stas Sergeev

14.10.2015 21:06, Andy Lutomirski пишет:
Quoted 6 lines
Quoted 2 lines
Also it doesn't seem to be saying what happens if CS is 32-bit and SS is invalid (the flag is not set).
A new signal will be delivered. sigreturn doesn't modify its behavior in this case -- it does the default thing, which is to honor the SS in the saved context.
Hmm, no, it didn't do this in the past for sure. It simply ignored SS, no matter to what mode it returns.

What I mean is: it has the behavior it would have normally on a new kernel, which is to honor the saved SS. I'll try to improve the comment.

Quoted 9 lines

Quoted 2 lines
So it will actually try to use that saved SS
value, which will fail, causing SIGSEGV.
So it seems this logic assumes that when dosemu returns to 32bit, the previous SS is always still valid, am I right with the understanding? I.e. the one that kernel have saved on a signal delivery (because old dosemu does not overwrite it).
If it is so, I'd say this assumption is very risky and will likely not hold. But maybe I am missing the point.

That's the assumption. If I understand correctly, though, old DOSEMU never actually returns to 32-bit using sigreturn in the first place, since old kernels gave no control over SS. Doesn't old DOSEMU always return to the 64-bit IRET trampoline?

--Andy

Stas Sergeev

2015-10-14 21:38 UTC #12 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

14.10.2015 21:52, Andy Lutomirski пишет:

Quoted 23 lines · Stas Sergeev

Quoted 9 lines
14.10.2015 21:06, Andy Lutomirski пишет:
Quoted 5 lines
Quoted 2 lines
Also it doesn't seem to be saying what happens if CS is 32-bit and SS is invalid (the flag is not set).
A new signal will be delivered. sigreturn doesn't modify its behavior in this case -- it does the default thing, which is to honor the SS in the saved context.
Hmm, no, it didn't do this in the past for sure. It simply ignored SS, no matter to what mode it returns.
What I mean is: it has the behavior it would have normally on a new kernel, which is to honor the saved SS. I'll try to improve the comment.
Quoted 9 lines
Quoted 2 lines
So it will actually try to use that saved SS
value, which will fail, causing SIGSEGV.
So it seems this logic assumes that when dosemu returns to 32bit, the previous SS is always still valid, am I right with the understanding? I.e. the one that kernel have saved on a signal delivery (because old dosemu does not overwrite it).
If it is so, I'd say this assumption is very risky and will likely not hold. But maybe I am missing the point.

Quoted 4 lines

That's the assumption. If I understand correctly, though, old DOSEMU never actually returns to 32-bit using sigreturn in the first place, since old kernels gave no control over SS. Doesn't old DOSEMU always return to the 64-bit IRET trampoline?

Ah, so the old progs simply never return to 32bit, so you implement the "Right Thing" (tm) for them, thanks. So the whole point of UC_STRICT_RESTORE_SS flag is not for the software to control it, but just for the kernel itself, so that it knows from whether 32 or 64 bit the signal came. This is probably quite undocumented in both the comments and the patch description, and I was confused because the approaches we discussed before, were targeted on the flag that is written by user-space. If this my understanding is correct and the flag is just an indication rather than a requested action, perhaps the name should be different, e.g. UC_SIG_FROM_32BIT or the like?
Anyway, this is minor. :)
I'll try to test the patch within a few days, thanks for you time!

Andy Lutomirski

2015-10-14 21:41 UTC #13 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

Quoted 35 lines · Stas Sergeev

14.10.2015 21:52, Andy Lutomirski пишет:
Quoted 29 lines · Stas Sergeev
Quoted 6 lines
14.10.2015 21:06, Andy Lutomirski пишет:
Quoted 4 lines
Quoted 3 lines
Also it doesn't seem to be saying what happens if CS is 32-bit and SS is invalid (the flag is not set).
Quoted 7 lines
Quoted 3 lines
A new signal will be delivered. sigreturn doesn't modify its behavior in this case -- it does the default thing, which is to honor the SS in the saved context.
Hmm, no, it didn't do this in the past for sure. It simply ignored SS, no matter to what mode it returns.
What I mean is: it has the behavior it would have normally on a new kernel, which is to honor the saved SS. I'll try to improve the comment.
Quoted 10 lines
Quoted 2 lines
So it will actually try to use that saved SS
value, which will fail, causing SIGSEGV.
So it seems this logic assumes that when dosemu returns to 32bit, the previous SS is always still valid, am I right with the understanding? I.e. the one that kernel have saved on a signal delivery (because old dosemu does not overwrite it).
If it is so, I'd say this assumption is very risky and will likely not hold. But maybe I am missing the point.
Quoted 4 lines
That's the assumption. If I understand correctly, though, old DOSEMU never actually returns to 32-bit using sigreturn in the first place, since old kernels gave no control over SS. Doesn't old DOSEMU always return to the 64-bit IRET trampoline?

Quoted 8 lines

Ah, so the old progs simply never return to 32bit, so you implement the "Right Thing" (tm) for them, thanks. So the whole point of UC_STRICT_RESTORE_SS flag is not for the software to control it, but just for the kernel itself, so that it knows from whether 32 or 64 bit the signal came. This is probably quite undocumented in both the comments and the patch description, and I was confused because the approaches we discussed before, were targeted on the flag that is written by user-space.

Yes, that's the idea. I'll improve the comments.

Quoted 6 lines

If this my
understanding is correct and the flag is just an indication rather than a requested action, perhaps the name should be different, e.g. UC_SIG_FROM_32BIT or the like?
Anyway, this is minor. :)
I'll try to test the patch within a few days, thanks for you time!

No problem. Thanks for being willing to test!

--Andy

Stas Sergeev

2015-10-18 13:37 UTC #14 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

15.10.2015 00:41, Andy Lutomirski пишет:

Quoted 7 lines

Quoted 6 lines
If this my
understanding is correct and the flag is just an indication rather than a requested action, perhaps the name should be different, e.g. UC_SIG_FROM_32BIT or the like?
Anyway, this is minor. :)
I'll try to test the patch within a few days, thanks for you time!
No problem. Thanks for being willing to test!

Hello Andy, I am unlucky at testing this.
dosemu doesn't even start for me on the git kernels. After a half day of debugging, it seems the kernel forgets to fill in the "err" field in the sigcontext struct when page fault occurs. That confuses the dosemu's instruction decoder.
Does this ring any bells?

Andy Lutomirski

2015-10-18 16:13 UTC #15 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

Quoted 11 lines · Stas Sergeev

15.10.2015 00:41, Andy Lutomirski пишет:
Quoted 9 lines
Quoted 7 lines
If this my
understanding is correct and the flag is just an indication rather than a requested action, perhaps the name should be different, e.g. UC_SIG_FROM_32BIT or the like?
Anyway, this is minor. :)
I'll try to test the patch within a few days, thanks for you time!
No problem. Thanks for being willing to test!

Quoted 7 lines

Hello Andy, I am unlucky at testing this.
dosemu doesn't even start for me on the git kernels. After a half day of debugging, it seems the kernel forgets to fill in the "err" field in the sigcontext struct when page fault occurs. That confuses the dosemu's instruction decoder.
Does this ring any bells?

No, but I can reproduce it on some kernels. Let me see if I can fix it, too.

--Andy

Signature · 2 lines

Andy Lutomirski
AMA Capital Management, LLC

Stas Sergeev

2015-10-18 16:29 UTC #16 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

18.10.2015 19:12, Andy Lutomirski пишет:

Quoted 9 lines · Stas Sergeev

Quoted 8 lines
15.10.2015 00:41, Andy Lutomirski пишет:
Quoted 7 lines
Quoted 6 lines
If this my
understanding is correct and the flag is just an indication rather than a requested action, perhaps the name should be different, e.g. UC_SIG_FROM_32BIT or the like?
Anyway, this is minor. :)
I'll try to test the patch within a few days, thanks for you time!
No problem. Thanks for being willing to test!

Quoted 8 lines

Quoted 7 lines
Hello Andy, I am unlucky at testing this.
dosemu doesn't even start for me on the git kernels. After a half day of debugging, it seems the kernel forgets to fill in the "err" field in the sigcontext struct when page fault occurs. That confuses the dosemu's instruction decoder.
Does this ring any bells?
No, but I can reproduce it on some kernels. Let me see if I can fix it, too.

Thanks!
You should really consider adding dosemu as your test-case. It feels very unhappy on all recent kernels. I was getting hard lock-ups under different circumstances (when starting windows, for example). But fedora-packaged kernels are quite good, as of yet. I fear the problems will soon populate to them too.

Andy Lutomirski

2015-10-18 16:36 UTC #17 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

Quoted 26 lines · Stas Sergeev

18.10.2015 19:12, Andy Lutomirski пишет:
Quoted 22 lines · Stas Sergeev
Quoted 12 lines
15.10.2015 00:41, Andy Lutomirski пишет:
Quoted 9 lines
Quoted 7 lines
If this my
understanding is correct and the flag is just an indication rather than a requested action, perhaps the name should be different, e.g. UC_SIG_FROM_32BIT or the like?
Anyway, this is minor. :)
I'll try to test the patch within a few days, thanks for you time!
No problem. Thanks for being willing to test!
Quoted 7 lines
Hello Andy, I am unlucky at testing this.
dosemu doesn't even start for me on the git kernels. After a half day of debugging, it seems the kernel forgets to fill in the "err" field in the sigcontext struct when page fault occurs. That confuses the dosemu's instruction decoder.
Does this ring any bells?
Quoted 2 lines
No, but I can reproduce it on some kernels. Let me see if I can fix it, too.

Quoted 6 lines

Thanks!
You should really consider adding dosemu as your test-case. It feels very unhappy on all recent kernels. I was getting hard lock-ups under different circumstances (when starting windows, for example). But fedora-packaged kernels are quite good, as of yet. I fear the problems will soon populate to them too.

I'll work on that. It's a lot easier to have a packaged set of tests that say yes/no, though, and it's handy when those tests are fully open-source and easy to run (DOSEMU, in contrast, requires the annoyingly impossible-to-compile freedos stuff iirc, and the interesting bits need other test programs). In any case, I'll add a self-test for the
err thing once I figure out what's going on.

--Andy

Signature · 2 lines

Andy Lutomirski
AMA Capital Management, LLC

Stas Sergeev

2015-10-18 16:47 UTC #18 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

18.10.2015 19:36, Andy Lutomirski пишет:

Quoted 26 lines · Stas Sergeev

Quoted 17 lines
18.10.2015 19:12, Andy Lutomirski пишет:
Quoted 16 lines · Stas Sergeev
Quoted 15 lines
15.10.2015 00:41, Andy Lutomirski пишет:
Quoted 7 lines
Quoted 6 lines
If this my
understanding is correct and the flag is just an indication rather than a requested action, perhaps the name should be different, e.g. UC_SIG_FROM_32BIT or the like?
Anyway, this is minor. :)
I'll try to test the patch within a few days, thanks for you time!
No problem. Thanks for being willing to test!
Hello Andy, I am unlucky at testing this.
dosemu doesn't even start for me on the git kernels. After a half day of debugging, it seems the kernel forgets to fill in the "err" field in the sigcontext struct when page fault occurs. That confuses the dosemu's instruction decoder.
Does this ring any bells?
Quoted 8 lines
Quoted 2 lines
No, but I can reproduce it on some kernels. Let me see if I can fix it, too.
Thanks!
You should really consider adding dosemu as your test-case. It feels very unhappy on all recent kernels. I was getting hard lock-ups under different circumstances (when starting windows, for example). But fedora-packaged kernels are quite good, as of yet. I fear the problems will soon populate to them too.

Quoted 7 lines

I'll work on that. It's a lot easier to have a packaged set of tests that say yes/no, though, and it's handy when those tests are fully open-source and easy to run (DOSEMU, in contrast, requires the annoyingly impossible-to-compile freedos stuff iirc, and the interesting bits need other test programs). In any case, I'll add a self-test for the
err thing once I figure out what's going on.

I meant just a local test-case on your PC. :) It can't be a part of an automated test-suit of course. But after changing anything in vm86 or signal handling, running dosemu will never hurt, as it is probably the most rigorous test-case for them. I deduce that based on its breakage frequency with the kernel updates.

Andy Lutomirski

2015-10-18 17:06 UTC #19 lore raw

Re: [RFC 3/4] x86/signal/64: Re-add support for SS in the 64-bit signal context

Quoted 45 lines · Stas Sergeev

18.10.2015 19:36, Andy Lutomirski пишет:
Quoted 35 lines · Stas Sergeev
Quoted 27 lines
18.10.2015 19:12, Andy Lutomirski пишет:
Quoted 22 lines · Stas Sergeev
Quoted 12 lines
15.10.2015 00:41, Andy Lutomirski пишет:
Quoted 9 lines
Quoted 7 lines
If this my
understanding is correct and the flag is just an indication rather than a requested action, perhaps the name should be different, e.g. UC_SIG_FROM_32BIT or the like?
Anyway, this is minor. :)
I'll try to test the patch within a few days, thanks for you time!
No problem. Thanks for being willing to test!
Quoted 7 lines
Hello Andy, I am unlucky at testing this.
dosemu doesn't even start for me on the git kernels. After a half day of debugging, it seems the kernel forgets to fill in the "err" field in the sigcontext struct when page fault occurs. That confuses the dosemu's instruction decoder.
Does this ring any bells?
Quoted 2 lines
No, but I can reproduce it on some kernels. Let me see if I can fix it, too.
Quoted 6 lines
Thanks!
You should really consider adding dosemu as your test-case. It feels very unhappy on all recent kernels. I was getting hard lock-ups under different circumstances (when starting windows, for example). But fedora-packaged kernels are quite good, as of yet. I fear the problems will soon populate to them too.
Quoted 7 lines
I'll work on that. It's a lot easier to have a packaged set of tests that say yes/no, though, and it's handy when those tests are fully open-source and easy to run (DOSEMU, in contrast, requires the annoyingly impossible-to-compile freedos stuff iirc, and the interesting bits need other test programs). In any case, I'll add a self-test for the
err thing once I figure out what's going on.

Quoted 6 lines

I meant just a local test-case on your PC. :) It can't be a part of an automated test-suit of course. But after changing anything in vm86 or signal handling, running dosemu will never hurt, as it is probably the most rigorous test-case for them. I deduce that based on its breakage frequency with the kernel updates.

Point taken.

Anyway, the bug was an error in the patches I sent out and aren't in anyone's tree yet, so there's no big worry here. I'll send out new patches after incorporating feedback and poking at it a bit.

--Andy