Message-ID: <CALCETrU2Kt5tk4EBA+ZnqXr327YWQ_+u-L40=oaTma3n0Coouw@mail.gmail.com>
Date:	Wed, 12 Aug 2015 11:25:35 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Stas Sergeev <stsp@...t.ru>
Cc:	X86 ML <x86@...nel.org>,
	Linux kernel <linux-kernel@...r.kernel.org>
Subject: Re: [regression] x86/signal/64: Fix SS handling for signals delivered
 to 64-bit programs breaks dosemu

On Wed, Aug 12, 2015 at 10:00 AM, Stas Sergeev <stsp@...t.ru> wrote:
> 12.08.2015 19:19, Andy Lutomirski пишет:
>>
>> Maybe I'm still missing something, but this seems like it should be
>> unnecessary.  What goes wrong without it?
>
> Without it, dosemu stores and fetches the ss value
> elsewhere. It could use any place for it, be it even a global var.
> But with your patch, dosemu _needs_ to use the sigcontext.ss,
> because that's where the kernel now puts it.
> As a result, dosemu had to be changed to use sigcontext.ss
> to load the ss from. Sounds good? Not! :)
> The reality is that you'll have to work with the old headers,
> that still have no sigcontext.ss, and so you'd need to access
> ss via __pad0 in pretty much 100% of real-life setups.
> If there is such a need to touch uapi from time to time
> (and I understand this is the case), then perhaps you should
> invent some versioning or whatever, to save people from
> surprises.
> Yes, you took the field that was used for padding.
> But this doesn't help, because this is not a new functionality.
> The existing programs now _need_ to use your new field
> for what they did in the past without it. So it is nearly the
> same as renaming any of the existing widely used fields:
> people will need the autoconf hacks to probe its existence.
>

Yeah, I see the problem here.

>>>
>>> https://github.com/stsp/dosemu2/commit/7898ac60d5e569964127d6cc48f592caecd20b81
>>
>> So the problem is that dosemu was actually hacking around the old
>> buggy behavior and thus relying on it.  Grr.
>
> What else it could do? :(

Going back in time?  Ask the kernel to fix the issue.  At this point,
it's a bit late for that.

>> It wouldn't even help here, because the breakage isn't caused by
>> incompatible sigcontext formats -- it's caused by dosemu's reliance on
>> ss being preserved across signal delivery
>
> I thought you mean some per-process flag that would
> preserve the backward-compatibility for the unaware apps.
> I probably got you wrong.

No, I meant a flag in sigcontext indicating which format was used.

>
>>   (even if it wasn't preserved
>> on the way back).
>
> How so?
> IIRC sometime fs/gs were restored, but I have no evidence
> the ss was. Could you clarify?

ss was never restored by sigreturn AFAIK.  I don't think that fs and
gs are, but I think they might have been a long long time ago (before
git).

>>>
>>> Good, but have you added any flag for dosemu to even know
>>> it can do this? Unless I am mistaken, you didn't. So the fix you
>>> suggest, is not easy to detect and make portable with the older
>>> kernels. Any suggestions?
>>>
>> You could probe for it directly: raise a signal, change the saved ss
>> and see what's in ss after sigreturn.
>
> Umm, nope.

Why not?  The safest general way to detect new features is to try to use them.

>
>> Let me see if I can come up with a clean kernel fix.
>
> The check for proper sigreturn would be good.

I still don't see how sigreturn matters here.  It's signal *delivery*
that's the problem.

I'm thinking of having signal delivery zap ss only if the old ss looks
bogus instead of zapping it unconditionally.  IOW, instead of setting
regs->ss = __USER_DS unconditionally, we'd do larl on the old regs->ss
and keep it if it's DPL 3 RW data (exp-down or otherwise) and present.

I'll have to check the precise rules in both the SDM and APM.  The
idea is that we don't want IRET to fail during signal delivery, which
can happen due to a bad sigreturn or a race against modify_ldt.

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

Powered by blists - more mailing lists