Re: [PATCH] Wasm support patch 2 (static syscalls)

[Nicholas Wilson <nicholas.wilson@realvnc.com>]

Oh dear, confusion between setuid and getuid is embarrassing!

You're quite right, we don't really need to make setuid work well, any applications intend to use it for something useful won't be portable to Wasm without some modification.

The (much slimmer) patch that just turns off __syscall_cp for Wasm is attached below. Thanks for the feedback.

Nick


diff --git a/src/internal/syscall.h b/src/internal/syscall.h
index 6d378a81..bef9fae9 100644
--- a/src/internal/syscall.h
+++ b/src/internal/syscall.h
@@ -34,15 +34,17 @@ long __syscall_ret(unsigned long), __syscall(syscall_arg_t, ...),
 #define __syscall4(n,a,b,c,d) (__syscall)(n,__scc(a),__scc(b),__scc(c),__scc(d))
 #define __syscall5(n,a,b,c,d,e) (__syscall)(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e))
 #define __syscall6(n,a,b,c,d,e,f) (__syscall)(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e),__scc(f))
+#define __syscall7(n,a,b,c,d,e,f,g) (__syscall)(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e),__scc(f),__scc(g))
 #else
+#define __syscall0(n) __syscall0(n)
 #define __syscall1(n,a) __syscall1(n,__scc(a))
 #define __syscall2(n,a,b) __syscall2(n,__scc(a),__scc(b))
 #define __syscall3(n,a,b,c) __syscall3(n,__scc(a),__scc(b),__scc(c))
 #define __syscall4(n,a,b,c,d) __syscall4(n,__scc(a),__scc(b),__scc(c),__scc(d))
 #define __syscall5(n,a,b,c,d,e) __syscall5(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e))
 #define __syscall6(n,a,b,c,d,e,f) __syscall6(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e),__scc(f))
+#define __syscall7(n,a,b,c,d,e,f,g) __syscall7(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e),__scc(f),__scc(g))
 #endif
-#define __syscall7(n,a,b,c,d,e,f,g) (__syscall)(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e),__scc(f),__scc(g))

 #define __SYSCALL_NARGS_X(a,b,c,d,e,f,g,h,n,...) n
 #define __SYSCALL_NARGS(...) __SYSCALL_NARGS_X(__VA_ARGS__,7,6,5,4,3,2,1,0,)
@@ -56,6 +58,18 @@ long __syscall_ret(unsigned long), __syscall(syscall_arg_t, ...),
 #define socketcall __socketcall
 #define socketcall_cp __socketcall_cp

+#ifdef SYSCALL_STATIC
+// For archs that define SYSCALL_STATIC (wasm), we basically just don't allow
+// for pthread_cancel().  I don't expect wasm will ever allow for cancellable
+// waits so that's OK.
+#define __syscall_cp0(n) __syscall0(n)
+#define __syscall_cp1(n,a) __syscall1(n,a)
+#define __syscall_cp2(n,a,b) __syscall2(n,a,b)
+#define __syscall_cp3(n,a,b,c) __syscall3(n,a,b,c)
+#define __syscall_cp4(n,a,b,c,d) __syscall4(n,a,b,c,d)
+#define __syscall_cp5(n,a,b,c,d,e) __syscall5(n,a,b,c,d,e)
+#define __syscall_cp6(n,a,b,c,d,e,f) __syscall6(n,a,b,c,d,e,f)
+#else
 #define __syscall_cp0(n) (__syscall_cp)(n,0,0,0,0,0,0)
 #define __syscall_cp1(n,a) (__syscall_cp)(n,__scc(a),0,0,0,0,0)
 #define __syscall_cp2(n,a,b) (__syscall_cp)(n,__scc(a),__scc(b),0,0,0,0)
@@ -63,6 +77,7 @@ long __syscall_ret(unsigned long), __syscall(syscall_arg_t, ...),
 #define __syscall_cp4(n,a,b,c,d) (__syscall_cp)(n,__scc(a),__scc(b),__scc(c),__scc(d),0,0)
 #define __syscall_cp5(n,a,b,c,d,e) (__syscall_cp)(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e),0)
 #define __syscall_cp6(n,a,b,c,d,e,f) (__syscall_cp)(n,__scc(a),__scc(b),__scc(c),__scc(d),__scc(e),__scc(f))
+#endif

 #define __syscall_cp(...) __SYSCALL_DISP(__syscall_cp,__VA_ARGS__)
 #define syscall_cp(...) __syscall_ret(__syscall_cp(__VA_ARGS__))
diff --git a/src/thread/pthread_cancel.c b/src/thread/pthread_cancel.c
index 3d229223..d14d96ed 100644
--- a/src/thread/pthread_cancel.c
+++ b/src/thread/pthread_cancel.c
@@ -20,6 +20,7 @@ long __syscall_cp_asm(volatile void *, syscall_arg_t,
                       syscall_arg_t, syscall_arg_t, syscall_arg_t,
                       syscall_arg_t, syscall_arg_t, syscall_arg_t);

+#ifndef SYSCALL_STATIC
 long __syscall_cp_c(syscall_arg_t nr,
                     syscall_arg_t u, syscall_arg_t v, syscall_arg_t w,
                     syscall_arg_t x, syscall_arg_t y, syscall_arg_t z)
@@ -38,6 +39,7 @@ long __syscall_cp_c(syscall_arg_t nr,
                r = __cancel();
        return r;
 }
+#endif

 static void _sigaddset(sigset_t *set, int sig)
 {

________________________________________
From: Rich Felker <dalias@aerifal.cx> on behalf of Rich Felker <dalias@libc.org>
Sent: 28 November 2017 15:50:14
To: musl@lists.openwall.com
Subject: Re: [musl] [PATCH] Wasm support patch 2 (static syscalls)

On Tue, Nov 28, 2017 at 04:08:06PM +0100, Szabolcs Nagy wrote:
> * Nicholas Wilson <nicholas.wilson@realvnc.com> [2017-11-28 14:53:58 +0000]:
> > I think LTO would be required - but I might be missing something
> > in your scheme? I'm considering the cases like __setxid and
> > pthread_cancel, where the compiler can't work out which syscall
> > numbers are possible (because the numbers come from another
> > translation unit). In these cases, there's no way to eliminate any
> > of the branches in your switch statement.
> >
> > In particular, getuid() is such a common library call that I think
> > we do want to be able to support it on Wasm, without having Musl
> > link in everything. I mean, it's not an obscure piece of
> > functionality, so it's worth a small refactor to make it usable
> > with Wasm.
>
> getuid is not affected, only __setxid and cancellable syscalls are.
>
> __setxid is not that important and can be worked around in generic code,
> but i don't see an easy solution for syscall_cp except that wasm
> probably don't need cancellation at all so it can be just defined
> to syscall.

I don't think calling __setxid even makes sense in wasm code. Do you
actually have a multiuser model with a privileged user who can change
uid to other users?

Cancellation is actually hard to do via an approach like this, since
you need some mechanism for determining the point at which the syscall
has atomically succeeded versus being blocked with no side effects
yet. Implementing it would probably require doing something similar to
what midipix does, but for now it can probably just be omitted.

Rich