[RFC 0/4] x86: sigcontext SS fixes, take 2

[RFC 0/4] x86: sigcontext SS fixes, take 2

[Andy Lutomirski]

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(-)

-- 
2.4.3

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

[Andy Lutomirski]

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 <luto@kernel.org>
---
 arch/x86/include/uapi/asm/sigcontext.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h
index 40836a9a7250..827c6ed0e26a 100644
--- a/arch/x86/include/uapi/asm/sigcontext.h
+++ b/arch/x86/include/uapi/asm/sigcontext.h
@@ -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;
-- 
2.4.3

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

[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 <luto@kernel.org>
---
 arch/x86/include/asm/desc_defs.h | 23 ++++++++++++++++++
 arch/x86/kernel/signal.c         | 51 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/desc_defs.h b/arch/x86/include/asm/desc_defs.h
index 278441f39856..00971705a16d 100644
--- a/arch/x86/include/asm/desc_defs.h
+++ b/arch/x86/include/asm/desc_defs.h
@@ -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 */
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index d87ce92d3404..ca8975096728 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -61,6 +61,35 @@
 	regs->seg = GET_SEG(seg) | 3;			\
 } while (0)
 
+#ifdef CONFIG_X86_64
+/*
+ * If regs->ss will cause an IRET fault, change it.  Otherwise leave it
+ * alone.  Using this generally makes no sense unless
+ * user_64bit_mode(regs) would return true.
+ */
+static void force_valid_ss(struct pt_regs *regs)
+{
+	u32 ar;
+	asm volatile ("lar %[old_ss], %[ar]\n\t"
+		      "jz 1f\n\t"		/* If invalid: */
+		      "xorl %[ar], %[ar]\n\t"	/* set ar = 0 */
+		      "1:"
+		      : [ar] "=r" (ar)
+		      : [old_ss] "rm" ((u16)regs->ss));
+
+	/*
+	 * For a valid 64-bit user context, we need DPL 3, type
+	 * read-write data or read-write exp-down data, and S and P
+	 * set.  We can't use VERW because VERW doesn't check the
+	 * P bit.
+	 */
+	ar &= AR_DPL_MASK | AR_S | AR_P | AR_TYPE_MASK;
+	if (ar != (AR_DPL3 | AR_S | AR_P | AR_TYPE_RWDATA) &&
+	    ar != (AR_DPL3 | AR_S | AR_P | AR_TYPE_RWDATA_EXPDOWN))
+		regs->ss = __USER_DS;
+}
+#endif
+
 int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
 {
 	void __user *buf;
@@ -457,10 +486,28 @@ static int __setup_rt_frame(int sig, struct ksignal *ksig,
 
 	regs->sp = (unsigned long)frame;
 
-	/* Set up the CS register to run signal handlers in 64-bit mode,
-	   even if the handler happens to be interrupting 32-bit code. */
+	/*
+	 * Set up the CS and SS registers to run signal handlers in
+	 * 64-bit mode, even if the handler happens to be interrupting
+	 * 32-bit or 16-bit code.
+	 *
+	 * SS is subtle.  In 64-bit mode, we don't need any particular
+	 * SS descriptor, but we do need SS to be valid.  It's possible
+	 * that the old SS is entirely bogus -- this can happen if the
+	 * signal we're trying to deliver is #GP or #SS caused by a bad
+	 * SS value.  We also have a compatbility issue here: DOSEMU
+	 * relies on the contents of the SS register indicating the
+	 * SS value at the time of the signal, even though that code in
+	 * DOSEMU predates sigreturn's ability to restore SS.  (DOSEMU
+	 * avoids relying on sigreturn to restore SS; instead it uses
+	 * a trampoline.)  So we do our best: if the old SS was valid,
+	 * we keep it.  Otherwise we replace it.
+	 */
 	regs->cs = __USER_CS;
 
+	if (unlikely(regs->ss != __USER_DS))
+		force_valid_ss(regs);
+
 	return 0;
 }
 #endif /* CONFIG_X86_32 */
-- 
2.4.3

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

[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 <stsp@list.ru>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 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(-)

diff --git a/arch/x86/include/asm/sigcontext.h b/arch/x86/include/asm/sigcontext.h
index 9dfce4e0417d..9789402b99b6 100644
--- a/arch/x86/include/asm/sigcontext.h
+++ b/arch/x86/include/asm/sigcontext.h
@@ -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;
diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
index 89db46752a8f..452c88b8ad06 100644
--- a/arch/x86/include/asm/sighandling.h
+++ b/arch/x86/include/asm/sighandling.h
@@ -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);
 
diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h
index 827c6ed0e26a..ad02ae519179 100644
--- a/arch/x86/include/uapi/asm/sigcontext.h
+++ b/arch/x86/include/uapi/asm/sigcontext.h
@@ -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;
diff --git a/arch/x86/include/uapi/asm/ucontext.h b/arch/x86/include/uapi/asm/ucontext.h
index b7c29c8017f2..b75b29fcd6aa 100644
--- a/arch/x86/include/uapi/asm/ucontext.h
+++ b/arch/x86/include/uapi/asm/ucontext.h
@@ -1,11 +1,42 @@
 #ifndef _ASM_X86_UCONTEXT_H
 #define _ASM_X86_UCONTEXT_H
 
-#define UC_FP_XSTATE	0x1	/* indicates the presence of extended state
-				 * information in the memory layout pointed
-				 * by the fpstate pointer in the ucontext's
-				 * sigcontext struct (uc_mcontext).
-				 */
+/*
+ * indicates the presence of extended state
+ * information in the memory layout pointed
+ * by the fpstate pointer in the ucontext's
+ * sigcontext struct (uc_mcontext).
+ */
+#define UC_FP_XSTATE	0x1
+
+#ifdef __x86_64__
+/*
+ * 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).
+ *
+ * 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.
+
+ * 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.
+ */
+#define UC_SIGCONTEXT_SS	0x2
+#define UC_STRICT_RESTORE_SS	0x4
+#endif
 
 #include <asm-generic/ucontext.h>
 
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index ca8975096728..2fd1e901550d 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -90,7 +90,9 @@ static void force_valid_ss(struct pt_regs *regs)
 }
 #endif
 
