Bug 1376653 - Loosen restrictions on clone flags for musl. r?gcp draft
authorJed Davis <jld@mozilla.com>
Tue, 11 Jul 2017 14:23:27 -0600
changeset 610045 cd75b8af5774789c18aa36c97d774d8ed4caaed1
parent 609865 e0b0865639cebc1b5afa0268a4b073fcdde0e69c
child 610046 611b3ee0879063f1885845a575e02fdcc796d9af
push id68770
push userbmo:jld@mozilla.com
push dateMon, 17 Jul 2017 20:57:18 +0000
reviewersgcp
bugs1376653
milestone56.0a1
Bug 1376653 - Loosen restrictions on clone flags for musl. r?gcp I've made this non-ifdef'ed, and removed currently unused ifdef'ed cases for old Android versions, because I'd rather have less code that we're not even compile-testing than save a few cycles on a non-critical path. MozReview-Commit-ID: B4Wn1elyK4f
security/sandbox/linux/SandboxFilter.cpp
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -120,35 +120,29 @@ public:
   virtual ResultExpr ClonePolicy(ResultExpr failPolicy) const {
     // Allow use for simple thread creation (pthread_create) only.
 
     // WARNING: s390 and cris pass the flags in the second arg -- see
     // CLONE_BACKWARDS2 in arch/Kconfig in the kernel source -- but we
     // don't support seccomp-bpf on those archs yet.
     Arg<int> flags(0);
 
-    // The glibc source hasn't changed the thread creation clone flags
-    // since 2004, so this *should* be safe to hard-code.  Bionic's
-    // value has changed a few times, and has converged on the same one
-    // as glibc; allow any of them.
-    static const int flags_common = CLONE_VM | CLONE_FS | CLONE_FILES |
-      CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM;
-    static const int flags_modern = flags_common | CLONE_SETTLS |
+    // The exact flags used can vary.  CLONE_DETACHED is used by musl
+    // and by old versions of Android (<= JB 4.2), but it's been
+    // ignored by the kernel since the beginning of the Git history.
+    //
+    // If we ever need to support Android <= KK 4.4 again, SETTLS
+    // and the *TID flags will need to be made optional.
+    static const int flags_required = CLONE_VM | CLONE_FS | CLONE_FILES |
+      CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM | CLONE_SETTLS |
       CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID;
+    static const int flags_optional = CLONE_DETACHED;
 
-    // Can't use CASES here because its decltype magic infers const
-    // int instead of regular int and bizarre voluminous errors issue
-    // forth from the depths of the standard library implementation.
-    return Switch(flags)
-#ifdef ANDROID
-      .Case(flags_common | CLONE_DETACHED, Allow()) // <= JB 4.2
-      .Case(flags_common, Allow()) // JB 4.3 or KK 4.4
-#endif
-      .Case(flags_modern, Allow()) // Android L or glibc
-      .Default(failPolicy);
+    return If((flags & ~flags_optional) == flags_required, Allow())
+      .Else(failPolicy);
   }
 
   virtual ResultExpr PrctlPolicy() const {
     // Note: this will probably need PR_SET_VMA if/when it's used on
     // Android without being overridden by an allow-all policy, and
     // the constant will need to be defined locally.
     Arg<int> op(0);
     return Switch(op)