2016-03-07 20:02:40

by Stas Sergeev

[permalink] [raw]
Subject: Re: sigaltstack breaks swapcontext()

09.01.2016 04:48, Andy Lutomirski пишет:
> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <[email protected]> wrote:
>> 09.01.2016 02:24, Andy Lutomirski пишет:
>>> It's not sigaltstack that I'm thinking about. It's signal delivery.
>>> If you end up in DOS mode with SP coincidentally pointing to the
>>> sigaltstack (but with different SS so it's not really the
>>> sigaltstack), then the signal delivery will malfunction.
>> Will you take care of this one?
>> Looks quite dangerous for dosemu! And absolutely
>> undebuggable: you never know when you hit it.
> I'll try to remember to tack it on to the sigcontext series.
How is this one going?
There seem to be one more bug in sigcontext handling.
dosemu have this code:
---
/*
* FIRST thing to do in signal handlers - to avoid being trapped into
int0x11
* forever, we must restore the eflags.
*/
loadflags(eflags_fs_gs.eflags);
---

I quickly checked the kernel code, and it seems the
flags are indeed forgotten, even on ia32! I think the
most dangerous flags are AC and NT. But most of
others are important too. IMHO the safe defaults
should be forced when entering the sighandler.
Would you mind taking a look at this problem too?


2016-03-07 21:10:32

by Andy Lutomirski

[permalink] [raw]
Subject: Re: sigaltstack breaks swapcontext()

On Mon, Mar 7, 2016 at 12:02 PM, Stas Sergeev <[email protected]> wrote:
> 09.01.2016 04:48, Andy Lutomirski пишет:
>>
>> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <[email protected]> wrote:
>>>
>>> 09.01.2016 02:24, Andy Lutomirski пишет:
>>>>
>>>> It's not sigaltstack that I'm thinking about. It's signal delivery.
>>>> If you end up in DOS mode with SP coincidentally pointing to the
>>>> sigaltstack (but with different SS so it's not really the
>>>> sigaltstack), then the signal delivery will malfunction.
>>>
>>> Will you take care of this one?
>>> Looks quite dangerous for dosemu! And absolutely
>>> undebuggable: you never know when you hit it.
>>
>> I'll try to remember to tack it on to the sigcontext series.
>
> How is this one going?
> There seem to be one more bug in sigcontext handling.
> dosemu have this code:
> ---
> /*
> * FIRST thing to do in signal handlers - to avoid being trapped into
> int0x11
> * forever, we must restore the eflags.
> */
> loadflags(eflags_fs_gs.eflags);
> ---
>
> I quickly checked the kernel code, and it seems the
> flags are indeed forgotten, even on ia32! I think the
> most dangerous flags are AC and NT. But most of
> others are important too. IMHO the safe defaults
> should be forced when entering the sighandler.
> Would you mind taking a look at this problem too?

Clearing NT seems sane.

Clearing AC seems like an ABI break, so I'd be a bit nervous about
clearing AC unconditionally. We could add yet another SS flag (sigh),
or we could make the change. As a more conservative option, we could
make it so that AC is cleared on entry to an alignment check signal.

--Andy

--
Andy Lutomirski
AMA Capital Management, LLC

2016-03-07 21:21:17

by Stas Sergeev

[permalink] [raw]
Subject: Re: sigaltstack breaks swapcontext()

08.03.2016 00:10, Andy Lutomirski пишет:
> On Mon, Mar 7, 2016 at 12:02 PM, Stas Sergeev <[email protected]> wrote:
>> 09.01.2016 04:48, Andy Lutomirski пишет:
>>> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <[email protected]> wrote:
>>>> 09.01.2016 02:24, Andy Lutomirski пишет:
>>>>> It's not sigaltstack that I'm thinking about. It's signal delivery.
>>>>> If you end up in DOS mode with SP coincidentally pointing to the
>>>>> sigaltstack (but with different SS so it's not really the
>>>>> sigaltstack), then the signal delivery will malfunction.
>>>> Will you take care of this one?
>>>> Looks quite dangerous for dosemu! And absolutely
>>>> undebuggable: you never know when you hit it.
>>> I'll try to remember to tack it on to the sigcontext series.
>> How is this one going?
So what do you think about checking SS when
evaluating the on_sig_stack condition? Will you
fix this, or should I try?