-int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
+static int restore_sigcontext(struct pt_regs *regs,
+			      struct sigcontext __user *sc,
+			      unsigned long uc_flags)
 {
 	void __user *buf;
 	unsigned int tmpflags;
@@ -122,15 +124,18 @@ int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
 		COPY(r15);
 #endif /* CONFIG_X86_64 */
 
-#ifdef CONFIG_X86_32
 		COPY_SEG_CPL3(cs);
 		COPY_SEG_CPL3(ss);
-#else /* !CONFIG_X86_32 */
-		/* Kernel saves and restores only the CS segment register on signals,
-		 * which is the bare minimum needed to allow mixed 32/64-bit code.
-		 * App's signal handler can save/restore other segments if needed. */
-		COPY_SEG_CPL3(cs);
-#endif /* CONFIG_X86_32 */
+
+#ifdef CONFIG_X86_64
+		/*
+		 * Fix up SS if needed for the benefit of old DOSEMU and
+		 * CRIU.
+		 */
+		if (unlikely(!(uc_flags & UC_STRICT_RESTORE_SS) &&
+			     user_64bit_mode(regs)))
+			force_valid_ss(regs);
+#endif
 
 		get_user_ex(tmpflags, &sc->flags);
 		regs->flags = (regs->flags & ~FIX_EFLAGS) | (tmpflags & FIX_EFLAGS);
@@ -186,13 +191,13 @@ int setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate,
 		put_user_ex(regs->cs, (unsigned int __user *)&sc->cs);
 		put_user_ex(regs->flags, &sc->flags);
 		put_user_ex(regs->sp, &sc->sp_at_signal);
-		put_user_ex(regs->ss, (unsigned int __user *)&sc->ss);
 #else /* !CONFIG_X86_32 */
 		put_user_ex(regs->flags, &sc->flags);
 		put_user_ex(regs->cs, &sc->cs);
 		put_user_ex(0, &sc->gs);
 		put_user_ex(0, &sc->fs);
 #endif /* CONFIG_X86_32 */
+		put_user_ex(regs->ss, (unsigned int __user *)&sc->ss);
 
 		put_user_ex(fpstate, &sc->fpstate);
 
@@ -430,6 +435,21 @@ static int __setup_rt_frame(int sig, struct ksignal *ksig,
 	return 0;
 }
 #else /* !CONFIG_X86_32 */
+static unsigned long frame_uc_flags(struct pt_regs *regs)
+{
+	unsigned long flags;
+
+	if (cpu_has_xsave)
+		flags = UC_FP_XSTATE | UC_SIGCONTEXT_SS;
+	else
+		flags = UC_SIGCONTEXT_SS;
+
+	if (likely(user_64bit_mode(regs)))
+		flags |= UC_STRICT_RESTORE_SS;
+
+	return flags;
+}
+
 static int __setup_rt_frame(int sig, struct ksignal *ksig,
 			    sigset_t *set, struct pt_regs *regs)
 {
@@ -449,10 +469,7 @@ static int __setup_rt_frame(int sig, struct ksignal *ksig,
 
 	put_user_try {
 		/* Create the ucontext.  */
-		if (cpu_has_xsave)
-			put_user_ex(UC_FP_XSTATE, &frame->uc.uc_flags);
-		else
-			put_user_ex(0, &frame->uc.uc_flags);
+		put_user_ex(frame_uc_flags(regs), &frame->uc.uc_flags);
 		put_user_ex(0, &frame->uc.uc_link);
 		save_altstack_ex(&frame->uc.uc_stack, regs->sp);
 
@@ -534,10 +551,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
 
 	put_user_try {
 		/* Create the ucontext.  */
-		if (cpu_has_xsave)
-			put_user_ex(UC_FP_XSTATE, &frame->uc.uc_flags);
-		else
-			put_user_ex(0, &frame->uc.uc_flags);
+		put_user_ex(frame_uc_flags(regs), &frame->uc.uc_flags);
 		put_user_ex(0, &frame->uc.uc_link);
 		compat_save_altstack_ex(&frame->uc.uc_stack, regs->sp);
 		put_user_ex(0, &frame->uc.uc__pad0);
@@ -599,7 +613,11 @@ asmlinkage unsigned long sys_sigreturn(void)
 
 	set_current_blocked(&set);
 
-	if (restore_sigcontext(regs, &frame->sc))
+	/*
+	 * x86_32 has no uc_flags bits relevant to restore_sigcontext.
+	 * Save a few cycles by skipping the __get_user.
+	 */
+	if (restore_sigcontext(regs, &frame->sc, 0))
 		goto badframe;
 	return regs->ax;
 
@@ -615,16 +633,19 @@ asmlinkage long sys_rt_sigreturn(void)
 	struct pt_regs *regs = current_pt_regs();
 	struct rt_sigframe __user *frame;
 	sigset_t set;
+	unsigned long uc_flags;
 
 	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
 	if (!access_ok(VERIFY_READ, frame, sizeof(*frame)))
 		goto badframe;
 	if (__copy_from_user(&set, &frame->uc.uc_sigmask, sizeof(set)))
 		goto badframe;
+	if (__get_user(uc_flags, &frame->uc.uc_flags))
+		goto badframe;
 
 	set_current_blocked(&set);
 
-	if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
+	if (restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
 		goto badframe;
 
 	if (restore_altstack(&frame->uc.uc_stack))
@@ -805,6 +826,7 @@ asmlinkage long sys32_x32_rt_sigreturn(void)
 	struct pt_regs *regs = current_pt_regs();
 	struct rt_sigframe_x32 __user *frame;
 	sigset_t set;
+	unsigned long uc_flags;
 
 	frame = (struct rt_sigframe_x32 __user *)(regs->sp - 8);
 
@@ -812,10 +834,12 @@ asmlinkage long sys32_x32_rt_sigreturn(void)
 		goto badframe;
 	if (__copy_from_user(&set, &frame->uc.uc_sigmask, sizeof(set)))
 		goto badframe;
+	if (__get_user(uc_flags, &frame->uc.uc_flags))
+		goto badframe;
 
 	set_current_blocked(&set);
 
-	if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
+	if (restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
 		goto badframe;
 
 	if (compat_restore_altstack(&frame->uc.uc_stack))
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 389701f59940..c2221786d835 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -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)
-- 
2.4.3

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

[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 <luto@kernel.org>
---
 tools/testing/selftests/x86/sigreturn.c | 240 ++++++++++++++++++++++++++++----
 1 file changed, 212 insertions(+), 28 deletions(-)

diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c
index b5aa1bab7416..43e840470e32 100644
--- a/tools/testing/selftests/x86/sigreturn.c
+++ b/tools/testing/selftests/x86/sigreturn.c
@@ -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;
 }
-- 
2.4.3

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

[Stas Sergeev]

13.10.2015 04:04, Andy Lutomirski пишет:
> + * 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.

> + *
> + * 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.
> +
> + * 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?

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

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

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

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

[Ingo Molnar]

* Stas Sergeev <stsp@list.ru> wrote:

> 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

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

[Stas Sergeev]

14.10.2015 18:01, Ingo Molnar пишет:
> 
> * Stas Sergeev <stsp@list.ru> wrote:
> 
>> 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.
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.

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

[Cyrill Gorcunov]

On Mon, Oct 12, 2015 at 06:04:07PM -0700, Andy Lutomirski wrote:
...
> 
> 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 <stsp@list.ru>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>

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?

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

[Andy Lutomirski]

On Oct 13, 2015 10:10 PM, "Stas Sergeev" <stsp@list.ru> wrote:
>
> 13.10.2015 04:04, Andy Lutomirski пишет:
> > + * 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.

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

>
> > + *
> > + * 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.
> > +
> > + * 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.
>

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.

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

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

Old progs = just DOSEMU here, I think.

> 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?
>
> > 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.
>
> > 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.

> - 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.)

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

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

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

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

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

[Andy Lutomirski]

On Wed, Oct 14, 2015 at 9:40 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On Mon, Oct 12, 2015 at 06:04:07PM -0700, Andy Lutomirski wrote:
> ...
>>
>> 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 <stsp@list.ru>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
>> Cc: Pavel Emelyanov <xemul@parallels.com>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>
> 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

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

[Stas Sergeev]

14.10.2015 19:40, Cyrill Gorcunov пишет:
> On Mon, Oct 12, 2015 at 06:04:07PM -0700, Andy Lutomirski wrote:
> ...
>>
>> 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 <stsp@list.ru>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
>> Cc: Pavel Emelyanov <xemul@parallels.com>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> 
> 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.

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

[Cyrill Gorcunov]

On Wed, Oct 14, 2015 at 09:42:58AM -0700, Andy Lutomirski wrote:
> 
> 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!

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

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

[Cyrill Gorcunov]

On Wed, Oct 14, 2015 at 07:57:02PM +0300, Stas Sergeev wrote:
...
> > 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!

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

[Stas Sergeev]

14.10.2015 19:40, Andy Lutomirski пишет:
>>> + *
>>> + * 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).

>>> 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.
>>
>>> 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.
Good.

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

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

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

>> 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. :)

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

[Andy Lutomirski]

On Wed, Oct 14, 2015 at 10:40 AM, Stas Sergeev <stsp@list.ru> wrote:
> 14.10.2015 19:40, Andy Lutomirski пишет:
>>>> + *
>>>> + * 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?

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.

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

>
>>>> 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.
>>>
>>>> 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.
> Good.
>
>>> - 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.

>>> - 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 :(

> 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

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

[Stas Sergeev]

14.10.2015 21:06, Andy Lutomirski пишет:
>> 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.

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

>>>> - 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 :(
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.

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

[Andy Lutomirski]

On Wed, Oct 14, 2015 at 11:34 AM, Stas Sergeev <stsp@list.ru> wrote:
> 14.10.2015 21:06, Andy Lutomirski пишет:
>>> 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.

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

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

[Stas Sergeev]

14.10.2015 21:52, Andy Lutomirski пишет:
> On Wed, Oct 14, 2015 at 11:34 AM, Stas Sergeev <stsp@list.ru> wrote:
>> 14.10.2015 21:06, Andy Lutomirski пишет:
>>>> 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.
>
>>>   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?
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!

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

[Andy Lutomirski]

On Wed, Oct 14, 2015 at 2:37 PM, Stas Sergeev <stsp@list.ru> wrote:
> 14.10.2015 21:52, Andy Lutomirski пишет:
>>
>> On Wed, Oct 14, 2015 at 11:34 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>> 14.10.2015 21:06, Andy Lutomirski пишет:
>>>>>
>>>>> 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.
>>
>>>>   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?
>
> 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.

> 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

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

[Stas Sergeev]

15.10.2015 00:41, Andy Lutomirski пишет:
>> 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?

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

[Andy Lutomirski]

On Sun, Oct 18, 2015 at 6:36 AM, Stas Sergeev <stsp@list.ru> wrote:
> 15.10.2015 00:41, Andy Lutomirski пишет:
>>>
>>> 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?

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

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

[Stas Sergeev]

18.10.2015 19:12, Andy Lutomirski пишет:
> On Sun, Oct 18, 2015 at 6:36 AM, Stas Sergeev <stsp@list.ru> wrote:
>> 15.10.2015 00:41, Andy Lutomirski пишет:
>>>> 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?
> 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.

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

[Andy Lutomirski]

On Sun, Oct 18, 2015 at 9:29 AM, Stas Sergeev <stsp@list.ru> wrote:
> 18.10.2015 19:12, Andy Lutomirski пишет:
>>
>> On Sun, Oct 18, 2015 at 6:36 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>> 15.10.2015 00:41, Andy Lutomirski пишет:
>>>>>
>>>>> 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?
>>
>> 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.

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

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

[Stas Sergeev]

18.10.2015 19:36, Andy Lutomirski пишет:
> On Sun, Oct 18, 2015 at 9:29 AM, Stas Sergeev <stsp@list.ru> wrote:
>> 18.10.2015 19:12, Andy Lutomirski пишет:
>>> On Sun, Oct 18, 2015 at 6:36 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>> 15.10.2015 00:41, Andy Lutomirski пишет:
>>>>>> 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?
>>> 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.
> 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.

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

[Andy Lutomirski]

On Sun, Oct 18, 2015 at 9:43 AM, Stas Sergeev <stsp@list.ru> wrote:
> 18.10.2015 19:36, Andy Lutomirski пишет:
>
>> On Sun, Oct 18, 2015 at 9:29 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>> 18.10.2015 19:12, Andy Lutomirski пишет:
>>>>
>>>> On Sun, Oct 18, 2015 at 6:36 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>>>
>>>>> 15.10.2015 00:41, Andy Lutomirski пишет:
>>>>>>>
>>>>>>> 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?
>>>>
>>>> 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.
>>
>> 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.

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