[LibOS] Rework checking of supported clone() flags

LibOS/shim/src/sys/shim_clone.c

@@ -202,16 +202,48 @@ int shim_do_clone (int flags, void * user_stack_addr, int * parent_tidptr,
         return shim_do_vfork();
     }
 
-    assert((flags & ~(CLONE_PARENT_SETTID|CLONE_CHILD_SETTID|
-                      CLONE_CHILD_CLEARTID|CLONE_SETTLS|
-                      CLONE_VM|CLONE_FILES|
-                      CLONE_FS|CLONE_SIGHAND|CLONE_THREAD|
-                      CLONE_DETACHED| // Unused
-#ifdef CLONE_PTRACE
-                      CLONE_PTRACE| // Unused
+    const int supported_flags =
+        CLONE_CHILD_CLEARTID |
+        CLONE_CHILD_SETTID |
+        CLONE_DETACHED | // Unused
+        CLONE_FILES |
+        CLONE_FS |
+        CLONE_PARENT_SETTID |
+#ifdef CLONE_PIDFD
+        CLONE_PIDFD |
 #endif
-                      CLONE_SYSVSEM|CSIGNAL)) == 0);
+        CLONE_PTRACE | // Unused
+        CLONE_SETTLS |
+        CLONE_SIGHAND |
+        CLONE_SYSVSEM |
+        CLONE_THREAD |
+        CLONE_VM |
+        CSIGNAL;
+
+    const int unsupported_flags = ~supported_flags;
+
+    if (flags & unsupported_flags) {
+        debug("clone called with unsupported flags argument.\n");
+        return -EINVAL;
+    }
+
+    /* Explicitly disallow CLONE_VM without either of CLONE_THREAD or CLONE_VFORK on Graphene. While
+     * Linux allows passing CLONE_VM without either of CLONE_THREAD or CLONE_VFORK, this usage is
+     * exotic enough to not attempt a faithful emulation in Graphene. */
+    if (flags & CLONE_VM)
+        if (!((flags & CLONE_THREAD) || (flags & CLONE_VFORK))) {
+            debug("CLONE_VM without either CLONE_THREAD or CLONE_VFORK is unsupported\n");
+            return -EINVAL;
+        }
+
+    if ((flags & CLONE_THREAD) && !(flags & CLONE_SIGHAND))
+        return -EINVAL;
+    if ((flags & CLONE_SIGHAND) && !(flags & CLONE_VM))
+        return -EINVAL;
 
+    /* The caller may not have set the following three flags, but Graphene treats them as set to
+     * simplify the implementation of clone. Only print a warning since returning an explicit error
+     * code breaks many applications. */
     if (!(flags & CLONE_FS))
         debug("clone without CLONE_FS is not yet implemented\n");
 
@@ -221,38 +253,6 @@ int shim_do_clone (int flags, void * user_stack_addr, int * parent_tidptr,
     if (!(flags & CLONE_SYSVSEM))
         debug("clone without CLONE_SYSVSEM is not yet implemented\n");
 
-    /* currently unsupported flags.
-     * Please update this once you added new flags support.
-     */
-    const int unsupported_flags =
-#ifdef CLONE_PIDFD
-        CLONE_PIDFD |
-#endif
-        CLONE_VFORK | /* vfork is handled above */
-        CLONE_PARENT |
-        CLONE_NEWNS |
-        CLONE_UNTRACED |
-        CLONE_NEWCGROUP |
-        CLONE_NEWUTS |
-        CLONE_NEWIPC |
-        CLONE_NEWUSER |
-        CLONE_NEWPID |
-        CLONE_NEWNET |
-        CLONE_IO;
-    if (flags & unsupported_flags)
-        debug("clone with flags 0x%x is not yet implemented\n",
-            flags & unsupported_flags);
-
-    if ((flags & (CLONE_NEWNS|CLONE_FS)) == (CLONE_NEWNS|CLONE_FS))
-        return -EINVAL;
-    if ((flags & (CLONE_NEWUSER|CLONE_FS)) == (CLONE_NEWUSER|CLONE_FS))
-        return -EINVAL;
-    if ((flags & CLONE_THREAD) && !(flags & CLONE_SIGHAND))
-        return -EINVAL;
-    if ((flags & CLONE_SIGHAND) && !(flags & CLONE_VM))
-        return -EINVAL;
-    if (flags & CLONE_THREAD && (flags & (CLONE_NEWUSER | CLONE_NEWPID)))
-        return -EINVAL;
 #ifdef CLONE_PIDFD
     if (flags & CLONE_PIDFD) {
         if (flags & (CLONE_DETACHED | CLONE_PARENT_SETTID | CLONE_THREAD))

LibOS/shim/test/regression/futex.c

@@ -59,7 +59,7 @@ int main(int argc, const char** argv) {
 
         // Call the clone system call to create the child thread
         pids[i] = clone(&thread_function, (void*)stacks[i] + FIBER_STACK,
-                        CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_VM, &varx[i]);
+                        CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_VM | CLONE_THREAD, &varx[i]);
 
         // printf("clone() creates new thread %d\n", pids[i]);