>> There seem to be one more bug in sigcontext handling.
>> dosemu have this code:
>> ---
>> /*
>> * FIRST thing to do in signal handlers - to avoid being trapped into
>> int0x11
>> * forever, we must restore the eflags.
>> */
>> loadflags(eflags_fs_gs.eflags);
>> ---
>>
>> I quickly checked the kernel code, and it seems the
>> flags are indeed forgotten, even on ia32! I think the
>> most dangerous flags are AC and NT. But most of
>> others are important too. IMHO the safe defaults
>> should be forced when entering the sighandler.
>> Would you mind taking a look at this problem too?
> Clearing NT seems sane.
>
> Clearing AC seems like an ABI break, so I'd be a bit nervous about
> clearing AC unconditionally.
What exactly do you mean? Is this a documented part of ABI?
Where can I find out how the flags are supposed to be set on
entering a sighandler, any docs on that?
I thought they should just be forced to some default value, the
same as the segregs are handled.

> We could add yet another SS flag (sigh),
But this is not a sigreturn() problem and not sigaltstack() problem,
so what exactly flag do you mean?

> or we could make the change. As a more conservative option, we could
> make it so that AC is cleared on entry to an alignment check signal.
Hmm. But if we deliver such signal, the userspace will still
crash, so what's the use?

2016-03-07 21:32:37

by Andy Lutomirski

[permalink] [raw]
Subject: Re: sigaltstack breaks swapcontext()

On Mon, Mar 7, 2016 at 1:20 PM, Stas Sergeev <[email protected]> wrote:
> 08.03.2016 00:10, Andy Lutomirski пишет:
>>
>> On Mon, Mar 7, 2016 at 12:02 PM, Stas Sergeev <[email protected]> wrote:
>>>
>>> 09.01.2016 04:48, Andy Lutomirski пишет:
>>>>
>>>> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <[email protected]> wrote:
>>>>>
>>>>> 09.01.2016 02:24, Andy Lutomirski пишет:
>>>>>>
>>>>>> It's not sigaltstack that I'm thinking about. It's signal delivery.
>>>>>> If you end up in DOS mode with SP coincidentally pointing to the
>>>>>> sigaltstack (but with different SS so it's not really the
>>>>>> sigaltstack), then the signal delivery will malfunction.
>>>>>
>>>>> Will you take care of this one?
>>>>> Looks quite dangerous for dosemu! And absolutely
>>>>> undebuggable: you never know when you hit it.
>>>>
>>>> I'll try to remember to tack it on to the sigcontext series.
>>>
>>> How is this one going?
>
> So what do you think about checking SS when
> evaluating the on_sig_stack condition? Will you
> fix this, or should I try?

It fits in with your series, and you're welcome to do it. You can
also poke me and get me to do it.

>
>>> There seem to be one more bug in sigcontext handling.
>>> dosemu have this code:
>>> ---
>>> /*
>>> * FIRST thing to do in signal handlers - to avoid being trapped into
>>> int0x11
>>> * forever, we must restore the eflags.
>>> */
>>> loadflags(eflags_fs_gs.eflags);
>>> ---
>>>
>>> I quickly checked the kernel code, and it seems the
>>> flags are indeed forgotten, even on ia32! I think the
>>> most dangerous flags are AC and NT. But most of
>>> others are important too. IMHO the safe defaults
>>> should be forced when entering the sighandler.
>>> Would you mind taking a look at this problem too?
>>
>> Clearing NT seems sane.
>>
>> Clearing AC seems like an ABI break, so I'd be a bit nervous about
>> clearing AC unconditionally.
>
> What exactly do you mean? Is this a documented part of ABI?
> Where can I find out how the flags are supposed to be set on
> entering a sighandler, any docs on that?
> I thought they should just be forced to some default value, the
> same as the segregs are handled.

ABI is that which existing programs rely on, which may or may not be
related to any docs. If there are AC users and they want their signal
handlers to be protected by AC, then this change would break them.

>
>> We could add yet another SS flag (sigh),
>
> But this is not a sigreturn() problem and not sigaltstack() problem,
> so what exactly flag do you mean?

I meant SA_ flag, not SS_. Whoops.

>
>> or we could make the change. As a more conservative option, we could
>> make it so that AC is cleared on entry to an alignment check signal.
>
> Hmm. But if we deliver such signal, the userspace will still
> crash, so what's the use?

Userspace could handle the SIGBUS and clear AC from regs->flags if
they were so inclined.

Anyway, maybe Linus or the x86 maintainers have some idea of how AC is
used. If there are people who use it for a whole program and if libc
can survive the experience, then they might expect even signal
handlers to run with AC set. But if they're sane and protect just
critical pieces of code being tested with AC, we could be polite and
clear AC on signal handler entry.

--Andy

--
Andy Lutomirski
AMA Capital Management, LLC

2016-03-07 21:38:33

by Alan Cox

[permalink] [raw]
Subject: Re: sigaltstack breaks swapcontext()

