Re: [PATCH 4/4] sigaltstack: allow disabling and re-enabling sas within sighandler

From: Stas Sergeev
Date: Sun Jan 31 2016 - 18:48:53 EST

01.02.2016 01:44, Andy Lutomirski ÐÐÑÐÑ:
On Sun, Jan 31, 2016 at 2:36 PM, Stas Sergeev <stsp@xxxxxxx> wrote:
31.01.2016 23:11, Andy Lutomirski ÐÐÑÐÑ:
On Sun, Jan 31, 2016 at 12:08 PM, Stas Sergeev <stsp@xxxxxxx> wrote:
31.01.2016 22:03, Andy Lutomirski ÐÐÑÐÑ:
Also, consider a use case like yours but with *two* contexts that use
their own altstack. If you go to context A, enable sigaltstack, get a
signal, temporarily disable, then swapcontext to B, which tries to
re-enable its own sigaltstack, then everything gets confusing with
your patch, because, with your patch, the kernel is only tracking one
temporarily disabled sigaltstack.
Of course the good practice is to set the sigaltstack
before creating the contexts. Then the above scenario
should involve switching between 2 signal handlers to get
into troubles. I think the scenario with switching between
2 signal handlers is very-very unrealistic.
Why is it so unrealistic? You're already using swapcontext, which
means you're doing something like userspace threads (although I
imagine that one of your thread-like things is DOS, but still), and,
to me, that suggests that the kernel interface should be agnostic as
to how many thread-like thinks are alive.
But you only get into troubles when you switch between 2
_active signal handlers_, rather than between 2 normal contexts,
or between 2 normal context and 1 sighandler.
So I am probably misunderstanding the scenario you describe.
Without 2 sighandlers that are active at the same time and you
switch between them, how would you get into troubles?
You say "then swapcontext to B, which tries to re-enable its own
but there can be only one sigaltstack per thread, so I am quite
sure by re-enabling "its own sigaltstack" it will still do the right
As long as the kernel has a concept of a programmed but disabled
sigaltstack, something is confused when there is more than one
user-created inactive sigaltstack.
I simply don't understand how can we have more than one
sigaltstack per thread. Is this supported? If not then I don't
expect the different non-sighandler user contexts trying to
set up the different ones. So you are probably talking about
2 sighandlers switching between each other, right? And that
case is likely broken anyway.

So I don't see why you want the
kernel to remember about disabled altstacks at all.
2 reasons:
- Language-lawyering around POSIX
- Consistently return oss->ss_flags when we are on a signal stack
Restoring the old sas is not among the goals, but allowing the
sighandler to freely install the new sas (as you suggest) is a clear
contradiction to POSIX. So that's why you propose SS_FORCE, yes,
but then the question: will _anyone_ use sigaltstack(SS_ONSTACK)
in a sighandler without SS_FORCE? And the answer is likely "no".
In which case your SS_FORCE erodes to "I run it from sighandler"
but this info the kernel already knows.

I don't think this is the problem because only the signal handler
should re-enable the sigaltstack, and I don't think we really should
switch between 2 active signal handlers. And even if we did, there
can be only one sigaltstack per thread, so it will re-enable always
the right stack (there is only one).
Why would there only be one per thread?
If you mean every sighandler installs its own, then I think
switching between such sighandlers is broken anyway.
If you mean the non-sighandler contexts should install multiple
sigaltstacks, then I don't think this is supported or can work.

ISTM it would be simpler if you did:

sigaltstack(disable, force)
swapcontext() to context using sigaltstack
sigaltstack(set new altstack)

and then later

sigaltstack(disable, force) /* just in case. save old state, too. */
swapcontext() to context not using sigaltstack
sigaltstack(set new altstack)
In the real world you don't even need sigaltstack(set new altstack)
because uc_stack does this for you on rt_sigreturn. It is only my
test-case that does so.
That's only the case if swapcontext is implemented using rt_sigreturn. Is it?
No, its when you use SA_SIGINFO with sigaction().
Then the sigaltstack will magically restore itself once you
leave the sighandler. That's why I wouldn't suggest to ever
modify the sas inside the sighandler the way you propose:
it will simply not work, uc_stack will set it back. My re-enable
trick is at least in agreement with uc_stack.

If it would be POSIX compliant to allow SS_DISABLE to work even if on
the altstack even without a new flag (which is what you're
suggesting), then getting rid of the temporary in-kernel state would
considerably simplify this patch series. Just skip the -EPERM check
in the disable path.
Yes, that's why I was suggesting to just remove the EPERM
check initially. We can still do exactly that. The only problem I
can see with removing EPERM is that it would be hard to emulate
the old behaviour if need be. For example if glibc want to return
EPERM behaviour, it will have problems doing so because oss->ss_flags
doesn't say if we are on a sigaltstack and there is no other way
to find out.
...which is why I suggested SS_FORCE in the first place.
I understand. With SS_FORCE there is no temptation to emulate
the old behaviour, so there may be a fewer need to look into
oss->sa_flags even when you return it an inconsistent ways.

We probably should make a summary of our findings or they
will be forgotten.
So far I was pointing to a couple of minor problems with SS_FORCE:
- bypasses overflow protection
- prevents from asking the kernel if we are on sigaltstack or not
... and a few that I consider more important:
- does not bring any value other than to say "I am calling from sighandler"
- allows the programmer to freely modify sas while later it will
still be reset with uc_stack
(and I've likely forgot some already)

There are the upsides compared to just removing EPERM:
- fewer need to look into oss->sa_flags, so its inconsistency
became forgivable
(have I missed something else? please fill in)

There are the upsides compared to remembering the ss_flags:
- simpler code
- slightly better semantic wrt kernel threads (with my approach
restoring the context on a different kernel thread, will require
setting a separate sas there instead of restoring... but I am not
sure this is a considerably bad semantic because whoever messes
with kernel threads this way, should know how to propoerly
set sigaltstacks)

So from that summary I can agree that SS_FORCE may from
some POV be better than simply removing EPERM, as they both
share the downsides, with SS_FORCE providing minor advantages.
But I still don't see it beating my new approach.