* [PATCH 0/2] x86_64,signal: Fix the set of saved segment registers @ 2014-07-11 16:29 Andy Lutomirski 2014-07-11 16:29 ` [PATCH 1/2] x86_64,signal: Save and restore SS in signal frames Andy Lutomirski 2014-07-11 16:29 ` [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext Andy Lutomirski 0 siblings, 2 replies; 16+ messages in thread From: Andy Lutomirski @ 2014-07-11 16:29 UTC (permalink / raw) To: H. Peter Anvin, Andi Kleen; +Cc: Al Viro, x86, linux-kernel, Andy Lutomirski The x86_64 signal code claims to save and restore CS, FS, and GS, and it further claims that this is the minimal set that's needed. Neither of these claims is true. The code does not, and AFAICT never has, saved or restored FS and GS, nor does it need to. On the other hand, all 64-bit syscalls (and rt_sigreturn in particular) clobber SS, making it impossible for signal handlers to correctly restore SS without using a trampoline or ptracing themselves. This patchset saves and restores SS in the __pad0 slot and renames the FS and GS slots __pad1 and __pad2 to more accurately document their purpose. I discovered this while writing a test for espfix64. Andy Lutomirski (2): x86_64,signal: Save and restore SS in signal frames x86_64,signal: Remove 'fs' and 'gs' from sigcontext arch/x86/include/asm/sigcontext.h | 6 +++--- arch/x86/include/uapi/asm/sigcontext.h | 6 +++--- arch/x86/kernel/signal.c | 12 +++--------- 3 files changed, 9 insertions(+), 15 deletions(-) -- 1.9.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] x86_64,signal: Save and restore SS in signal frames 2014-07-11 16:29 [PATCH 0/2] x86_64,signal: Fix the set of saved segment registers Andy Lutomirski @ 2014-07-11 16:29 ` Andy Lutomirski 2015-03-09 20:15 ` Andy Lutomirski 2014-07-11 16:29 ` [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext Andy Lutomirski 1 sibling, 1 reply; 16+ messages in thread From: Andy Lutomirski @ 2014-07-11 16:29 UTC (permalink / raw) To: H. Peter Anvin, Andi Kleen; +Cc: Al Viro, x86, linux-kernel, Andy Lutomirski The comment in the signal code says that apps can save/restore other segments on their own. It's true that apps can *save* SS on their own, but there's no way for apps to restore it: SYSCALL effectively resets SS to __USER_DS, so any value that user code tries to load into SS gets lost on entry to sigreturn. This recycles two padding bytes in the segment selector area for SS. I suspect that 64-bit programs that try to run 16-bit code and uses signals will have a lot of trouble without this. Signed-off-by: Andy Lutomirski <luto@amacapital.net> --- arch/x86/include/asm/sigcontext.h | 2 +- arch/x86/include/uapi/asm/sigcontext.h | 2 +- arch/x86/kernel/signal.c | 8 +------- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/arch/x86/include/asm/sigcontext.h b/arch/x86/include/asm/sigcontext.h index 9dfce4e..f910cdc 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; unsigned long err; unsigned long trapno; unsigned long oldmask; diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h index d8b9f90..076b11f 100644 --- a/arch/x86/include/uapi/asm/sigcontext.h +++ b/arch/x86/include/uapi/asm/sigcontext.h @@ -179,7 +179,7 @@ struct sigcontext { __u16 cs; __u16 gs; __u16 fs; - __u16 __pad0; + __u16 ss; __u64 err; __u64 trapno; __u64 oldmask; diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c index 2851d63..08d7a9e 100644 --- a/arch/x86/kernel/signal.c +++ b/arch/x86/kernel/signal.c @@ -94,15 +94,8 @@ int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc, COPY(r15); #endif /* CONFIG_X86_64 */ -#ifdef CONFIG_X86_32 COPY_SEG_CPL3(cs); COPY_SEG_CPL3(ss); -#else /* !CONFIG_X86_32 */ - /* Kernel saves and restores only the CS segment register on signals, - * which is the bare minimum needed to allow mixed 32/64-bit code. - * App's signal handler can save/restore other segments if needed. */ - COPY_SEG_CPL3(cs); -#endif /* CONFIG_X86_32 */ get_user_ex(tmpflags, &sc->flags); regs->flags = (regs->flags & ~FIX_EFLAGS) | (tmpflags & FIX_EFLAGS); @@ -164,6 +157,7 @@ int setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate, put_user_ex(regs->cs, &sc->cs); put_user_ex(0, &sc->gs); put_user_ex(0, &sc->fs); + put_user_ex(regs->ss, &sc->ss); #endif /* CONFIG_X86_32 */ put_user_ex(fpstate, &sc->fpstate); -- 1.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] x86_64,signal: Save and restore SS in signal frames 2014-07-11 16:29 ` [PATCH 1/2] x86_64,signal: Save and restore SS in signal frames Andy Lutomirski @ 2015-03-09 20:15 ` Andy Lutomirski 0 siblings, 0 replies; 16+ messages in thread From: Andy Lutomirski @ 2015-03-09 20:15 UTC (permalink / raw) To: H. Peter Anvin, Andi Kleen, Ingo Molnar Cc: Al Viro, X86 ML, linux-kernel@vger.kernel.org, Andy Lutomirski Hi Ingo, Here's a blast from the past. I asked Andi today, and he seems conceptually okay with this. Could you consider applying it? Aside from being a bug fix IMO, it will allow my sigreturn test to work reasonably well as a native 64-bit program. I checked: it still applies cleanly. --Andy On Fri, Jul 11, 2014 at 9:29 AM, Andy Lutomirski <luto@amacapital.net> wrote: > The comment in the signal code says that apps can save/restore other > segments on their own. It's true that apps can *save* SS on their > own, but there's no way for apps to restore it: SYSCALL effectively > resets SS to __USER_DS, so any value that user code tries to load > into SS gets lost on entry to sigreturn. > > This recycles two padding bytes in the segment selector area for SS. > > I suspect that 64-bit programs that try to run 16-bit code and uses > signals will have a lot of trouble without this. > > Signed-off-by: Andy Lutomirski <luto@amacapital.net> > --- > arch/x86/include/asm/sigcontext.h | 2 +- > arch/x86/include/uapi/asm/sigcontext.h | 2 +- > arch/x86/kernel/signal.c | 8 +------- > 3 files changed, 3 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/include/asm/sigcontext.h b/arch/x86/include/asm/sigcontext.h > index 9dfce4e..f910cdc 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; > unsigned long err; > unsigned long trapno; > unsigned long oldmask; > diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h > index d8b9f90..076b11f 100644 > --- a/arch/x86/include/uapi/asm/sigcontext.h > +++ b/arch/x86/include/uapi/asm/sigcontext.h > @@ -179,7 +179,7 @@ struct sigcontext { > __u16 cs; > __u16 gs; > __u16 fs; > - __u16 __pad0; > + __u16 ss; > __u64 err; > __u64 trapno; > __u64 oldmask; > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c > index 2851d63..08d7a9e 100644 > --- a/arch/x86/kernel/signal.c > +++ b/arch/x86/kernel/signal.c > @@ -94,15 +94,8 @@ int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc, > COPY(r15); > #endif /* CONFIG_X86_64 */ > > -#ifdef CONFIG_X86_32 > COPY_SEG_CPL3(cs); > COPY_SEG_CPL3(ss); > -#else /* !CONFIG_X86_32 */ > - /* Kernel saves and restores only the CS segment register on signals, > - * which is the bare minimum needed to allow mixed 32/64-bit code. > - * App's signal handler can save/restore other segments if needed. */ > - COPY_SEG_CPL3(cs); > -#endif /* CONFIG_X86_32 */ > > get_user_ex(tmpflags, &sc->flags); > regs->flags = (regs->flags & ~FIX_EFLAGS) | (tmpflags & FIX_EFLAGS); > @@ -164,6 +157,7 @@ int setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate, > put_user_ex(regs->cs, &sc->cs); > put_user_ex(0, &sc->gs); > put_user_ex(0, &sc->fs); > + put_user_ex(regs->ss, &sc->ss); > #endif /* CONFIG_X86_32 */ > > put_user_ex(fpstate, &sc->fpstate); > -- > 1.9.3 > -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-11 16:29 [PATCH 0/2] x86_64,signal: Fix the set of saved segment registers Andy Lutomirski 2014-07-11 16:29 ` [PATCH 1/2] x86_64,signal: Save and restore SS in signal frames Andy Lutomirski @ 2014-07-11 16:29 ` Andy Lutomirski 2014-07-11 18:12 ` Andi Kleen ` (2 more replies) 1 sibling, 3 replies; 16+ messages in thread From: Andy Lutomirski @ 2014-07-11 16:29 UTC (permalink / raw) To: H. Peter Anvin, Andi Kleen; +Cc: Al Viro, x86, linux-kernel, Andy Lutomirski As far as I can tell, these fields have been set to zero on save and ignored on restore since Linux was imported into git. Rename them '__pad1' and '__pad2' to avoid confusion and to allow them to be recycled some day. I'm intentionally avoiding calling either of them __pad0: the field formerly known as __pad0 is now ss. Signed-off-by: Andy Lutomirski <luto@amacapital.net> --- arch/x86/include/asm/sigcontext.h | 4 ++-- arch/x86/include/uapi/asm/sigcontext.h | 4 ++-- arch/x86/kernel/signal.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/sigcontext.h b/arch/x86/include/asm/sigcontext.h index f910cdc..5f0ef11 100644 --- a/arch/x86/include/asm/sigcontext.h +++ b/arch/x86/include/asm/sigcontext.h @@ -57,8 +57,8 @@ struct sigcontext { unsigned long ip; unsigned long flags; unsigned short cs; - unsigned short gs; - unsigned short fs; + unsigned short __pad2; /* Was called gs, but was always zero. */ + unsigned short __pad1; /* Was called gs, but was always zero. */ unsigned short ss; unsigned long err; unsigned long trapno; diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h index 076b11f..df9908b 100644 --- a/arch/x86/include/uapi/asm/sigcontext.h +++ b/arch/x86/include/uapi/asm/sigcontext.h @@ -177,8 +177,8 @@ struct sigcontext { __u64 rip; __u64 eflags; /* RFLAGS */ __u16 cs; - __u16 gs; - __u16 fs; + __u16 __pad2; /* Was called gs, but was always zero. */ + __u16 __pad1; /* Was called fs, but was always zero. */ __u16 ss; __u64 err; __u64 trapno; diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c index 08d7a9e..a1867ce 100644 --- a/arch/x86/kernel/signal.c +++ b/arch/x86/kernel/signal.c @@ -155,8 +155,8 @@ int setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate, #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); + put_user_ex(0, &sc->__pad2); + put_user_ex(0, &sc->__pad1); put_user_ex(regs->ss, &sc->ss); #endif /* CONFIG_X86_32 */ -- 1.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-11 16:29 ` [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext Andy Lutomirski @ 2014-07-11 18:12 ` Andi Kleen 2014-07-11 18:39 ` Andy Lutomirski 2014-07-12 2:04 ` H. Peter Anvin 2014-07-12 2:21 ` Linus Torvalds 2 siblings, 1 reply; 16+ messages in thread From: Andi Kleen @ 2014-07-11 18:12 UTC (permalink / raw) To: Andy Lutomirski; +Cc: H. Peter Anvin, Andi Kleen, Al Viro, x86, linux-kernel > diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h > index 076b11f..df9908b 100644 > --- a/arch/x86/include/uapi/asm/sigcontext.h > +++ b/arch/x86/include/uapi/asm/sigcontext.h I don't think renaming fields in uapi/asm is acceptable. These are likely used by user programs and you'll break compiles. -Andi > @@ -177,8 +177,8 @@ struct sigcontext { > __u64 rip; > __u64 eflags; /* RFLAGS */ > __u16 cs; > - __u16 gs; > - __u16 fs; > + __u16 __pad2; /* Was called gs, but was always zero. */ > + __u16 __pad1; /* Was called fs, but was always zero. */ > __u16 ss; > __u64 err; > __u64 trapno; -- ak@linux.intel.com -- Speaking for myself only. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-11 18:12 ` Andi Kleen @ 2014-07-11 18:39 ` Andy Lutomirski 2014-07-12 2:09 ` H. Peter Anvin 0 siblings, 1 reply; 16+ messages in thread From: Andy Lutomirski @ 2014-07-11 18:39 UTC (permalink / raw) To: Andi Kleen; +Cc: H. Peter Anvin, Al Viro, X86 ML, linux-kernel@vger.kernel.org On Fri, Jul 11, 2014 at 11:12 AM, Andi Kleen <andi@firstfloor.org> wrote: >> diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h >> index 076b11f..df9908b 100644 >> --- a/arch/x86/include/uapi/asm/sigcontext.h >> +++ b/arch/x86/include/uapi/asm/sigcontext.h > > I don't think renaming fields in uapi/asm is acceptable. These > are likely used by user programs and you'll break compiles. Hmm. That's a fair point. On the other hand, any user code that uses these fields explicitly may already be broken, since the current names of these fields rather strongly imply that they do something. Is there any clear policy on minor API breaks in the UAPI headers that don't affect ABI? --Andy ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-11 18:39 ` Andy Lutomirski @ 2014-07-12 2:09 ` H. Peter Anvin 0 siblings, 0 replies; 16+ messages in thread From: H. Peter Anvin @ 2014-07-12 2:09 UTC (permalink / raw) To: Andy Lutomirski, Andi Kleen Cc: Al Viro, X86 ML, linux-kernel@vger.kernel.org, Linus Torvalds On 07/11/2014 11:39 AM, Andy Lutomirski wrote: > On Fri, Jul 11, 2014 at 11:12 AM, Andi Kleen <andi@firstfloor.org> wrote: >>> diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h >>> index 076b11f..df9908b 100644 >>> --- a/arch/x86/include/uapi/asm/sigcontext.h >>> +++ b/arch/x86/include/uapi/asm/sigcontext.h >> >> I don't think renaming fields in uapi/asm is acceptable. These >> are likely used by user programs and you'll break compiles. > > Hmm. That's a fair point. On the other hand, any user code that uses > these fields explicitly may already be broken, since the current names > of these fields rather strongly imply that they do something. > > Is there any clear policy on minor API breaks in the UAPI headers that > don't affect ABI? > There really isn't, and this *definitely* a boundary case: as you state, it is very likely that anyone currently using them are doing so incorrectly, but it does induce potential source-level breakage. Linus, do you have any guidance here? -hpa ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-11 16:29 ` [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext Andy Lutomirski 2014-07-11 18:12 ` Andi Kleen @ 2014-07-12 2:04 ` H. Peter Anvin 2014-07-12 2:21 ` Linus Torvalds 2 siblings, 0 replies; 16+ messages in thread From: H. Peter Anvin @ 2014-07-12 2:04 UTC (permalink / raw) To: Andy Lutomirski, Andi Kleen; +Cc: Al Viro, x86, linux-kernel On 07/11/2014 09:29 AM, Andy Lutomirski wrote: > diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h > index 076b11f..df9908b 100644 > --- a/arch/x86/include/uapi/asm/sigcontext.h > +++ b/arch/x86/include/uapi/asm/sigcontext.h > @@ -177,8 +177,8 @@ struct sigcontext { > __u64 rip; > __u64 eflags; /* RFLAGS */ > __u16 cs; > - __u16 gs; > - __u16 fs; > + __u16 __pad2; /* Was called gs, but was always zero. */ > + __u16 __pad1; /* Was called fs, but was always zero. */ > __u16 ss; > __u64 err; > __u64 trapno; I'm just wondering if this is likely to cause compile error in existing code. I guess worst case we can just revert this patch... -hpa ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-11 16:29 ` [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext Andy Lutomirski 2014-07-11 18:12 ` Andi Kleen 2014-07-12 2:04 ` H. Peter Anvin @ 2014-07-12 2:21 ` Linus Torvalds 2014-07-12 2:26 ` H. Peter Anvin 2014-07-12 8:39 ` Andy Lutomirski 2 siblings, 2 replies; 16+ messages in thread From: Linus Torvalds @ 2014-07-12 2:21 UTC (permalink / raw) To: Andy Lutomirski Cc: H. Peter Anvin, Andi Kleen, Al Viro, the arch/x86 maintainers, Linux Kernel Mailing List On Fri, Jul 11, 2014 at 9:29 AM, Andy Lutomirski <luto@amacapital.net> wrote: > As far as I can tell, these fields have been set to zero on save and > ignored on restore since Linux was imported into git. Rename them > '__pad1' and '__pad2' to avoid confusion and to allow them to be > recycled some day. Shouldn't we actually try to save/restore gs/fs properly? Admittedly, changing them in the signal handler would be insane, but still.. See our context switching code with the whole segment selector vs base save/restore code. Hmm? Linus ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-12 2:21 ` Linus Torvalds @ 2014-07-12 2:26 ` H. Peter Anvin 2014-07-12 8:39 ` Andy Lutomirski 1 sibling, 0 replies; 16+ messages in thread From: H. Peter Anvin @ 2014-07-12 2:26 UTC (permalink / raw) To: Linus Torvalds, Andy Lutomirski Cc: Andi Kleen, Al Viro, the arch/x86 maintainers, Linux Kernel Mailing List Our current handling of fs/vs is really weird. The best may very well be to explicitly save and restore both fs and he as part of Andi's patchset to support wrxsbase. On July 11, 2014 7:21:54 PM PDT, Linus Torvalds <torvalds@linux-foundation.org> wrote: >On Fri, Jul 11, 2014 at 9:29 AM, Andy Lutomirski <luto@amacapital.net> >wrote: >> As far as I can tell, these fields have been set to zero on save and >> ignored on restore since Linux was imported into git. Rename them >> '__pad1' and '__pad2' to avoid confusion and to allow them to be >> recycled some day. > >Shouldn't we actually try to save/restore gs/fs properly? Admittedly, >changing them in the signal handler would be insane, but still.. See >our context switching code with the whole segment selector vs base >save/restore code. Hmm? > > Linus -- Sent from my mobile phone. Please pardon brevity and lack of formatting. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-12 2:21 ` Linus Torvalds 2014-07-12 2:26 ` H. Peter Anvin @ 2014-07-12 8:39 ` Andy Lutomirski 2014-07-12 18:37 ` Andi Kleen 1 sibling, 1 reply; 16+ messages in thread From: Andy Lutomirski @ 2014-07-12 8:39 UTC (permalink / raw) To: Linus Torvalds Cc: linux-kernel@vger.kernel.org, Andi Kleen, H. Peter Anvin, Al Viro, the arch/x86 maintainers On Jul 11, 2014 7:21 PM, "Linus Torvalds" <torvalds@linux-foundation.org> wrote: > > On Fri, Jul 11, 2014 at 9:29 AM, Andy Lutomirski <luto@amacapital.net> wrote: > > As far as I can tell, these fields have been set to zero on save and > > ignored on restore since Linux was imported into git. Rename them > > '__pad1' and '__pad2' to avoid confusion and to allow them to be > > recycled some day. > > Shouldn't we actually try to save/restore gs/fs properly? Admittedly, > changing them in the signal handler would be insane, but still.. See > our context switching code with the whole segment selector vs base > save/restore code. Hmm? This seems like it's asking for trouble. I think wxe'd have to separately save the selectors and the base registers to avoid breaking something, especially once wrgsbase, etc are enabled. Why would this be needed anyway? Does anyone implement makecontext, etc using raise/sigreturn? If so, they might be in for a surprise when their gs starts getting saved, too. Linus, for context, the other patch in this series saves and restores SS. Without that, 64-bit sigreturn to a nondefault stack segment is basically impossible. But I don't see why any other segments (besides CS) are needed for 64-bit sigreturn. --Andy > > Linus ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-12 8:39 ` Andy Lutomirski @ 2014-07-12 18:37 ` Andi Kleen 2014-07-12 18:40 ` H. Peter Anvin 0 siblings, 1 reply; 16+ messages in thread From: Andi Kleen @ 2014-07-12 18:37 UTC (permalink / raw) To: Andy Lutomirski Cc: Linus Torvalds, linux-kernel@vger.kernel.org, Andi Kleen, H. Peter Anvin, Al Viro, the arch/x86 maintainers > This seems like it's asking for trouble. I think wxe'd have to > separately save the selectors and the base registers to avoid breaking > something, especially once wrgsbase, etc are enabled. I agree, it would likely break existing code. > Linus, for context, the other patch in this series saves and restores > SS. > Without that, 64-bit sigreturn to a nondefault stack segment is > basically impossible. Why would you ever want to do that? -Andi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-12 18:37 ` Andi Kleen @ 2014-07-12 18:40 ` H. Peter Anvin 2014-07-12 18:52 ` Andi Kleen 0 siblings, 1 reply; 16+ messages in thread From: H. Peter Anvin @ 2014-07-12 18:40 UTC (permalink / raw) To: Andi Kleen, Andy Lutomirski Cc: Linus Torvalds, linux-kernel@vger.kernel.org, Al Viro, the arch/x86 maintainers Because you are doing something weird (like Pin, for example) and take an asynchronous fault? On July 12, 2014 11:37:48 AM PDT, Andi Kleen <andi@firstfloor.org> wrote: >> This seems like it's asking for trouble. I think wxe'd have to >> separately save the selectors and the base registers to avoid >breaking >> something, especially once wrgsbase, etc are enabled. > >I agree, it would likely break existing code. > >> Linus, for context, the other patch in this series saves and restores >> SS. > >> Without that, 64-bit sigreturn to a nondefault stack segment is >> basically impossible. > >Why would you ever want to do that? > >-Andi -- Sent from my mobile phone. Please pardon brevity and lack of formatting. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-12 18:40 ` H. Peter Anvin @ 2014-07-12 18:52 ` Andi Kleen 2014-07-12 21:17 ` Andy Lutomirski 0 siblings, 1 reply; 16+ messages in thread From: Andi Kleen @ 2014-07-12 18:52 UTC (permalink / raw) To: H. Peter Anvin Cc: Andi Kleen, Andy Lutomirski, Linus Torvalds, linux-kernel@vger.kernel.org, Al Viro, the arch/x86 maintainers On Sat, Jul 12, 2014 at 11:40:03AM -0700, H. Peter Anvin wrote: > Because you are doing something weird (like Pin, for example) and take an asynchronous fault? But even for pin that would need executing 16 bit code, or really weird 32bit code. AFAIK for 32bit the only good use case was NX emulation (and old virtualization) which are both completely obsolete. I don't think it's worth messing with the signal handlers for 16bit code. If there's any problem with saving/restoring state that emulator can always handle it by itself. -Andi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-12 18:52 ` Andi Kleen @ 2014-07-12 21:17 ` Andy Lutomirski 2014-07-18 1:13 ` Andy Lutomirski 0 siblings, 1 reply; 16+ messages in thread From: Andy Lutomirski @ 2014-07-12 21:17 UTC (permalink / raw) To: Andi Kleen Cc: H. Peter Anvin, Linus Torvalds, linux-kernel@vger.kernel.org, Al Viro, the arch/x86 maintainers On Sat, Jul 12, 2014 at 11:52 AM, Andi Kleen <andi@firstfloor.org> wrote: > On Sat, Jul 12, 2014 at 11:40:03AM -0700, H. Peter Anvin wrote: >> Because you are doing something weird (like Pin, for example) and take an asynchronous fault? > > But even for pin that would need executing 16 bit code, or really weird > 32bit code. AFAIK for 32bit the only good use case was NX emulation > (and old virtualization) which are both completely obsolete. Nothing particularly weird is needed. Set a non-default stack segment (e.g. any 16-bit ss) and take *any* fault. This could be something asynchronous or even a normal synchronous fault. Return from the signal handler: boom. We know that people use 16-bit stack segments: it's what prompted the whole espfix64 thing. > > I don't think it's worth messing with the signal handlers for 16bit > code. If there's any problem with saving/restoring state that emulator > can always handle it by itself. > How? I can think of at least two vaguely feasible ways. The program could ptrace itself, trap sigreturn, and fix ss. Or it could restore registers itself and return using far ret or iret. Both suck. --Andy ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext 2014-07-12 21:17 ` Andy Lutomirski @ 2014-07-18 1:13 ` Andy Lutomirski 0 siblings, 0 replies; 16+ messages in thread From: Andy Lutomirski @ 2014-07-18 1:13 UTC (permalink / raw) To: Andi Kleen Cc: H. Peter Anvin, Linus Torvalds, linux-kernel@vger.kernel.org, Al Viro, the arch/x86 maintainers On Sat, Jul 12, 2014 at 2:17 PM, Andy Lutomirski <luto@amacapital.net> wrote: > On Sat, Jul 12, 2014 at 11:52 AM, Andi Kleen <andi@firstfloor.org> wrote: >> On Sat, Jul 12, 2014 at 11:40:03AM -0700, H. Peter Anvin wrote: >>> Because you are doing something weird (like Pin, for example) and take an asynchronous fault? >> >> But even for pin that would need executing 16 bit code, or really weird >> 32bit code. AFAIK for 32bit the only good use case was NX emulation >> (and old virtualization) which are both completely obsolete. > > Nothing particularly weird is needed. Set a non-default stack segment > (e.g. any 16-bit ss) and take *any* fault. This could be something > asynchronous or even a normal synchronous fault. Return from the > signal handler: boom. > > We know that people use 16-bit stack segments: it's what prompted the > whole espfix64 thing. > >> >> I don't think it's worth messing with the signal handlers for 16bit >> code. If there's any problem with saving/restoring state that emulator >> can always handle it by itself. >> > > How? > > I can think of at least two vaguely feasible ways. The program could > ptrace itself, trap sigreturn, and fix ss. Or it could restore > registers itself and return using far ret or iret. Both suck. You can't even hack around this with ptrace -- ptrace silently fails to write ss for non-TIF_IA32 tasks. --Andy ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-03-09 20:15 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-11 16:29 [PATCH 0/2] x86_64,signal: Fix the set of saved segment registers Andy Lutomirski 2014-07-11 16:29 ` [PATCH 1/2] x86_64,signal: Save and restore SS in signal frames Andy Lutomirski 2015-03-09 20:15 ` Andy Lutomirski 2014-07-11 16:29 ` [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext Andy Lutomirski 2014-07-11 18:12 ` Andi Kleen 2014-07-11 18:39 ` Andy Lutomirski 2014-07-12 2:09 ` H. Peter Anvin 2014-07-12 2:04 ` H. Peter Anvin 2014-07-12 2:21 ` Linus Torvalds 2014-07-12 2:26 ` H. Peter Anvin 2014-07-12 8:39 ` Andy Lutomirski 2014-07-12 18:37 ` Andi Kleen 2014-07-12 18:40 ` H. Peter Anvin 2014-07-12 18:52 ` Andi Kleen 2014-07-12 21:17 ` Andy Lutomirski 2014-07-18 1:13 ` Andy Lutomirski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.