> Anyway, maybe Linus or the x86 maintainers have some idea of how AC is
> used. If there are people who use it for a whole program and if libc
> can survive the experience, then they might expect even signal
> handlers to run with AC set. But if they're sane and protect just
> critical pieces of code being tested with AC, we could be polite and
> clear AC on signal handler entry.

No idea about AC and signals but the main use I have seen is type
checking, particularly in old 16bit code.

Basically code does add type to address, deference. If the type is wrong
for the address you get an alignment trap.

Alan

2016-03-07 21:45:15

by Stas Sergeev

[permalink] [raw]
Subject: Re: sigaltstack breaks swapcontext()

08.03.2016 00:32, Andy Lutomirski пишет:
> On Mon, Mar 7, 2016 at 1:20 PM, Stas Sergeev <[email protected]> wrote:
>> 08.03.2016 00:10, Andy Lutomirski пишет:
>>> On Mon, Mar 7, 2016 at 12:02 PM, Stas Sergeev <[email protected]> wrote:
>>>> 09.01.2016 04:48, Andy Lutomirski пишет:
>>>>> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <[email protected]> wrote:
>>>>>> 09.01.2016 02:24, Andy Lutomirski пишет:
>>>>>>> It's not sigaltstack that I'm thinking about. It's signal delivery.
>>>>>>> If you end up in DOS mode with SP coincidentally pointing to the
>>>>>>> sigaltstack (but with different SS so it's not really the
>>>>>>> sigaltstack), then the signal delivery will malfunction.
>>>>>> Will you take care of this one?
>>>>>> Looks quite dangerous for dosemu! And absolutely
>>>>>> undebuggable: you never know when you hit it.
>>>>> I'll try to remember to tack it on to the sigcontext series.
>>>> How is this one going?
>> So what do you think about checking SS when
>> evaluating the on_sig_stack condition? Will you
>> fix this, or should I try?
> It fits in with your series, and you're welcome to do it. You can
> also poke me and get me to do it.
>
>>>> There seem to be one more bug in sigcontext handling.
>>>> dosemu have this code:
>>>> ---
>>>> /*
>>>> * FIRST thing to do in signal handlers - to avoid being trapped into
>>>> int0x11
>>>> * forever, we must restore the eflags.
>>>> */
>>>> loadflags(eflags_fs_gs.eflags);
>>>> ---
>>>>
>>>> I quickly checked the kernel code, and it seems the
>>>> flags are indeed forgotten, even on ia32! I think the
>>>> most dangerous flags are AC and NT. But most of
>>>> others are important too. IMHO the safe defaults
>>>> should be forced when entering the sighandler.
>>>> Would you mind taking a look at this problem too?
>>> Clearing NT seems sane.
>>>
>>> Clearing AC seems like an ABI break, so I'd be a bit nervous about
>>> clearing AC unconditionally.
>> What exactly do you mean? Is this a documented part of ABI?
>> Where can I find out how the flags are supposed to be set on
>> entering a sighandler, any docs on that?
>> I thought they should just be forced to some default value, the
>> same as the segregs are handled.
> ABI is that which existing programs rely on, which may or may not be
> related to any docs. If there are AC users and they want their signal
> handlers to be protected by AC, then this change would break them.
But aren't such users (I am sure there are none who use AC in
a sighandler) supposed to set the AC flag explicitly in a sighandler?
I just thought this is a bug, not an ABI feature at all. So no one
should rely on that IMHO.

>>> We could add yet another SS flag (sigh),
>> But this is not a sigreturn() problem and not sigaltstack() problem,
>> so what exactly flag do you mean?
> I meant SA_ flag, not SS_. Whoops.
But you said "yet another", and we haven't yet added any
SA_ flag here. :)

>>> or we could make the change. As a more conservative option, we could
>>> make it so that AC is cleared on entry to an alignment check signal.
>> Hmm. But if we deliver such signal, the userspace will still
>> crash, so what's the use?
> Userspace could handle the SIGBUS and clear AC from regs->flags if
> they were so inclined.
Oh, no, I'd better leave popf in the beginning of a sighandler,
as it is now done in dosemu. :)

> Anyway, maybe Linus or the x86 maintainers have some idea of how AC is
> used. If there are people who use it for a whole program and if libc
> can survive the experience, then they might expect even signal
> handlers to run with AC set.
I wonder how do they get such an expectation.
It is likely a bug, unexpected behaviour. I wonder if someone
could expect this, and I really doubt someone needs AC in a
sighandler.
So I agree, lets see what other people think, and if there are
no couter arguments, lets just force eflags to the default.