Re: Problems that emerged when trying to port dosemu2

[Rich Felker <dalias@libc.org>]

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...

Rich