All of lore.kernel.org
 help / color / mirror / Atom feed
* sigaltstack breaks swapcontext()
@ 2016-01-06 15:45 Stas Sergeev
  2016-01-06 18:05 ` Andy Lutomirski
  0 siblings, 1 reply; 20+ messages in thread
From: Stas Sergeev @ 2016-01-06 15:45 UTC (permalink / raw)
  To: Linux kernel; +Cc: Andy Lutomirski

Hello.

swapcontext() can be used with signal handlers,
it swaps the signal masks together with the other
parts of the context.
Unfortunately, linux implements the sigaltstack()
in a way that makes it impossible to use with
swapcontext().
Per the man page, sigaltstack is allowed to return
EPERM if the process is altering its sigaltstack while
running on sigaltstack. This is likely needed to
consistently return oss->ss_flags, that indicates
whether the process is being on sigaltstack or not.
Unfortunately, linux takes that permission to return
EPERM too literally: it returns EPERM even if you
don't want to change to another sigaltstack, but
only want to disable sigaltstack with SS_DISABLE.
To my reading of a man page, this is not a desired
behaviour. Moreover, you can't use swapcontext()
without disabling sigaltstack first, or the stack will
be re-used and overwritten by a subsequent signal.

The work-around from this, is not even trivial: I have
to use the shm tricks to duplicate the sigaltstack in
the VA space, and move the stack pointer to another
mirror before calling sigaltstack. Then I use longjmp()
to restore the stack pointer. Then I can finally use
swapcontext(). This is an unpleasant work-around.

The fix on a kernel side looks simple: kernel should
just use ss_flags to determine whether the sigaltstack
is active. I can make a patch for that, but the problem
is that the arch-specific code is not using any helper
function to check for sigaltstack; instead it just uses
"if (ss_size)" checks. So the patch will need to update
all arches... I wonder if maybe someone can fix that
problem and update the arch-specific code. If not,
I'll probably need to update only the x86-specific code
and add an arch-specific define, which is a bit nasty.

Any suggestions?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-06 15:45 sigaltstack breaks swapcontext() Stas Sergeev
@ 2016-01-06 18:05 ` Andy Lutomirski
  2016-01-06 18:42   ` Stas Sergeev
  2016-01-08 13:49   ` Stas Sergeev
  0 siblings, 2 replies; 20+ messages in thread
From: Andy Lutomirski @ 2016-01-06 18:05 UTC (permalink / raw)
  To: Stas Sergeev; +Cc: Linux kernel

On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
> Hello.
>
> swapcontext() can be used with signal handlers,
> it swaps the signal masks together with the other
> parts of the context.
> Unfortunately, linux implements the sigaltstack()
> in a way that makes it impossible to use with
> swapcontext().
> Per the man page, sigaltstack is allowed to return
> EPERM if the process is altering its sigaltstack while
> running on sigaltstack. This is likely needed to
> consistently return oss->ss_flags, that indicates
> whether the process is being on sigaltstack or not.
> Unfortunately, linux takes that permission to return
> EPERM too literally: it returns EPERM even if you
> don't want to change to another sigaltstack, but
> only want to disable sigaltstack with SS_DISABLE.
> To my reading of a man page, this is not a desired
> behaviour. Moreover, you can't use swapcontext()
> without disabling sigaltstack first, or the stack will
> be re-used and overwritten by a subsequent signal.
>

The EPERM thing is probably also to preserve the behavior that nested
SA_ONSTACK signals are supposed to work.  (Of course, the kernel gets
this a bit wrong because it forgets to check ss in addition to sp.
That would be relatively straightforward to fix.)

I don't see anything terribly wrong with allowing SS_DISABLE even if
you're on the alt stack.  You could also add a new flag SS_FORCE that
just overrides the check.

> The work-around from this, is not even trivial: I have
> to use the shm tricks to duplicate the sigaltstack in
> the VA space, and move the stack pointer to another
> mirror before calling sigaltstack. Then I use longjmp()
> to restore the stack pointer. Then I can finally use
> swapcontext(). This is an unpleasant work-around.
>
> The fix on a kernel side looks simple: kernel should
> just use ss_flags to determine whether the sigaltstack
> is active. I can make a patch for that, but the problem
> is that the arch-specific code is not using any helper
> function to check for sigaltstack; instead it just uses
> "if (ss_size)" checks.

Huh?  I'm not sure I understand what you're talking about.  It seems
reasonable to have the invariant that ss_size != 0 if and only if an
alt stack is enabled, and do_sigaltstack seems to enforce that
invariant.

> So the patch will need to update
> all arches... I wonder if maybe someone can fix that
> problem and update the arch-specific code. If not,
> I'll probably need to update only the x86-specific code
> and add an arch-specific define, which is a bit nasty.

Just change do_sigaltstack?

--Andy

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-06 18:05 ` Andy Lutomirski
@ 2016-01-06 18:42   ` Stas Sergeev
  2016-01-06 19:14     ` Andy Lutomirski
  2016-01-08 13:49   ` Stas Sergeev
  1 sibling, 1 reply; 20+ messages in thread
From: Stas Sergeev @ 2016-01-06 18:42 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

06.01.2016 21:05, Andy Lutomirski пишет:
> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>> Hello.
>>
>> swapcontext() can be used with signal handlers,
>> it swaps the signal masks together with the other
>> parts of the context.
>> Unfortunately, linux implements the sigaltstack()
>> in a way that makes it impossible to use with
>> swapcontext().
>> Per the man page, sigaltstack is allowed to return
>> EPERM if the process is altering its sigaltstack while
>> running on sigaltstack. This is likely needed to
>> consistently return oss->ss_flags, that indicates
>> whether the process is being on sigaltstack or not.
>> Unfortunately, linux takes that permission to return
>> EPERM too literally: it returns EPERM even if you
>> don't want to change to another sigaltstack, but
>> only want to disable sigaltstack with SS_DISABLE.
>> To my reading of a man page, this is not a desired
>> behaviour. Moreover, you can't use swapcontext()
>> without disabling sigaltstack first, or the stack will
>> be re-used and overwritten by a subsequent signal.
>>
> The EPERM thing is probably also to preserve the behavior that nested
> SA_ONSTACK signals are supposed to work.
Could you please clarify?
If I set up another stack inside the sighandler, the
nested SA_ONSTACK signal can just use that new stack,
which seems safe and sane. So I don't think EPERM helps
the nested signals, or could you explain the possible breakage
scenario?

>> The work-around from this, is not even trivial: I have
>> to use the shm tricks to duplicate the sigaltstack in
>> the VA space, and move the stack pointer to another
>> mirror before calling sigaltstack. Then I use longjmp()
>> to restore the stack pointer. Then I can finally use
>> swapcontext(). This is an unpleasant work-around.
>>
>> The fix on a kernel side looks simple: kernel should
>> just use ss_flags to determine whether the sigaltstack
>> is active. I can make a patch for that, but the problem
>> is that the arch-specific code is not using any helper
>> function to check for sigaltstack; instead it just uses
>> "if (ss_size)" checks.
> Huh?  I'm not sure I understand what you're talking about.  It seems
> reasonable to have the invariant that ss_size != 0 if and only if an
> alt stack is enabled, and do_sigaltstack seems to enforce that
> invariant.
But we have that (IMO quite silly) requirement that the
returned oss->ss_flags is consistent.
So if inside the signal handler I use SS_DISABLE and
the kernel translates this into "ss_size = 0", the next
call to sigaltstack() will return 0 in oss->ss_flags.

>> So the patch will need to update
>> all arches... I wonder if maybe someone can fix that
>> problem and update the arch-specific code. If not,
>> I'll probably need to update only the x86-specific code
>> and add an arch-specific define, which is a bit nasty.
> Just change do_sigaltstack?
But if its that easy and we do not even need a consistent
oss->ss_flags - why not to remove the EPERM check entirely,
rather than only for SS_DISABLE? Note that if it is removed
only for SS_DISABLE and yet SS_DISABLE is translated to
"ss_size=0", then by the next sigaltstack() call you can do
whatever you want: the EPERM check will be entirely bypassed.
So if you are fine with even this, I can send the patch to
completely remove the check. Much easier for me. :)
I think the semantic of oss->ss_size is quite bad, but it is
already documented, so I am not sure.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-06 18:42   ` Stas Sergeev
@ 2016-01-06 19:14     ` Andy Lutomirski
  2016-01-06 19:31       ` Stas Sergeev
  0 siblings, 1 reply; 20+ messages in thread
From: Andy Lutomirski @ 2016-01-06 19:14 UTC (permalink / raw)
  To: Stas Sergeev; +Cc: Linux kernel

On Wed, Jan 6, 2016 at 10:42 AM, Stas Sergeev <stsp@list.ru> wrote:
> 06.01.2016 21:05, Andy Lutomirski пишет:
>>
>> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>> Hello.
>>>
>>> swapcontext() can be used with signal handlers,
>>> it swaps the signal masks together with the other
>>> parts of the context.
>>> Unfortunately, linux implements the sigaltstack()
>>> in a way that makes it impossible to use with
>>> swapcontext().
>>> Per the man page, sigaltstack is allowed to return
>>> EPERM if the process is altering its sigaltstack while
>>> running on sigaltstack. This is likely needed to
>>> consistently return oss->ss_flags, that indicates
>>> whether the process is being on sigaltstack or not.
>>> Unfortunately, linux takes that permission to return
>>> EPERM too literally: it returns EPERM even if you
>>> don't want to change to another sigaltstack, but
>>> only want to disable sigaltstack with SS_DISABLE.
>>> To my reading of a man page, this is not a desired
>>> behaviour. Moreover, you can't use swapcontext()
>>> without disabling sigaltstack first, or the stack will
>>> be re-used and overwritten by a subsequent signal.
>>>
>> The EPERM thing is probably also to preserve the behavior that nested
>> SA_ONSTACK signals are supposed to work.
>
> Could you please clarify?
> If I set up another stack inside the sighandler, the
> nested SA_ONSTACK signal can just use that new stack,
> which seems safe and sane. So I don't think EPERM helps
> the nested signals, or could you explain the possible breakage
> scenario?

It's probably safe in most cases, but the current behavior explicitly
checks whether you're on the alt stack during signal delivery, and
probably no one wanted to think about it too hard.

It might also avoid cases where you *free* the stack you're running
on.  Arguably that would be your fault.

>
>>> The work-around from this, is not even trivial: I have
>>> to use the shm tricks to duplicate the sigaltstack in
>>> the VA space, and move the stack pointer to another
>>> mirror before calling sigaltstack. Then I use longjmp()
>>> to restore the stack pointer. Then I can finally use
>>> swapcontext(). This is an unpleasant work-around.
>>>
>>> The fix on a kernel side looks simple: kernel should
>>> just use ss_flags to determine whether the sigaltstack
>>> is active. I can make a patch for that, but the problem
>>> is that the arch-specific code is not using any helper
>>> function to check for sigaltstack; instead it just uses
>>> "if (ss_size)" checks.
>>
>> Huh?  I'm not sure I understand what you're talking about.  It seems
>> reasonable to have the invariant that ss_size != 0 if and only if an
>> alt stack is enabled, and do_sigaltstack seems to enforce that
>> invariant.
>
> But we have that (IMO quite silly) requirement that the
> returned oss->ss_flags is consistent.
> So if inside the signal handler I use SS_DISABLE and
> the kernel translates this into "ss_size = 0", the next
> call to sigaltstack() will return 0 in oss->ss_flags.

It should returns SS_DISABLE, right?  And it won't set SS_ONSTACK
because you're not in the alt stack because there is no alt stack.

Of course, there *was* an alt stack when the signal was delivered, and
you're on that stack.

>
>>> So the patch will need to update
>>> all arches... I wonder if maybe someone can fix that
>>> problem and update the arch-specific code. If not,
>>> I'll probably need to update only the x86-specific code
>>> and add an arch-specific define, which is a bit nasty.
>>
>> Just change do_sigaltstack?
>
> But if its that easy and we do not even need a consistent
> oss->ss_flags - why not to remove the EPERM check entirely,
> rather than only for SS_DISABLE? Note that if it is removed
> only for SS_DISABLE and yet SS_DISABLE is translated to
> "ss_size=0", then by the next sigaltstack() call you can do
> whatever you want: the EPERM check will be entirely bypassed.
> So if you are fine with even this, I can send the patch to
> completely remove the check. Much easier for me. :)
> I think the semantic of oss->ss_size is quite bad, but it is
> already documented, so I am not sure.

I would send a patch to remove the check or a patch to add a new
SS_FORCE that disables the check.  It should be just a couple of lines
of code.  A selftests patch along with it would help.  Cc linux-abi on
all of it.

BTW, the sigcontext SS stuff is queued for -next.  I doubt it'll make
4.5 since I think that all the relevant maintainers are just
recovering from vacations, and I already have a decent backlog of
stuff that hasn't landed in -tip yet.

--Andy

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-06 19:14     ` Andy Lutomirski
@ 2016-01-06 19:31       ` Stas Sergeev
  2016-01-06 19:53         ` Andy Lutomirski
  0 siblings, 1 reply; 20+ messages in thread
