linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Stas Sergeev <stsp@list.ru>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Raymond Jennings <shentino@gmail.com>,
	Cyrill Gorcunov <gorcunov@gmail.com>,
	Pavel Emelyanov <xemul@parallels.com>,
	Linux kernel <linux-kernel@vger.kernel.org>
Subject: Re: [regression] x86/signal/64: Fix SS handling for signals delivered to 64-bit programs breaks dosemu
Date: Wed, 2 Sep 2015 14:39:31 -0700	[thread overview]
Message-ID: <CALCETrVphMtmb+tW395Tj2LAa5DReh5b5j5oMjnMcp+GsMtH7g@mail.gmail.com> (raw)
In-Reply-To: <55E7638F.1060002@list.ru>

On Wed, Sep 2, 2015 at 2:01 PM, Stas Sergeev <stsp@list.ru> wrote:
> 02.09.2015 22:06, Andy Lutomirski пишет:
>
>> On Wed, Sep 2, 2015 at 11:23 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>
>>> 02.09.2015 21:17, Andy Lutomirski пишет:
>>>>
>>>> On Wed, Sep 2, 2015 at 10:46 AM, Stas Sergeev <stsp@list.ru> wrote:
>>>>>
>>>>> 02.09.2015 17:21, Andy Lutomirski пишет:
>>>>>>>>
>>>>>>>> This should work for old DOSEMU.  It's a bit gross, but it has the
>>>>>>>> nice benefit that everyone (even things that aren't DOSEMU) gain the
>>>>>>>> ability to catch signals thrown from bogus SS contexts, which
>>>>>>>> probably
>>>>>>>> improves debugability.  It's also nice to not have the SA flag.
>>>>>>>
>>>>>>> Pros:
>>>>>>> - No new SA flag
>>>>>>> - May improve debugability in some unknown scenario where people
>>>>>>> do not want to just use the new flag to get their things improved
>>>>>>>
>>>>>>> Cons:
>>>>>>> - Does not allow to cleanly use siglongjmp(), as then there is a risk
>>>>>>> to jump to 64bit code with bad SS
>>>>>>
>>>>>> What's the issue here?  I don't understand.
>>>>>>
>>>>>> On musl, (sig)longjmp just restores rsp, rbx, rbp, and r12-r15, so it
>>>>>> won't be affected.  AFAIK all implementations of siglongjmp are likely
>>>>>> to call sigprocmask or similar, and that will clobber SS.  I'm not
>>>>>> aware of an implementation of siglongjmp that uses sigreturn.
>>>>>
>>>>> I am not saying siglongjmp() will be affected.
>>>>> Quite the opposite: it won't, which is bad. :)
>>>>> If you have always correct SS, you can use siglongjmp(). If you have
>>>>> broken SS at times, siglongjmp() will be an asking for troubles, as
>>>>> it exactly does not restore SS.
>>>>> dosemu could do a good use of siglongjmp() to get back to 64bit code
>>>>> from its sighandler.
>>>>
>>>> This seems like it would be relying unpleasantly heavily on libc
>>>> internals.
>>>
>>> Could you please clarify?
>>> If kernel always passes the right SS to the sighandler, then what's
>>> the problem?
>>
>> What's the exact siglongjmp usage you have in mind?  Signal context
>> isn't normally involved AFAIK.
>
> dosemu needs 2 return pathes:
> 1. to DOS code
> 2. to 64bit code (dosemu is not all in a sighandler, right?)
>
> How it is currently achieved:
> dosemu1:
> 1. sigreturn() + iret (to DOS)
> 2. modify sigcontext -> sigreturn() (to 64bit asm helper)
>
> dosemu2:
> 1. sigreturn() + iret (to DOS)
> 2. modify sigcontext -> sigreturn() -> longjmp() (to 64bit C-coded)

So you're modifying sigcontext such that it returns to a C function
that calls longjmp?

>
> How dosemu2 is supposed to do this:
> 1. sigreturn() (to DOS)
> 2. siglongjmp() (to 64bit C-coded)

This should work fine on any kernel, right?  The main problem will be
that you presumably need to remember the old context so you can go
back to DOS, I assume.  So SS needs to be there somewhere.

>
>>>>>>> - Async signals can silently "validate" SS behind your back
>>>>>>
>>>>>> True, and that's unfortunate.  But async signals without SA_SAVE_SS
>>>>>> set with the other approach have exactly the same problem.
>>>>>
>>>>> Yes, and as such, they should be blocked.
>>>>> You could improve on that and on siglongjmp().
>>>>> And on TLS in the future.
>>>>
>>>> *I* can't do anything to siglongjmp because that's almost entirely
>>>> outside the kernel. :-/
>>>
>>> Except for passing the SS=__USER_DS to the sighandler, for which we
>>> discussed the new SA_hyz?
>>
>> I'm still not understanding what you're looking for.  If you
>> siglongjmp out of a signal handler, the hardware SS value is
>> irrelevant, at least on 64-bit binaries, because siglongjmp is just
>> going to replace it.
>
> Hmm? IIRC you've just said this:
> ---
> On musl, (sig)longjmp just restores rsp, rbx, rbp, and r12-r15, so it won't
> be affected.
> ---
> So why would siglongjmp() replace it?

Because siglongjmp calls sigprocmask, which uses SYSCALL, which clobbers SS.

>
>>>>>>> Is the new SA flag such a big deal here to even bother?
>>>>>>
>>>>>> Not really, but given that the new behavior seems clearly better
>>>>>> behaved than the old, it would be nice to be able to have the good
>>>>>> behavior, or at least most of it, be the default.
>>>>>
>>>>> Surely, but how about then having the heuristics you suggest,
>>>>> only if the new SA_hyz is not set? And when it is set, have a
>>>>> properly defined and predictable behaviour. Then it seems like
>>>>> we'll get all the possible wishes covered.
>>>>
>>>> That could work.  The result is quite similar to explicitly setting
>>>> UC_STRICT_RESTORE_SS.
>>>
>>> I am much more bothered with delivering the right SS than with
>>> restoring it on sigreturn().
>>
>> For 64-bit delivery, ignoring backwards compatibility, delivering
>> signals with ss = __USER_DS would be the right solution, I think: it's
>> trivial and it works.  Because of backwards compatibility, we need to
>
> ... add the SA_hyz flag.
> I don't understand why do you constantly ignore that part as
> if it was never spelled. Lets discuss the proposal as a whole, rather
> than with the random bits thrown away. The flag is exactly for
> backward compatibility, so why do you present it as a problem
> without the context of the new flag?

For backwards compat, we either need the default behavior to be
unchanged, or we need the default behavior to be something that works
with existing dosemu.  For existing dosemu, the only interesting cases
(I think) are signal delivery from *valid* 16-bit context, in which
case we need to preserve SS so that the signal handler can read it out
with mov ..., %ss, and sigreturn to 64-bit mode for the IRET
trampoline.  For sigreturn, IIUC old dosemu will replace the saved CS
with a 64-bit code segment selector and won't touch the saved SS
because it doesn't know about the saved SS.  Those dosemu versions
don't care what SS actually contains after sigreturn, because they're
immediately going to change it again using IRET.  So we just need to
make sure we return without faulting.

New dosemu2 would like to sigreturn directly back to 16-bit mode, so
it needs the kernel to honor the saved ss value and restore it,
possibly changed by dosemu.

We obviously can't require old dosemu to set an SA flag to keep
working.  But, if we can get away with it, I think it's somewhat
preferable not to require new DOSEMU to set an SA flag either.

This has one major benefit at least: if new dosemu loads some random
library that installs some async signal handler (SIGALRM for example),
everything will work with regard to CS and SS.  If SIGALRM hits 16-bit
code, CS and SS get saved, the signal handler gets invoked in 64-bit
mode, and sigreturn restores the old state.

Of course, FS and GS still screw this up.

--Andy

  reply	other threads:[~2015-09-02 21:39 UTC|newest]

Thread overview: 121+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-12  0:17 [regression] x86/signal/64: Fix SS handling for signals delivered to 64-bit programs breaks dosemu Stas Sergeev
2015-08-12  0:38 ` Andy Lutomirski
2015-08-12  8:02   ` Stas Sergeev
2015-08-12 16:19     ` Andy Lutomirski
2015-08-12 17:00       ` Stas Sergeev
2015-08-12 18:25         ` Andy Lutomirski
2015-08-12 18:55           ` Stas Sergeev
2015-08-12 19:20             ` Andy Lutomirski
2015-08-12 19:55               ` Stas Sergeev
2015-08-12 20:01                 ` Andy Lutomirski
2015-08-12 20:14                   ` Stas Sergeev
2015-08-12 20:28                     ` Andy Lutomirski
2015-08-12 20:45                       ` Stas Sergeev
2015-08-12 20:47                         ` Andy Lutomirski
2015-08-12 20:55                           ` Stas Sergeev
2015-08-12 21:37                             ` Andy Lutomirski
2015-08-12 21:50                               ` Stas Sergeev
2015-08-12 22:00                                 ` Andy Lutomirski
2015-08-13  8:39                                   ` Ingo Molnar
2015-08-13 10:14                                     ` Stas Sergeev
2015-08-13 12:44                                     ` Stas Sergeev
2015-08-13 14:58                                       ` Andy Lutomirski
2015-08-13 15:22                                         ` Stas Sergeev
2015-08-13 15:38                                           ` Andy Lutomirski
2015-08-13 16:03                                             ` Stas Sergeev
2015-08-13 16:09                                               ` Andy Lutomirski
2015-08-13 16:20                                                 ` Stas Sergeev
2015-08-13 16:24                                                   ` Andy Lutomirski
2015-08-13 16:38                                                     ` Stas Sergeev
2015-08-13 16:42                                                       ` Andy Lutomirski
2015-08-13 16:48                                                         ` Stas Sergeev
2015-08-13 16:59                                                           ` Andy Lutomirski
2015-08-13 17:13                                                             ` Stas Sergeev
2015-08-13 17:17                                                               ` Andy Lutomirski
2015-08-13 18:00                                                                 ` Stas Sergeev
2015-08-13 18:05                                                                   ` Andy Lutomirski
2015-08-13 18:19                                                                     ` Stas Sergeev
2015-08-13 18:25                                                                       ` Andy Lutomirski
2015-08-13 18:35                                                                         ` Stas Sergeev
2015-08-22 12:38                                             ` Ingo Molnar
2015-08-22 14:19                                               ` Stas Sergeev
2015-08-23  6:25                                                 ` Ingo Molnar
2015-08-13 11:08                                   ` Stas Sergeev
2015-08-13 15:37 ` Linus Torvalds
2015-08-13 15:43   ` Andy Lutomirski
2015-08-13 16:19     ` Linus Torvalds
2015-08-13 16:23       ` Andy Lutomirski
2015-08-13 16:34         ` Linus Torvalds
2015-08-13 16:43           ` Linus Torvalds
2015-08-13 16:44             ` Andy Lutomirski
2015-08-13 17:00     ` Brian Gerst
2015-08-18  6:29       ` Stas Sergeev
2015-08-18 22:42         ` Andy Lutomirski
2015-08-18 22:47           ` Andy Lutomirski
2015-08-19  9:35             ` Stas Sergeev
2015-08-19 15:46               ` Andy Lutomirski
2015-08-19 16:30                 ` Stas Sergeev
2015-09-02  5:12                   ` Andy Lutomirski
2015-09-02  9:17                     ` Stas Sergeev
2015-09-02 14:21                       ` Andy Lutomirski
2015-09-02 15:02                         ` Andy Lutomirski
2015-09-02 17:46                         ` Stas Sergeev
2015-09-02 18:17                           ` Andy Lutomirski
2015-09-02 18:23                             ` Stas Sergeev
2015-09-02 19:06                               ` Andy Lutomirski
2015-09-02 21:01                                 ` Stas Sergeev
2015-09-02 21:39                                   ` Andy Lutomirski [this message]
2015-09-02 22:25                                     ` Stas Sergeev
2015-09-02 22:25                                       ` Andy Lutomirski
2015-09-02 23:01                                         ` Stas Sergeev
2015-08-19 10:10           ` Stas Sergeev
2015-08-19 15:35             ` Andy Lutomirski
2015-08-14  8:10     ` Cyrill Gorcunov
2015-08-13 17:51   ` Stas Sergeev
2015-08-13 18:35     ` Linus Torvalds
2015-08-13 18:41       ` Andy Lutomirski
2015-08-13 19:05         ` Stas Sergeev
2015-08-13 19:49           ` Andy Lutomirski
2015-08-13 20:09             ` Stas Sergeev
2015-08-13 19:53         ` Linus Torvalds
2015-08-13 20:08           ` Cyrill Gorcunov
2015-08-13 20:09             ` Linus Torvalds
2015-08-13 21:42               ` Raymond Jennings
2015-08-13 21:46                 ` Linus Torvalds
2015-08-13 22:01                   ` Raymond Jennings
2015-08-13 22:05                     ` Stas Sergeev
2015-08-13 23:05                     ` Linus Torvalds
2015-08-13 23:18                       ` Linus Torvalds
2015-08-13 23:35                         ` Raymond Jennings
2015-08-13 23:43                         ` Stas Sergeev
2015-08-14  0:02                           ` Linus Torvalds
2015-08-13 22:02                   ` Stas Sergeev
2015-08-13 22:11                     ` Andy Lutomirski
2015-08-13 22:25                       ` Stas Sergeev
2015-08-13 22:29                         ` Andy Lutomirski
2015-08-13 22:51                           ` Stas Sergeev
2015-08-13 23:00                             ` Andy Lutomirski
2015-08-13 23:17                               ` Stas Sergeev
2015-08-14  0:00                               ` Stas Sergeev
2015-08-14  0:05                                 ` Andy Lutomirski
2015-08-14  0:17                                   ` Stas Sergeev
2015-08-14  0:27                                     ` Linus Torvalds
2015-08-14  0:50                                       ` Stas Sergeev
2015-08-14  1:21                                         ` Andy Lutomirski
2015-08-14  1:32                                           ` Stas Sergeev
2015-08-14  1:37                                             ` Andy Lutomirski
2015-08-14  2:03                                               ` Stas Sergeev
2015-08-18  6:19                                               ` Stas Sergeev
2015-08-14  0:08                                 ` Linus Torvalds
2015-08-14  0:24                                   ` Andy Lutomirski
2015-08-14  0:40                                     ` Linus Torvalds
2015-08-14  7:22               ` Cyrill Gorcunov
2015-08-14 10:02                 ` Pavel Emelyanov
2015-08-14 10:53                   ` Cyrill Gorcunov
2015-08-13 18:57       ` Stas Sergeev
2015-08-13 19:01         ` Andy Lutomirski
2015-08-13 19:13           ` Stas Sergeev
2015-08-13 19:37             ` Linus Torvalds
2015-08-13 19:59               ` Stas Sergeev
2015-08-13 20:07                 ` Linus Torvalds
2015-08-18  6:40                   ` Stas Sergeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALCETrVphMtmb+tW395Tj2LAa5DReh5b5j5oMjnMcp+GsMtH7g@mail.gmail.com \
    --to=luto@amacapital.net \
    --cc=gorcunov@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shentino@gmail.com \
    --cc=stsp@list.ru \
    --cc=torvalds@linux-foundation.org \
    --cc=xemul@parallels.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).