Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 3 Dec 2017 15:49:20 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: Problems that emerged when trying to port dosemu2

* bluemoon <blaumolch@...lbox.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)

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

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

> -       }
>         return syscall(SYS_sigaltstack, ss, old);
>  }

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.