From: Stas Sergeev @ 2016-01-06 19:31 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

06.01.2016 22:14, Andy Lutomirski пишет:
> On Wed, Jan 6, 2016 at 10:42 AM, Stas Sergeev <stsp@list.ru> wrote:
>> 06.01.2016 21:05, Andy Lutomirski пишет:
>>> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>> Hello.
>>>>
>>>> swapcontext() can be used with signal handlers,
>>>> it swaps the signal masks together with the other
>>>> parts of the context.
>>>> Unfortunately, linux implements the sigaltstack()
>>>> in a way that makes it impossible to use with
>>>> swapcontext().
>>>> Per the man page, sigaltstack is allowed to return
>>>> EPERM if the process is altering its sigaltstack while
>>>> running on sigaltstack. This is likely needed to
>>>> consistently return oss->ss_flags, that indicates
>>>> whether the process is being on sigaltstack or not.
>>>> Unfortunately, linux takes that permission to return
>>>> EPERM too literally: it returns EPERM even if you
>>>> don't want to change to another sigaltstack, but
>>>> only want to disable sigaltstack with SS_DISABLE.
>>>> To my reading of a man page, this is not a desired
>>>> behaviour. Moreover, you can't use swapcontext()
>>>> without disabling sigaltstack first, or the stack will
>>>> be re-used and overwritten by a subsequent signal.
>>>>
>>> The EPERM thing is probably also to preserve the behavior that nested
>>> SA_ONSTACK signals are supposed to work.
>> Could you please clarify?
>> If I set up another stack inside the sighandler, the
>> nested SA_ONSTACK signal can just use that new stack,
>> which seems safe and sane. So I don't think EPERM helps
>> the nested signals, or could you explain the possible breakage
>> scenario?
> It's probably safe in most cases, but the current behavior explicitly
> checks whether you're on the alt stack during signal delivery,
... to not re-use it occasionally.
But if you set up a new sigaltstack, then perhaps the
kernel can just check for overlaps with the current one to stay
safe (but I'd rather not check anything).

>>>> The work-around from this, is not even trivial: I have
>>>> to use the shm tricks to duplicate the sigaltstack in
>>>> the VA space, and move the stack pointer to another
>>>> mirror before calling sigaltstack. Then I use longjmp()
>>>> to restore the stack pointer. Then I can finally use
>>>> swapcontext(). This is an unpleasant work-around.
>>>>
>>>> The fix on a kernel side looks simple: kernel should
>>>> just use ss_flags to determine whether the sigaltstack
>>>> is active. I can make a patch for that, but the problem
>>>> is that the arch-specific code is not using any helper
>>>> function to check for sigaltstack; instead it just uses
>>>> "if (ss_size)" checks.
>>> Huh?  I'm not sure I understand what you're talking about.  It seems
>>> reasonable to have the invariant that ss_size != 0 if and only if an
>>> alt stack is enabled, and do_sigaltstack seems to enforce that
>>> invariant.
>> But we have that (IMO quite silly) requirement that the
>> returned oss->ss_flags is consistent.
>> So if inside the signal handler I use SS_DISABLE and
>> the kernel translates this into "ss_size = 0", the next
>> call to sigaltstack() will return 0 in oss->ss_flags.
> It should returns SS_DISABLE, right?
Who knows?
Man page says:
        SS_ONSTACK
               The process is  currently  executing  on  the alternate  
signal
               stack.   (Note  that  it is not possible to change the 
alternate
               signal stack if the process is currently executing on it.)

        SS_DISABLE
               The alternate signal stack is currently disabled.

Both applies, and, judging from the description of EPERM, I
can assume that SS_ONSTACK should be returned.
If we remove EPERM completely, then obviously SS_DISABLE
should be returned.

>    And it won't set SS_ONSTACK
> because you're not in the alt stack because there is no alt stack.
>
> Of course, there *was* an alt stack when the signal was delivered, and
> you're on that stack.
Exactly.
Do you think this can be ignored?
A man page should then be corrected with EPERM and the
above note removed, right?

>>>> So the patch will need to update
>>>> all arches... I wonder if maybe someone can fix that
>>>> problem and update the arch-specific code. If not,
>>>> I'll probably need to update only the x86-specific code
>>>> and add an arch-specific define, which is a bit nasty.
>>> Just change do_sigaltstack?
>> But if its that easy and we do not even need a consistent
>> oss->ss_flags - why not to remove the EPERM check entirely,
>> rather than only for SS_DISABLE? Note that if it is removed
>> only for SS_DISABLE and yet SS_DISABLE is translated to
>> "ss_size=0", then by the next sigaltstack() call you can do
>> whatever you want: the EPERM check will be entirely bypassed.
>> So if you are fine with even this, I can send the patch to
>> completely remove the check. Much easier for me. :)
>> I think the semantic of oss->ss_size is quite bad, but it is
>> already documented, so I am not sure.
> I would send a patch to remove the check or a patch to add a new
> SS_FORCE that disables the check.  It should be just a couple of lines
> of code.  A selftests patch along with it would help.  Cc linux-abi on
> all of it.
Hmm, OKey. But this can potentially contradict the man page,
and I fear it can be rejected.
So don't be surprised if I add your Acked-by, warning-warning. :)

