Re: [regression] x86/signal/64: Fix SS handling for signals delivered to 64-bit programs breaks dosemu

[Andy Lutomirski]

On Wed, Aug 19, 2015 at 2:35 AM, Stas Sergeev <stsp@xxxxxxx> wrote:
> 19.08.2015 01:47, Andy Lutomirski ÐÐÑÐÑ:
>> On Mon, Aug 17, 2015 at 11:19 PM, Stas Sergeev <stsp@xxxxxxx> wrote:
>>> 14.08.2015 04:37, Andy Lutomirski ÐÐÑÐÑ:
>>>
>>>> On Thu, Aug 13, 2015 at 6:32 PM, Stas Sergeev <stsp@xxxxxxx> wrote:
>>>>>
>>>>> 14.08.2015 04:21, Andy Lutomirski ÐÐÑÐÑ:
>>>>>
>>>>>> On Thu, Aug 13, 2015 at 5:50 PM, Stas Sergeev <stsp@xxxxxxx> wrote:
>>>>>>>
>>>>>>> 14.08.2015 03:27, Linus Torvalds ÐÐÑÐÑ:
>>>>>>>>
>>>>>>>> On Thu, Aug 13, 2015 at 5:17 PM, Stas Sergeev <stsp@xxxxxxx> wrote:
>>>>>>>>>
>>>>>>>>> For example because you can as well do:
>>>>>>>>> prctl(ARCH_SET_SIGNAL_SS, 0)
>>>>>>>>> which will mean "restore ss in sighandler to its current value",
>>>>>>>>
>>>>>>>> I really think a prctl() is the wrong thing to do.
>>>>>>>>
>>>>>>>> If you want a signal handler to save/restore segments, I think it
>>>>>>>> should be a SA_xyz flag to sigaction() (the way we have SA_RESTART
>>>>>>>
>>>>>>> Yes, I was proposing the new sigaction() flag in this thread
>>>>>>> already too. But at the end, prctl() looks better to me because
>>>>>>> it allows to pass the TLS value to use when restoring FS.
>>>>>>> The thing is that I am trying to find the similar treatment for
>>>>>>> both the SS and FS problems. If you don't think they need a
>>>>>>> similar treatment, then perhaps the Andy's patch is enough.
>>>>>>>
>>>>>>>> etc).  And off by default because of the obvious compatibility issues.
>>>>>>>
>>>>>>> Of course.
>>>>>>>
>>>>>>> So, what we have right now (in the latest Andy's patch) is:
>>>>>>> 1. lar heuristics
>>>>>>> 2. new uc_flags flag
>>>>>>>
>>>>>>> What it solves: dosemu's regression.
>>>>>>>
>>>>>>> What prctl() can give:
>>>>>>> - fix to dosemu's regression
>>>>>>> - fix to the TLS problem in the future
>>>>>>> - no hack and heuristics
>>>>>>>
>>>>>>> With SA_xyz you can only solve the SS problem, so it is
>>>>>>> probably not any better than the uc_flags things coded
>>>>>>> up by Andy.
>>>>>>
>>>>>> I'm leaning slightly toward LAR heuristic + SA_SAVE_SS.
>>>>>
>>>>> Stop right here, doesn't the SA_xyz allow to avoid the
>>>>> lar heuristic? Why would you still need the lar heuristic then?
>>>>> Just call it SA_RESTORE_SS instead of SA_SAVE_SS, and
>>>>> the lar heuristic is gone.
>>>>
>>>> The LAR heuristic is about five lines of code, and it makes signal
>>>> delivery more reliable.
>>>
>>> Why more reliable? In what case?
>>>
>>>>    Sure, we could gate the "regs->ss =
>>>> __USER_DS" line on a flag, but why?
>>>
>>> A few things I can think of why:
>>> - nested signals (usual for dosemu)
>>
>> What's the issue with nested signals?
> If nested signal is async and SS is from LDT just freed, then
> the nested signal will silently change the SS value. So if you
> are saving it somewhere, you'll save it by luck.
> Now this is unlikely to happen, as the async signals in dosemu
> are all blocked inside any sighandler. So the siglongjmp() case
> is more expressive: if dosemu jumped via siglongjmp() and as such
> unblocked the async signals, it will likely want to save its
> registers before doing a new switch. Now, since SS is invalid
> and will be therefore changed by a sighandler, what it will save
> depends on a luck.
> Not that dosemu uses siglongjmp() right now, but I am just asking
> to please not implement the unreliable interfaces _if possible_.

Ok, I think I get it.

I'm not proposing the LAR thing as something that anyone should
intentionally use going forward.  It would only be for compatbility
with old code if needed.  We certainly should save SS somewhere.

>> Incidentally, I tried implementing the sigaction flag approach.  I
>> think it's no good.  When we return from a signal, there's no concept
>> of sigaction -- it's just sigreturn.  Sigreturn can't look up the
>> sigaction flags -- what if the signal handler calls sigaction itself.
> How about the SA_hyz flag that does the following:
> - Saves SS into sigcontext
> - Forces SS to USER_DS on signal delivery
> - Sets the uc_flags flag for sigreturn() to take care of the rest.
> You'll have both the control on every bit of action, and a simple
> detection logic: if SA_hyz didn't set the uc flag - it didn't work.
> You can even employ your lar heuristic here for the case when the
> aforementioned SA_hyz is not set. But please, please not when it is
> set! In fact, I wonder if you had in mind exactly that: using the
> lar heuristic only if the SA_hyz is not set. If so - I misunderstood.
> Just please don't add it when it is set.

Hmm, interesting.  Maybe that would work for everything.  How's this
to make it concrete?

Add a sigaction flag SA_RESTORE_SS.

On signal delivery, always save SS into sigcontext->ss. if
SA_RESTORE_SS is set, then unconditionally switch HW SS to __USER_DS
and set UC_RESTORE_SS.  If SA_RESTORE_SS is clear, then leave HW SS
alone (i.e. preserve the old behavior).

On signal return, if UC_RESTORE_SS is set, then restore
sigcontext->ss.  If not, then set SS to __USER_DS (as old kernels
did).

This should change nothing at all (except the initial value of
sigcontext->ss / __pad0) on old kernels.

>
>> So we either need a per-task flag, a per-sighand flag, or a sigcontext
>> flag indicating what we should do.
>>
>> (Yes, I suspect we really might want some way to get FS, GS, and their
>> bases saved and restored, but I still think we should do that
>> separately.)
> In fact, I have already convinced myself that SA_hyz can take
> care of both cases. :) Maybe you'll just need to extend the struct sigaction
> to pass the TLS address, but this doesn't look absolutely impossible...

I think that should be a separate SA_ flag down the road, if for no
other reason than that the SS thing can be done more or less
immediately, whereas getting FS and GS right needs to wait.

Also, it occurs to me that FS and GS could become more complicated.
There are some proposals to allow tasks to opt in to having a per-cpu
GS.  If that happens, then figuring out how it would interact with
signals could be complicated.

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