commit 38f01e05ef7f7ef18a43339436fba645d123b838
author Elliott Hughes <enh@google.com> Fri Oct 27 15:28:54 2017 -0700
committer Elliott Hughes <enh@google.com> Fri Oct 27 15:40:15 2017 -0700
tree e6dbb0977a34d052056e935d86bd3cdcc9473188
parent dfece7a3ee1b8a11abea3f55bf44df21ebc4bc79

Preserve historical pthread_create scheduler behavior better.

At the cost of two flag bits for what POSIX thinks should be a boolean
choice, plus somewhat confusing behavior from pthread_attr_getinheritsched
depending on when you call it/what specific scheduler attributes you've
set in the pthread_attr_t, we can emulate the old behavior exactly and
prevent annoying SELinux denial spam caused by calls to sched_setscheduler.

Bug: http://b/68391226
Test: adb logcat on boot contains no sys_nice avc denials
Change-Id: I4f759c2c4fd1d80cceb0912d7da09d35902e2e5e

libc/bionic/pthread_attr.cpp

diff --git a/libc/bionic/pthread_attr.cpp b/libc/bionic/pthread_attr.cpp
index 04d8112..93177f1 100644
--- a/libc/bionic/pthread_attr.cpp
+++ b/libc/bionic/pthread_attr.cpp
@@ -61,8 +61,10 @@
 int pthread_attr_setinheritsched(pthread_attr_t* attr, int flag) {
   if (flag == PTHREAD_EXPLICIT_SCHED) {
     attr->flags &= ~PTHREAD_ATTR_FLAG_INHERIT;
+    attr->flags |= PTHREAD_ATTR_FLAG_EXPLICIT;
   } else if (flag == PTHREAD_INHERIT_SCHED) {
     attr->flags |= PTHREAD_ATTR_FLAG_INHERIT;
+    attr->flags &= ~PTHREAD_ATTR_FLAG_EXPLICIT;
   } else {
     return EINVAL;
   }
@@ -71,8 +73,14 @@
 
 __BIONIC_WEAK_FOR_NATIVE_BRIDGE
 int pthread_attr_getinheritsched(const pthread_attr_t* attr, int* flag) {
-  *flag = (attr->flags & PTHREAD_ATTR_FLAG_INHERIT) ? PTHREAD_INHERIT_SCHED
-                                                    : PTHREAD_EXPLICIT_SCHED;
+  if ((attr->flags & PTHREAD_ATTR_FLAG_INHERIT) != 0) {
+    *flag = PTHREAD_INHERIT_SCHED;
+  } else if ((attr->flags & PTHREAD_ATTR_FLAG_EXPLICIT) != 0) {
+    *flag = PTHREAD_EXPLICIT_SCHED;
+  } else {
+    // Historical behavior before P, when pthread_attr_setinheritsched was added.
+    *flag = (attr->sched_policy != SCHED_NORMAL) ? PTHREAD_EXPLICIT_SCHED : PTHREAD_INHERIT_SCHED;
+  }
   return 0;
 }
 

libc/bionic/pthread_create.cpp

diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp
index 40db785..21a707b 100644
--- a/libc/bionic/pthread_create.cpp
+++ b/libc/bionic/pthread_create.cpp
@@ -122,7 +122,7 @@
   bool need_set = true;
   int policy;
   sched_param param;
-  if (thread->attr.flags & PTHREAD_ATTR_FLAG_INHERIT) {
+  if ((thread->attr.flags & PTHREAD_ATTR_FLAG_INHERIT) != 0) {
     // Unless the parent has SCHED_RESET_ON_FORK set, we've already inherited from the parent.
     policy = sched_getscheduler(0);
     need_set = ((policy & SCHED_RESET_ON_FORK) != 0);
@@ -142,6 +142,11 @@
     policy = thread->attr.sched_policy;
     param.sched_priority = thread->attr.sched_priority;
   }
+  // Backwards compatibility: before P, Android didn't have pthread_attr_setinheritsched,
+  // and our behavior was neither of the POSIX behaviors.
+  if ((thread->attr.flags & (PTHREAD_ATTR_FLAG_INHERIT|PTHREAD_ATTR_FLAG_EXPLICIT)) == 0) {
+    need_set = (thread->attr.sched_policy != SCHED_NORMAL);
+  }
   if (need_set) {
     if (sched_setscheduler(thread->tid, policy, &param) == -1) {
       async_safe_format_log(ANDROID_LOG_WARN, "libc",

libc/bionic/pthread_internal.h

diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h
index 2dd0cff..a4cba87 100644
--- a/libc/bionic/pthread_internal.h
+++ b/libc/bionic/pthread_internal.h
@@ -40,8 +40,10 @@
 // Has the thread been joined by another thread?
 #define PTHREAD_ATTR_FLAG_JOINED 0x00000002
 
-// Should we inherit scheduling attributes from the parent on pthread_create?
+// Used for pthread_attr_setinheritsched. We need two flags for this apparent
+// boolean because our historical behavior matches neither of the POSIX choices.
 #define PTHREAD_ATTR_FLAG_INHERIT 0x00000004
+#define PTHREAD_ATTR_FLAG_EXPLICIT 0x00000008
 
 class pthread_key_data_t {
  public: