x86: sigcontext fixes, again

PATCH Superseded Andy Lutomirski 5 participants 22 messages Oct 26, 2015 – Dec 29, 2015

Subsystems (2)

x86 Kselftest

lore

Share & tools

lore

Andy Lutomirski

author

2015-10-26 01:25 UTC #0 lore raw

v2 cover x86: sigcontext fixes, again

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, this now seems to pass the test you sent me. It works with stock dosemu2 (I haven't tested classic dosemu because I can't get it to work regardless). It also works with a patched dosemu2 that bypasses the userspace trampoline:

https://github.com/amluto/dosemu2/commit/571b4d08dc885b7a133e444a2ad23e0d21366206

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.

Changes from v1:
- Comment fixes
- Fix screwed up uaccess that broke things

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    |  43 +++++-
 arch/x86/kernel/signal.c                | 114 ++++++++++++---
 tools/testing/selftests/x86/Makefile    |   4 +-
 tools/testing/selftests/x86/sigreturn.c | 240 ++++++++++++++++++++++++++++----
 8 files changed, 391 insertions(+), 59 deletions(-)

Signature · 1 line

2.4.3

Andy Lutomirski

author

2015-10-26 01:25 UTC #1 lore raw

PATCH v2 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-26 01:25 UTC #6 lore raw

PATCH v2 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

Andy Lutomirski

author

2015-10-26 01:25 UTC #7 lore raw

PATCH v2 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   | 43 ++++++++++++++++++++---
 arch/x86/kernel/signal.c               | 63 ++++++++++++++++++++++++----------
 tools/testing/selftests/x86/Makefile   |  4 +--
 6 files changed, 89 insertions(+), 29 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 +38 -5

49 lines not on this page · raw message

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

137 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-26 01:26 UTC #9 lore raw

PATCH v2 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

Stas Sergeev

2015-10-26 11:46 UTC #10 lore raw

In reply to Andy Lutomirski

26.10.2015 04:25, Andy Lutomirski пишет:

Quoted 8 lines

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, this now seems to pass the test you sent me. It works with stock dosemu2 (I haven't tested classic dosemu because I can't get it to work regardless).

I'll test it myself then.
But this will have to wait till a week-end I am afraid. In a mean time you can test vm86() - last time I tried, I got oops and hard lockup.

Quoted 10 lines

It also works with a patched dosemu2 that bypasses the userspace trampoline:
https://github.com/amluto/dosemu2/commit/571b4d08dc885b7a133e444a2ad23e0d21366206
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

No, another option is a new SA_ flag.

Andy Lutomirski

2015-10-27 00:53 UTC #11 lore raw

Quoted 6 lines · Stas Sergeev

26.10.2015 04:25, Andy Lutomirski пишет:
Quoted 5 lines
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.

Quoted 7 lines

Quoted 3 lines
Stas, this now seems to pass the test you sent me. It works with stock dosemu2 (I haven't tested classic dosemu because I can't get it to work regardless).
I'll test it myself then.
But this will have to wait till a week-end I am afraid. In a mean time you can test vm86() - last time I tried, I got oops and hard lockup.

Can you tell me exactly what kernel version (release by Linus or commit hash) oopses and, if it's easy, post a screenshot or log?

--Andy

Stas Sergeev

2015-10-27 14:05 UTC #12 lore raw

27.10.2015 03:52, Andy Lutomirski пишет:

Quoted 10 lines · Stas Sergeev

Quoted 9 lines
26.10.2015 04:25, Andy Lutomirski пишет:
Quoted 8 lines
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, this now seems to pass the test you sent me. It works with stock dosemu2 (I haven't tested classic dosemu because I can't get it to work regardless).

Quoted 7 lines

Quoted 4 lines
I'll test it myself then.
But this will have to wait till a week-end I am afraid. In a mean time you can test vm86() - last time I tried, I got oops and hard lockup.
Can you tell me exactly what kernel version (release by Linus or commit hash) oopses and, if it's easy, post a screenshot or log?

I archived my config and git hash.
I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321

Linus Torvalds

2015-10-27 22:37 UTC #13 lore raw

Quoted 8 lines · Stas Sergeev

I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321

Hmm. Andrew Morton responded to that initially, but then nothing happened, and now it's been another six months. Andrew?

The arch/x86/mm/pat.c error handling does seem to be suspect. This is all code several years old, so none of this is new, and I think Suresh is gone. Adding a few other people with recent sign-offs to that file, in the hope that somebody feels like they own it..

Linus

Toshi Kani

2015-10-28 00:08 UTC #14 lore raw

Quoted 10 lines · Linus Torvalds

Quoted 8 lines · Stas Sergeev
I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321

Quoted 7 lines

Hmm. Andrew Morton responded to that initially, but then nothing happened, and now it's been another six months. Andrew?
The arch/x86/mm/pat.c error handling does seem to be suspect. This is all code several years old, so none of this is new, and I think Suresh is gone. Adding a few other people with recent sign-offs to that file, in the hope that somebody feels like they own it..

In the case of PFNMAP, the range should always be mapped. So, I wonder why follow_phys() failed with the !pte_present() check.

Stas, do you have a test program that can reproduce 97321?

-Toshi

Stas Sergeev

2015-10-28 09:53 UTC #15 lore raw

28.10.2015 03:04, Toshi Kani пишет:

Quoted 19 lines · Linus Torvalds

Quoted 10 lines · Stas Sergeev
Quoted 8 lines
I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321
Quoted 7 lines
Hmm. Andrew Morton responded to that initially, but then nothing happened, and now it's been another six months. Andrew?
The arch/x86/mm/pat.c error handling does seem to be suspect. This is all code several years old, so none of this is new, and I think Suresh is gone. Adding a few other people with recent sign-offs to that file, in the hope that somebody feels like they own it..

Quoted 4 lines

In the case of PFNMAP, the range should always be mapped. So, I wonder why follow_phys() failed with the !pte_present() check.
Stas, do you have a test program that can reproduce 97321?

Get dosemu2 from here:
https://github.com/stsp/dosemu2/releases
or from git, or get dosemu1.
Then boot your kernel with "nomodeset=1" to get a text console. Run

dosemu -s

and you'll get the bug.
You don't even need to mess with FreeDOS or whatever, because the problem happens on a very start. And since it is WARN_OCNCE, you'll need to reboot to get it again.
You will probably need to adjust /etc/sudoers because -s means that dosemu (a wrapper script for dosemu.bin) will try sudo so that dosemu.bin can grab /dev/mem as root.

If it is too complicated, let me know and I'll try to code up a reduced test-case (but can't promise to get to that quickly).

Thanks to everyone (esp Linus of course :-) who got involved in finally supporting dosemu again - it was rather ill for the last years.

Toshi Kani

2015-10-28 16:38 UTC #16 lore raw

Quoted 42 lines · Stas Sergeev

28.10.2015 03:04, Toshi Kani пишет:
Quoted 20 lines · Linus Torvalds
Quoted 10 lines · Stas Sergeev
Quoted 8 lines
I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321
Quoted 8 lines
Hmm. Andrew Morton responded to that initially, but then nothing happened, and now it's been another six months. Andrew?
The arch/x86/mm/pat.c error handling does seem to be suspect. This is all code several years old, so none of this is new, and I think Suresh
is gone. Adding a few other people with recent sign-offs to that file, in the hope that somebody feels like they own it..
Quoted 5 lines
In the case of PFNMAP, the range should always be mapped. So, I wonder why
follow_phys() failed with the !pte_present() check.
Stas, do you have a test program that can reproduce 97321?
Get dosemu2 from here:
https://github.com/stsp/dosemu2/releases
or from git, or get dosemu1.
Then boot your kernel with "nomodeset=1" to get a text console. Run
dosemu -s
and you'll get the bug.
You don't even need to mess with FreeDOS or whatever, because the problem happens on a very start. And since it is WARN_OCNCE, you'll need to reboot to get it again.
You will probably need to adjust /etc/sudoers because -s means that dosemu (a wrapper script for dosemu.bin) will try sudo so that dosemu.bin can grab /dev/mem as root.

Quoted 6 lines

If it is too complicated, let me know and I'll try to code up a reduced test-case (but can't promise to get to that quickly).
Thanks to everyone (esp Linus of course :-) who got involved in finally supporting dosemu again - it was rather ill for the last years.

I compiled "pre4" dosemu2, and ran on 4.3-rc7 kernel booted with the "nomodeset=1" option. I may be missing something, but I am not able to reproduce your problem because dosemu fails with the segfault below on my system...

Program terminated with signal SIGSEGV, Segmentation fault. #0 0x0000000000421fdb in video_init () at video.c:242
242 if (Video->priv_init)

(gdb) bt

#0  0x0000000000421fdb in video_init () at video.c:242
#1  video_config_init () at video.c:391
#2  0x00000000004b5693 in device_init () at init.c:237
#3  0x000000000041ea3a in main (argc=3, argv=0x7fff70851d28) at emu.c:356

(gdb) p Video
$1 = (struct video_system *) 0x0

Thanks,
-Toshi

Toshi Kani

2015-10-28 19:26 UTC #17 lore raw

Quoted 32 lines · Toshi Kani

Quoted 31 lines · Stas Sergeev
28.10.2015 03:04, Toshi Kani пишет:
Quoted 20 lines · Linus Torvalds
Quoted 11 lines
Quoted 8 lines · Stas Sergeev
I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321
Quoted 7 lines
Hmm. Andrew Morton responded to that initially, but then nothing happened, and now it's been another six months. Andrew?
The arch/x86/mm/pat.c error handling does seem to be suspect. This is all code several years old, so none of this is new, and I think Suresh is gone. Adding a few other people with recent sign-offs to that file, in the hope that somebody feels like they own it..
Quoted 4 lines
In the case of PFNMAP, the range should always be mapped. So, I wonder why follow_phys() failed with the !pte_present() check.
Stas, do you have a test program that can reproduce 97321?
Get dosemu2 from here:
https://github.com/stsp/dosemu2/releases
or from git, or get dosemu1.
Then boot your kernel with "nomodeset=1" to get a text console. Run

Quoted 3 lines

Quoted 3 lines
dosemu -s
and you'll get the bug.

I looked at the dosemu code and was able to reproduce the issue with a test program. This problem happens when mremap() to /dev/mem (or PFNMAP) is called with MREMAP_FIXED.

In this case, mremap calls move_vma(), which first calls move_page_tables() to remap the translation and then calls do_munmap() to remove the original mapping. Hence, when untrack_pfn() is called from do_munmap(), the original map is already removed, and follow_phys() fails with the
!pte_present() check.

I think there are a couple of issues:
- If track_pfn() ignores an error from follow_phys() and skips
free_pfn_range(), PAT continues to track the original map that is removed.
- track_pfn() calls free_pfn_range() to untrack a given free range. However, rbt_memtype_erase() requires the free range match exactly to the
tracked range. This does not support mremap, which needs to free up part of the tracked range.
- PAT does not track a new translation specified by mremap() with MREMAP_F
IXED.

Thanks,
-Toshi

Toshi Kani

2015-10-28 22:55 UTC #18 lore raw

Quoted 47 lines · Toshi Kani

Quoted 32 lines · Toshi Kani
Quoted 31 lines · Stas Sergeev
28.10.2015 03:04, Toshi Kani пишет:
Quoted 20 lines · Linus Torvalds
Quoted 11 lines
Quoted 8 lines · Stas Sergeev
I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321
Quoted 7 lines
Hmm. Andrew Morton responded to that initially, but then nothing happened, and now it's been another six months. Andrew?
The arch/x86/mm/pat.c error handling does seem to be suspect. This is all code several years old, so none of this is new, and I think Suresh is gone. Adding a few other people with recent sign-offs to that file, in the hope that somebody feels like they own it..
Quoted 4 lines
In the case of PFNMAP, the range should always be mapped. So, I wonder why follow_phys() failed with the !pte_present() check.
Stas, do you have a test program that can reproduce 97321?
Get dosemu2 from here:
https://github.com/stsp/dosemu2/releases
or from git, or get dosemu1.
Then boot your kernel with "nomodeset=1" to get a text console. Run
Quoted 3 lines
Quoted 3 lines
dosemu -s
and you'll get the bug.
I looked at the dosemu code and was able to reproduce the issue with a test program. This problem happens when mremap() to /dev/mem (or PFNMAP) is called with MREMAP_FIXED.
In this case, mremap calls move_vma(), which first calls move_page_tables() to remap the translation and then calls do_munmap() to remove the original mapping. Hence, when untrack_pfn() is called from do_munmap(), the original map is already removed, and follow_phys() fails with the
!pte_present() check.

Quoted 9 lines

I think there are a couple of issues:
- If untrack_pfn() ignores an error from follow_phys() and skips
free_pfn_range(), PAT continues to track the original map that is removed.
- untrack_pfn() calls free_pfn_range() to untrack a given free range. However, rbt_memtype_erase() requires the free range match exactly to the
tracked range. This does not support mremap, which needs to free up part of the tracked range.
- PAT does not track a new translation specified by mremap() with MREMAP_F
IXED.

Thinking further, I think the 1st and 3rd items are non-issues. mremap remaps virtual address, but keeps the same cache type and pfns. So, PAT does not have to change the tracked pfns in this case. The 2nd item is still a problem, though.

Thanks,
-Toshi

Andy Lutomirski

2015-10-30 23:50 UTC #21 lore raw

In reply to Stas Sergeev

Quoted 18 lines · Stas Sergeev

27.10.2015 03:52, Andy Lutomirski пишет:
Quoted 10 lines · Stas Sergeev
Quoted 9 lines
26.10.2015 04:25, Andy Lutomirski пишет:
Quoted 8 lines
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, this now seems to pass the test you sent me. It works with stock dosemu2 (I haven't tested classic dosemu because I can't get it to work regardless).
Quoted 7 lines
Quoted 4 lines
I'll test it myself then.
But this will have to wait till a week-end I am afraid. In a mean time you can test vm86() - last time I tried, I got oops and hard lockup.
Can you tell me exactly what kernel version (release by Linus or commit hash) oopses and, if it's easy, post a screenshot or log?

Quoted 8 lines

I archived my config and git hash.
I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321

Like this?

[  288.221786] BUG: unable to handle kernel paging request at ffffffb9
[  288.222475] IP: [<c169bf48>] snd_seq_delete_port+0x48/0xd0
[  288.222743] *pde = 01c8c067 *pte = 00000000
[  288.222743] Oops: 0000 [#1] SMP
[  288.222743] Modules linked in:
[  288.222743] CPU: 0 PID: 5480 Comm: dosemu.bin Not tainted 4.3.0-rc7+ #345
[  288.222743] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
[  288.222743] task: c7006b40 ti: c7bb4000 task.ti: c7bb4000
[  288.222743] EIP: 0060:[<c169bf48>] EFLAGS: 00010082 CPU: 0
[  288.222743] EIP is at snd_seq_delete_port+0x48/0xd0
[  288.222743] EAX: 00000000 EBX: ffffffb8 ECX: c707c67c EDX: 00000001
[  288.222743] ESI: c707c600 EDI: c707c684 EBP: c7bb5d60 ESP: c7bb5d48
[  288.222743]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[  288.222743] CR0: 80050033 CR2: ffffffb9 CR3: 07b00000 CR4: 000406d0
[  288.222743] Stack:
[  288.222743]  00000001 00000246 c707c68c c707c600 40a45321 c7bb5ee0
c7bb5e14 c16965cb
[  288.222743]  0000010f 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[  288.222743]  00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[  288.222743] Call Trace:
[  288.222743]  [<c16965cb>] snd_seq_ioctl_delete_port+0x3b/0x90
[  288.222743]  [<c1696c65>] snd_seq_do_ioctl+0x85/0x90
[  288.222743]  [<c1696ca3>] snd_seq_kernel_client_ctl+0x33/0x50
[  288.222743]  [<c169b78b>] snd_seq_event_port_detach+0x3b/0x50
[  288.222743]  [<c169d6a2>] delete_port+0x12/0x30
[  288.222743]  [<c169dbc1>] snd_seq_oss_release+0x41/0x50
[  288.222743]  [<c169d406>] odev_release+0x26/0x40
[  288.222743]  [<c11a46a3>] __fput+0xc3/0x1d0
[  288.222743]  [<c11a47e8>] ____fput+0x8/0x10
[  288.222743]  [<c10b924f>] task_work_run+0x6f/0x90
[  288.222743]  [<c10017e5>] prepare_exit_to_usermode+0xd5/0x100
[  288.222743]  [<c1001841>] syscall_return_slowpath+0x31/0x120
[  288.222743]  [<c11bd094>] ? __close_fd+0x54/0x70
[  288.222743]  [<c188b372>] syscall_exit_work+0x7/0xc
[  288.222743] Code: 5f d0 1e 00 89 f8 e8 68 f0 1e 00 89 45 ec 8b 46
7c 8d 4e 7c 39 c1 74 25 8d 58 b8 0f b6 40 b9 8b 55 e8 39 d0 75 0d eb
3b 8d 76 00 <0f> b6 40 b9 39 d0 74 30 8b 43 48 39 c1 8d 58 b8 75 ee 8b
55 ec
[  288.222743] EIP: [<c169bf48>] snd_seq_delete_port+0x48/0xd0 SS:ESP
0068:c7bb5d48
[  288.222743] CR2: 00000000ffffffb9
[  288.222743] ---[ end trace f216bf40eb9b39d6 ]---

I'll try to narrow that down a little bit and email the appropriate maintainer.

--Andy

Stas Sergeev

2015-10-31 12:43 UTC #19 lore raw

In reply to Toshi Kani

29.10.2015 01:51, Toshi Kani пишет:

Quoted 53 lines · Toshi Kani

Quoted 43 lines · Toshi Kani
Quoted 29 lines · Stas Sergeev
Quoted 28 lines
28.10.2015 03:04, Toshi Kani пишет:
Quoted 13 lines · Linus Torvalds
Quoted 12 lines
Quoted 7 lines · Stas Sergeev
I can't easily post an Oops: under X it doesn't even appear - machine freezes immediately, and under non-KMS console it is possible to get one, but difficult to screen-shot (using bare metal, not VM). Also the Oops was seemingly unrelated. And if you run "dosemu -s" under non-KMS console, you'll also reproduce this one:
https://bugzilla.kernel.org/show_bug.cgi?id=97321
Hmm. Andrew Morton responded to that initially, but then nothing happened, and now it's been another six months. Andrew?
Quoted 8 lines
Quoted 4 lines
The arch/x86/mm/pat.c error handling does seem to be suspect. This is all code several years old, so none of this is new, and I think Suresh is gone. Adding a few other people with recent sign-offs to that file, in the hope that somebody feels like they own it..
In the case of PFNMAP, the range should always be mapped. So, I wonder why follow_phys() failed with the !pte_present() check.
Stas, do you have a test program that can reproduce 97321?
Get dosemu2 from here:
https://github.com/stsp/dosemu2/releases
or from git, or get dosemu1.
Then boot your kernel with "nomodeset=1" to get a text console. Run
Quoted 3 lines
Quoted 3 lines
dosemu -s
and you'll get the bug.
I looked at the dosemu code and was able to reproduce the issue with a test program. This problem happens when mremap() to /dev/mem (or PFNMAP) is called with MREMAP_FIXED.
In this case, mremap calls move_vma(), which first calls move_page_tables() to remap the translation and then calls do_munmap() to remove the original mapping. Hence, when untrack_pfn() is called from do_munmap(), the original map is already removed, and follow_phys() fails with the
!pte_present() check.
Quoted 9 lines
I think there are a couple of issues:
- If untrack_pfn() ignores an error from follow_phys() and skips
free_pfn_range(), PAT continues to track the original map that is removed.
- untrack_pfn() calls free_pfn_range() to untrack a given free range. However, rbt_memtype_erase() requires the free range match exactly to the
tracked range. This does not support mremap, which needs to free up part of the tracked range.
- PAT does not track a new translation specified by mremap() with MREMAP_F
IXED.

Quoted 4 lines

Thinking further, I think the 1st and 3rd items are non-issues. mremap remaps virtual address, but keeps the same cache type and pfns. So, PAT does not have to change the tracked pfns in this case. The 2nd item is still a problem, though.

Hello Toshi, thanks for your analysis.
Now as you do not seem to be preparing a fix, how about attaching your test-case to the bug-report for others to re-use? Or maybe you can even make it a part of the kernel's test suit - I suppose this will help.

Stas Sergeev

Tested-by on v2 3/4

2015-10-31 15:19 UTC #8 lore raw

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

In reply to Andy Lutomirski

26.10.2015 04:25, Andy Lutomirski пишет:

Quoted 50 lines

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.

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]>

