Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-17 Thread Andy Lutomirski
On Sat, Jul 12, 2014 at 2:17 PM, Andy Lutomirski  wrote:
> On Sat, Jul 12, 2014 at 11:52 AM, Andi Kleen  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-12 Thread Andy Lutomirski
On Sat, Jul 12, 2014 at 11:52 AM, Andi Kleen  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-12 Thread Andi Kleen
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-12 Thread H. Peter Anvin
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  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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-12 Thread Andi Kleen
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-12 Thread Andy Lutomirski
On Jul 11, 2014 7:21 PM, "Linus Torvalds"  wrote:
>
> On Fri, Jul 11, 2014 at 9:29 AM, Andy Lutomirski  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-11 Thread H. Peter Anvin
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  
wrote:
>On Fri, Jul 11, 2014 at 9:29 AM, Andy Lutomirski 
>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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-11 Thread Linus Torvalds
On Fri, Jul 11, 2014 at 9:29 AM, Andy Lutomirski  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-11 Thread H. Peter Anvin
On 07/11/2014 11:39 AM, Andy Lutomirski wrote:
> On Fri, Jul 11, 2014 at 11:12 AM, Andi Kleen  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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-11 Thread H. Peter Anvin
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-11 Thread Andy Lutomirski
On Fri, Jul 11, 2014 at 11:12 AM, Andi Kleen  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] x86_64,signal: Remove 'fs' and 'gs' from sigcontext

2014-07-11 Thread Andi Kleen
> 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;

-- 
a...@linux.intel.com -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/