> BTW, the sigcontext SS stuff is queued for -next.  I doubt it'll make
> 4.5 since I think that all the relevant maintainers are just
> recovering from vacations, and I already have a decent backlog of
> stuff that hasn't landed in -tip yet.
Thanks for taking care of that!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-06 19:31       ` Stas Sergeev
@ 2016-01-06 19:53         ` Andy Lutomirski
  2016-01-07 15:33           ` Stas Sergeev
  0 siblings, 1 reply; 20+ messages in thread
From: Andy Lutomirski @ 2016-01-06 19:53 UTC (permalink / raw)
  To: Stas Sergeev; +Cc: Linux kernel

On Wed, Jan 6, 2016 at 11:31 AM, Stas Sergeev <stsp@list.ru> wrote:
> 06.01.2016 22:14, Andy Lutomirski пишет:
>
>> On Wed, Jan 6, 2016 at 10:42 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>> 06.01.2016 21:05, Andy Lutomirski пишет:
>>>>
>>>> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>>>
>>>>> Hello.
>>>>>
>>>>> swapcontext() can be used with signal handlers,
>>>>> it swaps the signal masks together with the other
>>>>> parts of the context.
>>>>> Unfortunately, linux implements the sigaltstack()
>>>>> in a way that makes it impossible to use with
>>>>> swapcontext().
>>>>> Per the man page, sigaltstack is allowed to return
>>>>> EPERM if the process is altering its sigaltstack while
>>>>> running on sigaltstack. This is likely needed to
>>>>> consistently return oss->ss_flags, that indicates
>>>>> whether the process is being on sigaltstack or not.
>>>>> Unfortunately, linux takes that permission to return
>>>>> EPERM too literally: it returns EPERM even if you
>>>>> don't want to change to another sigaltstack, but
>>>>> only want to disable sigaltstack with SS_DISABLE.
>>>>> To my reading of a man page, this is not a desired
>>>>> behaviour. Moreover, you can't use swapcontext()
>>>>> without disabling sigaltstack first, or the stack will
>>>>> be re-used and overwritten by a subsequent signal.
>>>>>
>>>> The EPERM thing is probably also to preserve the behavior that nested
>>>> SA_ONSTACK signals are supposed to work.
>>>
>>> Could you please clarify?
>>> If I set up another stack inside the sighandler, the
>>> nested SA_ONSTACK signal can just use that new stack,
>>> which seems safe and sane. So I don't think EPERM helps
>>> the nested signals, or could you explain the possible breakage
>>> scenario?
>>
>> It's probably safe in most cases, but the current behavior explicitly
>> checks whether you're on the alt stack during signal delivery,
>
> ... to not re-use it occasionally.
> But if you set up a new sigaltstack, then perhaps the
> kernel can just check for overlaps with the current one to stay
> safe (but I'd rather not check anything).
>
>>>>> The work-around from this, is not even trivial: I have
>>>>> to use the shm tricks to duplicate the sigaltstack in
>>>>> the VA space, and move the stack pointer to another
>>>>> mirror before calling sigaltstack. Then I use longjmp()
>>>>> to restore the stack pointer. Then I can finally use
>>>>> swapcontext(). This is an unpleasant work-around.
>>>>>
>>>>> The fix on a kernel side looks simple: kernel should
>>>>> just use ss_flags to determine whether the sigaltstack
>>>>> is active. I can make a patch for that, but the problem
>>>>> is that the arch-specific code is not using any helper
>>>>> function to check for sigaltstack; instead it just uses
>>>>> "if (ss_size)" checks.
>>>>
>>>> Huh?  I'm not sure I understand what you're talking about.  It seems
>>>> reasonable to have the invariant that ss_size != 0 if and only if an
>>>> alt stack is enabled, and do_sigaltstack seems to enforce that
>>>> invariant.
>>>
>>> But we have that (IMO quite silly) requirement that the
>>> returned oss->ss_flags is consistent.
>>> So if inside the signal handler I use SS_DISABLE and
>>> the kernel translates this into "ss_size = 0", the next
>>> call to sigaltstack() will return 0 in oss->ss_flags.
>>
>> It should returns SS_DISABLE, right?
>
> Who knows?
> Man page says:
>        SS_ONSTACK
>               The process is  currently  executing  on  the alternate
> signal
>               stack.   (Note  that  it is not possible to change the
> alternate
>               signal stack if the process is currently executing on it.)
>
>        SS_DISABLE
>               The alternate signal stack is currently disabled.
>
> Both applies, and, judging from the description of EPERM, I
> can assume that SS_ONSTACK should be returned.
> If we remove EPERM completely, then obviously SS_DISABLE
> should be returned.
>
>>    And it won't set SS_ONSTACK
>> because you're not in the alt stack because there is no alt stack.
>>
>> Of course, there *was* an alt stack when the signal was delivered, and
>> you're on that stack.
>
> Exactly.
> Do you think this can be ignored?
> A man page should then be corrected with EPERM and the
> above note removed, right?
>

I think it can be ignored.  I'd go the SS_FORCE route, though, to
maintain POSIX compliance.

>>>>> So the patch will need to update
>>>>> all arches... I wonder if maybe someone can fix that
>>>>> problem and update the arch-specific code. If not,
>>>>> I'll probably need to update only the x86-specific code
>>>>> and add an arch-specific define, which is a bit nasty.
>>>>
>>>> Just change do_sigaltstack?
>>>
>>> But if its that easy and we do not even need a consistent
>>> oss->ss_flags - why not to remove the EPERM check entirely,
>>> rather than only for SS_DISABLE? Note that if it is removed
>>> only for SS_DISABLE and yet SS_DISABLE is translated to
>>> "ss_size=0", then by the next sigaltstack() call you can do
>>> whatever you want: the EPERM check will be entirely bypassed.
>>> So if you are fine with even this, I can send the patch to
>>> completely remove the check. Much easier for me. :)
>>> I think the semantic of oss->ss_size is quite bad, but it is
>>> already documented, so I am not sure.
>>
>> I would send a patch to remove the check or a patch to add a new
>> SS_FORCE that disables the check.  It should be just a couple of lines
>> of code.  A selftests patch along with it would help.  Cc linux-abi on
>> all of it.
>
> Hmm, OKey. But this can potentially contradict the man page,
> and I fear it can be rejected.
> So don't be surprised if I add your Acked-by, warning-warning. :)
>

You can add my Suggested-by.  No Acked-by until I actually ack it,
which involves seeing the code :)

>> BTW, the sigcontext SS stuff is queued for -next.  I doubt it'll make
>> 4.5 since I think that all the relevant maintainers are just
>> recovering from vacations, and I already have a decent backlog of
>> stuff that hasn't landed in -tip yet.
>
> Thanks for taking care of that!



-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-06 19:53         ` Andy Lutomirski
@ 2016-01-07 15:33           ` Stas Sergeev
  2016-01-07 17:23             ` Andy Lutomirski
  0 siblings, 1 reply; 20+ messages in thread
From: Stas Sergeev @ 2016-01-07 15:33 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

06.01.2016 22:53, Andy Lutomirski пишет:
> On Wed, Jan 6, 2016 at 11:31 AM, Stas Sergeev <stsp@list.ru> wrote:
>> Exactly.
>> Do you think this can be ignored?
>> A man page should then be corrected with EPERM and the
>> above note removed, right?
>>
> I think it can be ignored.  I'd go the SS_FORCE route, though, to
> maintain POSIX compliance.
I think such a flag would be a wrong thing to do.
Allowing only SS_DISABLE (without any new flags) keeps
you still "compatible with posix", and anything beyond
SS_DISABLE in a sighandler is not needed.

So I think we only have the following options:
1. Remove the check and forget (if anything, glibc can
add the EPERM check to stay compatible with crap).
2. Allow only SS_DISABLE. This will mean a large patch,
touching all arches, but the bonus is the compatibility
with posix, that no one needs in this particular case.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-07 15:33           ` Stas Sergeev
@ 2016-01-07 17:23             ` Andy Lutomirski
  2016-01-07 19:10               ` Stas Sergeev
  0 siblings, 1 reply; 20+ messages in thread
From: Andy Lutomirski @ 2016-01-07 17:23 UTC (permalink / raw)
  To: Stas Sergeev; +Cc: Linux kernel

On Thu, Jan 7, 2016 at 7:33 AM, Stas Sergeev <stsp@list.ru> wrote:
> 06.01.2016 22:53, Andy Lutomirski пишет:
>>
>> On Wed, Jan 6, 2016 at 11:31 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>> Exactly.
>>> Do you think this can be ignored?
>>> A man page should then be corrected with EPERM and the
>>> above note removed, right?
>>>
>> I think it can be ignored.  I'd go the SS_FORCE route, though, to
>> maintain POSIX compliance.
>
> I think such a flag would be a wrong thing to do.
> Allowing only SS_DISABLE (without any new flags) keeps
> you still "compatible with posix", and anything beyond
> SS_DISABLE in a sighandler is not needed.
>
> So I think we only have the following options:
> 1. Remove the check and forget (if anything, glibc can
> add the EPERM check to stay compatible with crap).
> 2. Allow only SS_DISABLE. This will mean a large patch,
> touching all arches, but the bonus is the compatibility
> with posix, that no one needs in this particular case.

Why does allowing SS_DISABLE require touching all arches?

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-07 17:23             ` Andy Lutomirski
@ 2016-01-07 19:10               ` Stas Sergeev
  0 siblings, 0 replies; 20+ messages in thread
From: Stas Sergeev @ 2016-01-07 19:10 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

07.01.2016 20:23, Andy Lutomirski пишет:
> On Thu, Jan 7, 2016 at 7:33 AM, Stas Sergeev <stsp@list.ru> wrote:
>> 06.01.2016 22:53, Andy Lutomirski пишет:
>>> On Wed, Jan 6, 2016 at 11:31 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>> Exactly.
>>>> Do you think this can be ignored?
>>>> A man page should then be corrected with EPERM and the
>>>> above note removed, right?
>>>>
>>> I think it can be ignored.  I'd go the SS_FORCE route, though, to
>>> maintain POSIX compliance.
>> I think such a flag would be a wrong thing to do.
>> Allowing only SS_DISABLE (without any new flags) keeps
>> you still "compatible with posix", and anything beyond
>> SS_DISABLE in a sighandler is not needed.
>>
>> So I think we only have the following options:
>> 1. Remove the check and forget (if anything, glibc can
>> add the EPERM check to stay compatible with crap).
>> 2. Allow only SS_DISABLE. This will mean a large patch,
>> touching all arches, but the bonus is the compatibility
>> with posix, that no one needs in this particular case.
> Why does allowing SS_DISABLE require touching all arches?
I mean, if we also consistently return SA_ONSTACK even
after SS_DISABLE. This will require kernel to save the
ss_flags separately, and not to use "if (ss_size)" checks.
Of course if we don't want to return SA_ONSTACK, we
should just remove EPERM as I don't think it serves any
other purpose than that.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-06 18:05 ` Andy Lutomirski
  2016-01-06 18:42   ` Stas Sergeev
@ 2016-01-08 13:49   ` Stas Sergeev
  2016-01-08 23:24     ` Andy Lutomirski
  1 sibling, 1 reply; 20+ messages in thread
From: Stas Sergeev @ 2016-01-08 13:49 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

06.01.2016 21:05, Andy Lutomirski пишет:
> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>> Hello.
>>
>> swapcontext() can be used with signal handlers,
>> it swaps the signal masks together with the other
>> parts of the context.
>> Unfortunately, linux implements the sigaltstack()
>> in a way that makes it impossible to use with
>> swapcontext().
>> Per the man page, sigaltstack is allowed to return
>> EPERM if the process is altering its sigaltstack while
>> running on sigaltstack. This is likely needed to
>> consistently return oss->ss_flags, that indicates
>> whether the process is being on sigaltstack or not.
>> Unfortunately, linux takes that permission to return
>> EPERM too literally: it returns EPERM even if you
>> don't want to change to another sigaltstack, but
>> only want to disable sigaltstack with SS_DISABLE.
>> To my reading of a man page, this is not a desired
>> behaviour. Moreover, you can't use swapcontext()
>> without disabling sigaltstack first, or the stack will
>> be re-used and overwritten by a subsequent signal.
>>
> The EPERM thing is probably also to preserve the behavior that nested
> SA_ONSTACK signals are supposed to work.  (Of course, the kernel gets
> this a bit wrong because it forgets to check ss in addition to sp.
> That would be relatively straightforward to fix.)
I don't think it needs a fix: in 64bit mode SS doesn't matter, and
in 32bit mode the SS is properly restored in a sighandler, so no
one can run sigaltstack() with non-flat SS (unless the DOS code
itself does this, which it does not).

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-08 13:49   ` Stas Sergeev
@ 2016-01-08 23:24     ` Andy Lutomirski
  2016-01-08 23:40       ` Stas Sergeev
  2016-01-09  1:43       ` Stas Sergeev
  0 siblings, 2 replies; 20+ messages in thread
From: Andy Lutomirski @ 2016-01-08 23:24 UTC (permalink / raw)
  To: Stas Sergeev; +Cc: Linux kernel

On Fri, Jan 8, 2016 at 5:49 AM, Stas Sergeev <stsp@list.ru> wrote:
> 06.01.2016 21:05, Andy Lutomirski пишет:
>>
>> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>>
>>> Hello.
>>>
>>> swapcontext() can be used with signal handlers,
>>> it swaps the signal masks together with the other
>>> parts of the context.
>>> Unfortunately, linux implements the sigaltstack()
>>> in a way that makes it impossible to use with
>>> swapcontext().
>>> Per the man page, sigaltstack is allowed to return
>>> EPERM if the process is altering its sigaltstack while
>>> running on sigaltstack. This is likely needed to
>>> consistently return oss->ss_flags, that indicates
>>> whether the process is being on sigaltstack or not.
>>> Unfortunately, linux takes that permission to return
>>> EPERM too literally: it returns EPERM even if you
>>> don't want to change to another sigaltstack, but
>>> only want to disable sigaltstack with SS_DISABLE.
>>> To my reading of a man page, this is not a desired
>>> behaviour. Moreover, you can't use swapcontext()
>>> without disabling sigaltstack first, or the stack will
>>> be re-used and overwritten by a subsequent signal.
>>>
>> The EPERM thing is probably also to preserve the behavior that nested
>> SA_ONSTACK signals are supposed to work.  (Of course, the kernel gets
>> this a bit wrong because it forgets to check ss in addition to sp.
>> That would be relatively straightforward to fix.)
>
> I don't think it needs a fix: in 64bit mode SS doesn't matter, and
> in 32bit mode the SS is properly restored in a sighandler, so no
> one can run sigaltstack() with non-flat SS (unless the DOS code
> itself does this, which it does not).

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.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-08 23:24     ` Andy Lutomirski
@ 2016-01-08 23:40       ` Stas Sergeev
  2016-01-09  1:43       ` Stas Sergeev
  1 sibling, 0 replies; 20+ messages in thread