Tested-by Stas Sergeev <[email protected]>

Both dosemu2 and 1 work fine.
Still as this approach seems very risky, I'd appreciate if some of the explanations from this msg:
https://lkml.org/lkml/2015/10/14/726
go into the code comments.
What I find especially useful are your notes about siglongjmp(), threads, and the fact that running with bad SS is now impossible on the recent kernels. Without this info at hands, no one can trust this code perhaps.

Stas Sergeev

2015-10-31 15:25 UTC #2 lore raw

Re: [PATCH v2 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs

In reply to Andy Lutomirski

26.10.2015 04:25, Andy Lutomirski пишет:

Quoted 7 lines

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]>

Now the strategy about SS is to always save it to the sigcontext. This is good because the syscall can clobber it, so the app had to be very careful in the past trying to save it by hands. How about saving also fs and gs? (without restoring yet) If you do, you'll save dosemu a headache of doing any "pre-syscall" work. Pre-syscall work is very nasty.
I haven't checked if the syscall clobber also these or only SS, but perhaps saving them by the kernel won't hurt?

Toshi Kani

2015-11-02 17:05 UTC #20 lore raw

In reply to Stas Sergeev

Quoted 3 lines · Stas Sergeev

29.10.2015 01:51, Toshi Kani пишет:
Quoted 2 lines · Toshi Kani
Quoted 1 line · Toshi Kani

Quoted 23 lines

Quoted 19 lines
Quoted 10 lines
I looked at the dosemu code and was able to reproduce the issue with a test program. This problem happens when mremap() to /dev/mem (or PFNMAP) is called with MREMAP_FIXED.
In this case, mremap calls move_vma(), which first calls move_page_tables() to remap the translation and then calls do_munmap() to remove the original mapping. Hence, when untrack_pfn() is called from do_munmap(), the original map is already removed, and follow_phys() fails with the !pte_present() check.
Quoted 9 lines
I think there are a couple of issues:
- If untrack_pfn() ignores an error from follow_phys() and skips
free_pfn_range(), PAT continues to track the original map that is removed.
- untrack_pfn() calls free_pfn_range() to untrack a given free range. However, rbt_memtype_erase() requires the free range match exactly to
the tracked range. This does not support mremap, which needs to free up part of the tracked range.
- PAT does not track a new translation specified by mremap() with
MREMAP_FIXED.
Quoted 4 lines
Thinking further, I think the 1st and 3rd items are non-issues. mremap remaps virtual address, but keeps the same cache type and pfns. So, PAT does not have to change the tracked pfns in this case. The 2nd item is still a problem, though.

