Re: Problems that emerged when trying to port dosemu2

[bluemoon <blaumolch@mailbox.org>]

Am 03.12.2017 um 23:01 schrieb Rich Felker:
> On Sun, Dec 03, 2017 at 03:49:20PM +0100, Szabolcs Nagy wrote:
>> * bluemoon <blaumolch@mailbox.org> [2017-12-03 11:50:34 +0100]:
>>> My knowledge of the matter is too limited to explain it in my own words, but
>>> he summarized what’s going on here (patches are below):
>>> https://github.com/stsp/dosemu2/issues/537#issuecomment-346177776
>>>
>>>> The checks that you remove, are nonsense:
>>>> they check for "ss_size" and return ENOMEM
>>>> even for SS_DISABLE. They check for ~SS_DISABLE
>>>> and return error for SS_AUTODISARM, even
>>>> though it is defined in their headers. Overall
>>>> they try to check the syscall parameters -
>>>> something they should never do simply because
>>>> libc does not understand the syscall parameters.
>>>> It should just call the syscall - not more, not less.
>>>> syscall understands its parameters, so it will
>>>> check them correctly and return error as appropriate.
>>>> Check from musl should be removed, and I think
>>>> it would be good to try to submit that change.
>>>>
>>>> Stack-protector problem is a kernel mis-feature,
>>>> and a very unfortunate one. We should pester
>>>> Andy Lutomirski (@amluto) to finally fix it. :)
>>>> I don't know if musl can accept this patch, maybe
>>>> it can if the attribute is put under #ifdef __GNUC__
>>>> check.
>>>
>>> To make it work the following two patches were applied:
>>>
>>> --- src/misc/syscall.c.orig     2017-10-31 20:13:58.000000000 +0100
>>> +++ src/misc/syscall.c  2017-11-21 18:36:38.912082672 +0100
>>> @@ -3,7 +3,7 @@
>>>
>>>  #undef syscall
>>>
>>> -long syscall(long n, ...)
>>> +__attribute__((optimize("no-stack-protector"))) long syscall(long n, ...)
>>>  {
>>
>> changing fs/gs behind the back of the c runtime is not
>> guaranteed to work, but it makes sense to me to compile
>> syscall.c without ssp instrumentation to allow certain hacks.
>> (but i think this should be done in the makefile)
> 
> I'm not clear why this would even work on glibc, since %gs is used to
> access the vdso syscall pointer. I don't think it makes sense to treat
> syscall() specially here. If the thread pointer register is not valid,
> then the ABI is not being satisfied and that's all there is to say.
> Programs that have changed the thread pointer in a thread must refrain
> from doing anything that could cause a call into libc. If they need to
> make syscalls, they can do it via [inline] asm; they're already using
> target-specific asm anyway if they're changing %fs/%gs.
> 
>>>         va_list ap;
>>>         syscall_arg_t a,b,c,d,e,f;
>>>
>>> --- src/signal/sigaltstack.c.orig       2017-10-31 20:13:58.000000000 +0100
>>> +++ src/signal/sigaltstack.c    2017-11-21 20:56:59.740814704 +0100
>>> @@ -4,15 +4,5 @@
>>>
>>>  int sigaltstack(const stack_t *restrict ss, stack_t *restrict old)
>>>  {
>>> -       if (ss) {
>>> -               if (ss->ss_size < MINSIGSTKSZ) {
>>> -                       errno = ENOMEM;
>>> -                       return -1;
>>> -               }
>>
>> i think this part has to be kept for conformance reasons:
>> the kernel does not check MINSIGSTKSZ (it does not even
>> know how it is defined in musl, so it is musl abi, not
>> kernel abi), but posix requires the check.
> 
> Indeed, but POSIX says:
> 
>     "If it is set to SS_DISABLE, the stack is disabled and ss_sp and
>     ss_size are ignored."
> 
> so it's a bug to be checking ss_size when ss_flags == SS_DISABLE. We
> should only check ss_size if !(ss_flags & SS_DISABLE) or similar.
> 
>>> -               if (ss->ss_flags & ~SS_DISABLE) {
>>> -                       errno = EINVAL;
>>> -                       return -1;
>>> -               }
>>
>> this is another conformance check, but one can argue
>> that linux extensions should be allowed here.
>> (it's unfortunate that some useful linux extensions
>> are in conflict with posix requirements..)
> 
> Yes, this one is a requirement and I don't see any way around it...

Thank you for your replies!

The developer of dosemu2 was able to implement some work-arounds so that
now it’s possible to run protected mode programs.

However, a remaining question is that if linux extensions aren’t allowed
why is

#define SS_AUTODISARM (1U << 31)
#define SS_FLAG_BITS SS_AUTODISARM

defined in signal.h? This confuses the detections.