From: Stas Sergeev @ 2016-01-08 23:40 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

09.01.2016 02:24, Andy Lutomirski пишет:
> On Fri, Jan 8, 2016 at 5:49 AM, Stas Sergeev <stsp@list.ru> wrote:
>> 06.01.2016 21:05, Andy Lutomirski пишет:
>>> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>>> Hello.
>>>>
>>>> swapcontext() can be used with signal handlers,
>>>> it swaps the signal masks together with the other
>>>> parts of the context.
>>>> Unfortunately, linux implements the sigaltstack()
>>>> in a way that makes it impossible to use with
>>>> swapcontext().
>>>> Per the man page, sigaltstack is allowed to return
>>>> EPERM if the process is altering its sigaltstack while
>>>> running on sigaltstack. This is likely needed to
>>>> consistently return oss->ss_flags, that indicates
>>>> whether the process is being on sigaltstack or not.
>>>> Unfortunately, linux takes that permission to return
>>>> EPERM too literally: it returns EPERM even if you
>>>> don't want to change to another sigaltstack, but
>>>> only want to disable sigaltstack with SS_DISABLE.
>>>> To my reading of a man page, this is not a desired
>>>> behaviour. Moreover, you can't use swapcontext()
>>>> without disabling sigaltstack first, or the stack will
>>>> be re-used and overwritten by a subsequent signal.
>>>>
>>> The EPERM thing is probably also to preserve the behavior that nested
>>> SA_ONSTACK signals are supposed to work.  (Of course, the kernel gets
>>> this a bit wrong because it forgets to check ss in addition to sp.
>>> That would be relatively straightforward to fix.)
>> I don't think it needs a fix: in 64bit mode SS doesn't matter, and
>> in 32bit mode the SS is properly restored in a sighandler, so no
>> one can run sigaltstack() with non-flat SS (unless the DOS code
>> itself does this, which it does not).
> 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.
Ah, sounds like a real bug then!
Though if bitness differ (64bit mode and signal comes from
32bit code), there is probably no need to check anything and
just switch the stack.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-08 23:24     ` Andy Lutomirski
  2016-01-08 23:40       ` Stas Sergeev