Quoted 5 lines

Hello Toshi, thanks for your analysis.
Now as you do not seem to be preparing a fix, how about attaching your test-case to the bug-report for others to re-use? Or maybe you can even make it a part of the kernel's test suit - I suppose this will help.

I can work on the fix, but I did not think we needed to rush on it since this issue exists for a long time. Can we target it to 4.4-rcX and then cc to stables?

As for the test, it might be tricky to make it run on any platforms since it needs to mmap/mremap a non-RAM range. It could just map a reserved range randomly, but I chose a range manually which I know is safe to map & read/write on my platform in my test.

Thanks,
-Toshi

Andy Lutomirski

2015-12-07 23:23 UTC #3 lore raw

Re: [PATCH v2 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs

In reply to Stas Sergeev

Quoted 11 lines · Stas Sergeev

26.10.2015 04:25, Andy Lutomirski пишет:
Quoted 7 lines
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]>

Quoted 8 lines

Now the strategy about SS is to always save it to the sigcontext. This is good because the syscall can clobber it, so the app had to be very careful in the past trying to save it by hands. How about saving also fs and gs? (without restoring yet) If you do, you'll save dosemu a headache of doing any "pre-syscall" work. Pre-syscall work is very nasty.
I haven't checked if the syscall clobber also these or only SS, but perhaps saving them by the kernel won't hurt?

There's a bunch of ongoing work about FS and GS. I want to wait and see.

Stas Sergeev

2015-12-29 12:25 UTC #4 lore raw

Re: [PATCH v2 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs

08.12.2015 02:23, Andy Lutomirski пишет:

Quoted 21 lines · Stas Sergeev

Quoted 11 lines
26.10.2015 04:25, Andy Lutomirski пишет:
Quoted 7 lines
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]>
Quoted 8 lines
Now the strategy about SS is to always save it to the sigcontext. This is good because the syscall can clobber it, so the app had to be very careful in the past trying to save it by hands. How about saving also fs and gs? (without restoring yet) If you do, you'll save dosemu a headache of doing any "pre-syscall" work. Pre-syscall work is very nasty.
I haven't checked if the syscall clobber also these or only SS, but perhaps saving them by the kernel won't hurt?

Quoted 1 line

There's a bunch of ongoing work about FS and GS. I want to wait and see.

Andy, have you postponed only this, or also the sigreturn patch? Long time no news...
sigreturn patch would really be nice to have.

Andy Lutomirski

2015-12-29 12:32 UTC #5 lore raw

Re: [PATCH v2 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs

Quoted 22 lines · Stas Sergeev

08.12.2015 02:23, Andy Lutomirski пишет:
Quoted 21 lines · Stas Sergeev
Quoted 19 lines
26.10.2015 04:25, Andy Lutomirski пишет:
Quoted 7 lines
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]>
Now the strategy about SS is to always save it to the sigcontext. This is good because the syscall can clobber it, so the app had to be very careful in the past trying to save it by hands. How about saving also fs and gs? (without restoring yet) If you do, you'll save dosemu a headache of doing any "pre-syscall" work. Pre-syscall work is very nasty.
I haven't checked if the syscall clobber also these or only SS, but perhaps saving them by the kernel won't hurt?

Quoted 4 lines

Quoted 1 line
There's a bunch of ongoing work about FS and GS. I want to wait and see.
Andy, have you postponed only this, or also the sigreturn patch? Long time no news...
sigreturn patch would really be nice to have.

Sorry, I got caught up with a bunch of other stuff, and everyone being slow with the holiday season. There's some slight chance we'll get it in for 4.5, but 4.6 is more likely.

--Andy

Signature · 2 lines

Andy Lutomirski
AMA Capital Management, LLC