Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1332338 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 3/4] x86/signal/64: Re-add support for SS in the  64-bit signal context Borislav Petkov <bp@alien8.de> - 2016-02-11 20:50 +0100
    Re: [PATCH v3 3/4] x86/signal/64: Re-add support for SS in the 64-bit  signal context Andy Lutomirski <luto@amacapital.net> - 2016-02-12 02:10 +0100
      Re: [PATCH v3 3/4] x86/signal/64: Re-add support for SS in the  64-bit signal context Borislav Petkov <bp@alien8.de> - 2016-02-12 15:00 +0100

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

On Mon, Jan 25, 2016 at 01:34:14PM -0800, Andy Lutomirski wrote:
> 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 set a non-flag SS:ESP.
> (DOSEMU has a monstrous hack to partially work around this
> limitation.)
> 
> 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 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.
> 
> This patch also re-enables the sigreturn_64 and ldt_gdt_64 selftests,
> as the kernel change allows both of them to pass.
> 
> 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>
> Tested-by: Stas Sergeev <stsp@list.ru>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/include/asm/sighandling.h     |  1 -
>  arch/x86/include/uapi/asm/sigcontext.h |  7 ++--
>  arch/x86/include/uapi/asm/ucontext.h   | 43 ++++++++++++++++++++---
>  arch/x86/kernel/signal.c               | 63 ++++++++++++++++++++++++----------
>  tools/testing/selftests/x86/Makefile   |  7 ++--
>  5 files changed, 91 insertions(+), 30 deletions(-)
> 
> 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 47dae8150520..bb0dde737b59 100644
> --- a/arch/x86/include/uapi/asm/sigcontext.h
> +++ b/arch/x86/include/uapi/asm/sigcontext.h
> @@ -256,7 +256,7 @@ struct sigcontext_64 {
>  	__u16				cs;
>  	__u16				gs;
>  	__u16				fs;
> -	__u16				__pad0;
> +	__u16				ss;
>  	__u64				err;
>  	__u64				trapno;
>  	__u64				oldmask;
> @@ -362,7 +362,10 @@ struct sigcontext {
>  	 */
>  	__u16				gs;
>  	__u16				fs;
> -	__u16				__pad0;
> +	union {
> +		__u16			ss;	/* If UC_SAVED_SS */
> +		__u16			__pad0;	/* If !UC_SAVED_SS */

						UC_SIGCONTEXT_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..a5c1718ce65b 100644
> --- a/arch/x86/include/uapi/asm/ucontext.h
> +++ b/arch/x86/include/uapi/asm/ucontext.h
> @@ -1,11 +1,44 @@
>  #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).
> + */

Please reflow that comment to match the rest of the comments in this file.

> +#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, or if
> + * the CS value in the signal context does not refer to a 64-bit
> + * code segment, 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, then SS it will be replaced

s/it //

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

I'm having hard time parsing that sentence and especially placing all
those "either", "or", "and" connectors at the proper levels. Would it be
more understandable as pseudocode?

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

So my brain is reliably in a knot after this text.

> + */
> +#define UC_SIGCONTEXT_SS	0x2
> +#define UC_STRICT_RESTORE_SS	0x4
> +#endif
>  
>  #include <asm-generic/ucontext.h>
>  
-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [next] | [standalone]

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

On Thu, Feb 11, 2016 at 11:49 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Jan 25, 2016 at 01:34:14PM -0800, Andy Lutomirski wrote:
>
>> + * with a flat 32-bit selector.

How about:

Sigreturn restores SS as follows:

if (saved SS is valid || UC_STRICT_RESTORE_SS is set || saved CS is not 64-bit)
 new SS = saved SS
else
  new SS = a flat 32-bit data segment

>> +
>> + * 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.
>
> I'm having hard time parsing that sentence and especially placing all
> those "either", "or", "and" connectors at the proper levels. Would it be
> more understandable as pseudocode?
>
>> 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.
>
> So my brain is reliably in a knot after this text.


How about:

--- cut here ---

This behavior serves three purposes:

 - Legacy programs that construct a 64-bit sigcontext from scratch
with zero or garbage in the SS slot (e.g. old CRIU) and call sigreturn
will still work.

 - Old DOSEMU versions sometimes catch a signal from a segmented
context, delete the old SS segment (with modify_ldt), and change the
saved CS to a 64-bit segment.  These DOSEMU versions expect sigreturn
to send them back to 64-bit mode without killing them, despite the
fact that the SS selector when the signal was raised is no longer
valid.  With UC_STRICT_RESTORE_SS clear, the kernel will fix up SS for
these DOSEMU versions.

 - Old and new programs that catch a signal and return without
modifying the saved context will end up in exactly the state they
started in.  Old kernels would lose track of the previous SS value.

--- cut here ---

FWIW, I have a DOSEMU patch that makes it use UC_STRICT_RESTORE_SS to
get the behavior it actually wants on new kernels.  It should make it
faster and more reliable than was possible before.

--Andy

[toc] | [prev] | [next] | [standalone]

#1332749

On Thu, Feb 11, 2016 at 05:01:39PM -0800, Andy Lutomirski wrote:
> On Thu, Feb 11, 2016 at 11:49 AM, Borislav Petkov <bp@alien8.de> wrote:
> > On Mon, Jan 25, 2016 at 01:34:14PM -0800, Andy Lutomirski wrote:
> >
> >> + * with a flat 32-bit selector.
> 
> How about:
> 
> Sigreturn restores SS as follows:
> 
> if (saved SS is valid || UC_STRICT_RESTORE_SS is set || saved CS is not 64-bit)
>  new SS = saved SS
> else
>   new SS = a flat 32-bit data segment

Much better!

> How about:
> 
> --- cut here ---
> 
> This behavior serves three purposes:
> 
>  - Legacy programs that construct a 64-bit sigcontext from scratch
> with zero or garbage in the SS slot (e.g. old CRIU) and call sigreturn
> will still work.
> 
>  - Old DOSEMU versions sometimes catch a signal from a segmented
> context, delete the old SS segment (with modify_ldt), and change the
> saved CS to a 64-bit segment.  These DOSEMU versions expect sigreturn
> to send them back to 64-bit mode without killing them, despite the
> fact that the SS selector when the signal was raised is no longer
> valid.  With UC_STRICT_RESTORE_SS clear, the kernel will fix up SS for
> these DOSEMU versions.

... and with UC_STRICT_RESTORE_SS set, they'll get __USER_DS.

>  - Old and new programs that catch a signal and return without
> modifying the saved context will end up in exactly the state they
> started in.  Old kernels would lose track of the previous SS value.
> 
> --- cut here ---

Yap, definitely better.

> FWIW, I have a DOSEMU patch that makes it use UC_STRICT_RESTORE_SS to
> get the behavior it actually wants on new kernels.  It should make it
> faster and more reliable than was possible before.

Cool.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web