@ 2016-01-09  1:43       ` Stas Sergeev
  2016-01-09  1:48         ` Andy Lutomirski
  1 sibling, 1 reply; 20+ messages in thread
From: Stas Sergeev @ 2016-01-09  1:43 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

09.01.2016 02:24, Andy Lutomirski пишет:
> On Fri, Jan 8, 2016 at 5:49 AM, Stas Sergeev <stsp@list.ru> wrote:
>> 06.01.2016 21:05, Andy Lutomirski пишет:
>>> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>>> Hello.
>>>>
>>>> swapcontext() can be used with signal handlers,
>>>> it swaps the signal masks together with the other
>>>> parts of the context.
>>>> Unfortunately, linux implements the sigaltstack()
>>>> in a way that makes it impossible to use with
>>>> swapcontext().
>>>> Per the man page, sigaltstack is allowed to return
>>>> EPERM if the process is altering its sigaltstack while
>>>> running on sigaltstack. This is likely needed to
>>>> consistently return oss->ss_flags, that indicates
>>>> whether the process is being on sigaltstack or not.
>>>> Unfortunately, linux takes that permission to return
>>>> EPERM too literally: it returns EPERM even if you
>>>> don't want to change to another sigaltstack, but
>>>> only want to disable sigaltstack with SS_DISABLE.
>>>> To my reading of a man page, this is not a desired
>>>> behaviour. Moreover, you can't use swapcontext()
>>>> without disabling sigaltstack first, or the stack will
>>>> be re-used and overwritten by a subsequent signal.
>>>>
>>> The EPERM thing is probably also to preserve the behavior that nested
>>> SA_ONSTACK signals are supposed to work.  (Of course, the kernel gets
>>> this a bit wrong because it forgets to check ss in addition to sp.
>>> That would be relatively straightforward to fix.)
>> I don't think it needs a fix: in 64bit mode SS doesn't matter, and
>> in 32bit mode the SS is properly restored in a sighandler, so no
>> one can run sigaltstack() with non-flat SS (unless the DOS code
>> itself does this, which it does not).
> 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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-09  1:43       ` Stas Sergeev
@ 2016-01-09  1:48         ` Andy Lutomirski
  2016-03-07 20:02           ` Stas Sergeev
  0 siblings, 1 reply; 20+ messages in thread
From: Andy Lutomirski @ 2016-01-09  1:48 UTC (permalink / raw)
  To: Stas Sergeev; +Cc: Linux kernel

On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <stsp@list.ru> wrote:
> 09.01.2016 02:24, Andy Lutomirski пишет:
>>
>> On Fri, Jan 8, 2016 at 5:49 AM, Stas Sergeev <stsp@list.ru> wrote:
>>
>>> 06.01.2016 21:05, Andy Lutomirski пишет:
>>>>
>>>> On Wed, Jan 6, 2016 at 7:45 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>>
>>>>> Hello.
>>>>>
>>>>> swapcontext() can be used with signal handlers,
>>>>> it swaps the signal masks together with the other
>>>>> parts of the context.
>>>>> Unfortunately, linux implements the sigaltstack()
>>>>> in a way that makes it impossible to use with
>>>>> swapcontext().
>>>>> Per the man page, sigaltstack is allowed to return
>>>>> EPERM if the process is altering its sigaltstack while
>>>>> running on sigaltstack. This is likely needed to
>>>>> consistently return oss->ss_flags, that indicates
>>>>> whether the process is being on sigaltstack or not.
>>>>> Unfortunately, linux takes that permission to return
>>>>> EPERM too literally: it returns EPERM even if you
>>>>> don't want to change to another sigaltstack, but
>>>>> only want to disable sigaltstack with SS_DISABLE.
>>>>> To my reading of a man page, this is not a desired
>>>>> behaviour. Moreover, you can't use swapcontext()
>>>>> without disabling sigaltstack first, or the stack will
>>>>> be re-used and overwritten by a subsequent signal.
>>>>>
>>>> The EPERM thing is probably also to preserve the behavior that nested
>>>> SA_ONSTACK signals are supposed to work.  (Of course, the kernel gets
>>>> this a bit wrong because it forgets to check ss in addition to sp.
>>>> That would be relatively straightforward to fix.)
>>>
>>> I don't think it needs a fix: in 64bit mode SS doesn't matter, and
>>> in 32bit mode the SS is properly restored in a sighandler, so no
>>> one can run sigaltstack() with non-flat SS (unless the DOS code
>>> itself does this, which it does not).
>>
>> 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.

--Andy

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-01-09  1:48         ` Andy Lutomirski
@ 2016-03-07 20:02           ` Stas Sergeev
  2016-03-07 21:10             ` Andy Lutomirski
  0 siblings, 1 reply; 20+ messages in thread
From: Stas Sergeev @ 2016-03-07 20:02 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

09.01.2016 04:48, Andy Lutomirski пишет:
> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <stsp@list.ru> 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?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-03-07 20:02           ` Stas Sergeev
@ 2016-03-07 21:10             ` Andy Lutomirski
  2016-03-07 21:20               ` Stas Sergeev
  0 siblings, 1 reply; 20+ messages in thread
From: Andy Lutomirski @ 2016-03-07 21:10 UTC (permalink / raw)
  To: Stas Sergeev; +Cc: Linux kernel

On Mon, Mar 7, 2016 at 12:02 PM, Stas Sergeev <stsp@list.ru> wrote:
> 09.01.2016 04:48, Andy Lutomirski пишет:
>>
>> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <stsp@list.ru> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-03-07 21:10             ` Andy Lutomirski
@ 2016-03-07 21:20               ` Stas Sergeev
  2016-03-07 21:32                 ` Andy Lutomirski
  0 siblings, 1 reply; 20+ messages in thread
From: Stas Sergeev @ 2016-03-07 21:20 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Linux kernel

08.03.2016 00:10, Andy Lutomirski пишет:
> On Mon, Mar 7, 2016 at 12:02 PM, Stas Sergeev <stsp@list.ru> wrote:
>> 09.01.2016 04:48, Andy Lutomirski пишет:
>>> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <stsp@list.ru> 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?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-03-07 21:20               ` Stas Sergeev
@ 2016-03-07 21:32                 ` Andy Lutomirski
  2016-03-07 21:38                   ` One Thousand Gnomes
  2016-03-07 21:45                   ` Stas Sergeev
  0 siblings, 2 replies; 20+ messages in thread
From: Andy Lutomirski @ 2016-03-07 21:32 UTC (permalink / raw)
  To: Stas Sergeev, Linus Torvalds, X86 ML; +Cc: Linux kernel

On Mon, Mar 7, 2016 at 1:20 PM, Stas Sergeev <stsp@list.ru> wrote:
> 08.03.2016 00:10, Andy Lutomirski пишет:
>>
>> On Mon, Mar 7, 2016 at 12:02 PM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>> 09.01.2016 04:48, Andy Lutomirski пишет:
>>>>
>>>> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <stsp@list.ru> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-03-07 21:32                 ` Andy Lutomirski
@ 2016-03-07 21:38                   ` One Thousand Gnomes
  2016-03-07 21:45                   ` Stas Sergeev
  1 sibling, 0 replies; 20+ messages in thread
From: One Thousand Gnomes @ 2016-03-07 21:38 UTC (permalink / raw)
  To: Andy Lutomirski; +Cc: Stas Sergeev, Linus Torvalds, X86 ML, Linux kernel

> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sigaltstack breaks swapcontext()
  2016-03-07 21:32                 ` Andy Lutomirski
  2016-03-07 21:38                   ` One Thousand Gnomes
@ 2016-03-07 21:45                   ` Stas Sergeev
  1 sibling, 0 replies; 20+ messages in thread
From: Stas Sergeev @ 2016-03-07 21:45 UTC (permalink / raw)
  To: Andy Lutomirski, Linus Torvalds, X86 ML; +Cc: Linux kernel

08.03.2016 00:32, Andy Lutomirski пишет:
> On Mon, Mar 7, 2016 at 1:20 PM, Stas Sergeev <stsp@list.ru> wrote:
>> 08.03.2016 00:10, Andy Lutomirski пишет:
>>> On Mon, Mar 7, 2016 at 12:02 PM, Stas Sergeev <stsp@list.ru> wrote:
>>>> 09.01.2016 04:48, Andy Lutomirski пишет:
>>>>> On Fri, Jan 8, 2016 at 5:43 PM, Stas Sergeev <stsp@list.ru> 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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-03-07 21:45 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-06 15:45 sigaltstack breaks swapcontext() Stas Sergeev
2016-01-06 18:05 ` Andy Lutomirski
2016-01-06 18:42   ` Stas Sergeev
2016-01-06 19:14     ` Andy Lutomirski
2016-01-06 19:31       ` Stas Sergeev
2016-01-06 19:53         ` Andy Lutomirski
2016-01-07 15:33           ` Stas Sergeev
2016-01-07 17:23             ` Andy Lutomirski
2016-01-07 19:10               ` Stas Sergeev
2016-01-08 13:49   ` Stas Sergeev
2016-01-08 23:24     ` Andy Lutomirski
2016-01-08 23:40       ` Stas Sergeev
2016-01-09  1:43       ` Stas Sergeev
2016-01-09  1:48         ` Andy Lutomirski
2016-03-07 20:02           ` Stas Sergeev
2016-03-07 21:10             ` Andy Lutomirski
2016-03-07 21:20               ` Stas Sergeev
2016-03-07 21:32                 ` Andy Lutomirski
2016-03-07 21:38                   ` One Thousand Gnomes
2016-03-07 21:45                   ` Stas Sergeev

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.