Change _stdio_handles_locking into _caller_handles_locking.

It is reported by tsan that funlockfile() can unlock an unlocked mutex.
It happens when printf() is called before fopen() or other stdio stuff.
As FLOCKFILE(fp) is called before __sinit(), _stdio_handles_locking is false,
and _FLOCK(fp) will not be locked. But then cantwrite(fp) in __vfprintf()
calls__sinit(), which makes _stdio_handles_locking become true, and
FUNLOCKFILE(fp) unlocks _FLOCK(fp).

Change _stdio_handles_locking into _caller_handles_locking,
so __sinit() won't change its value. Add test due to my previous fault.

Bug: 25392375
Change-Id: I483e3c3cdb28da65e62f1fd9615bf58c5403b4dd

libc/bionic/flockfile.cpp

@@ -40,7 +40,7 @@ void flockfile(FILE* fp) {
     __sinit();
   }
 
-  if (fp != NULL) {
+  if (fp != nullptr) {
     pthread_mutex_lock(&_FLOCK(fp));
   }
 }
@@ -52,7 +52,7 @@ int ftrylockfile(FILE* fp) {
 
   // The specification for ftrylockfile() says it returns 0 on success,
   // or non-zero on error. So return an errno code directly on error.
-  if (fp == NULL) {
+  if (fp == nullptr) {
     return EINVAL;
   }
 
@@ -64,7 +64,7 @@ void funlockfile(FILE* fp) {
     __sinit();
   }
 
-  if (fp != NULL) {
+  if (fp != nullptr) {
     pthread_mutex_unlock(&_FLOCK(fp));
   }
 }

libc/stdio/local.h

@@ -109,7 +109,7 @@ struct __sfileext {
   pthread_mutex_t _lock;
 
   /* __fsetlocking support */
-  bool _stdio_handles_locking;
+  bool _caller_handles_locking;
 };
 
 #if defined(__cplusplus)
@@ -131,7 +131,7 @@ do { \
 	pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); \
 	pthread_mutex_init(&_FLOCK(fp), &attr); \
 	pthread_mutexattr_destroy(&attr); \
-	_EXT(fp)->_stdio_handles_locking = true; \
+	_EXT(fp)->_caller_handles_locking = false; \
 } while (0)
 
 #define _FILEEXT_SETUP(f, fext) \
@@ -208,8 +208,8 @@ extern void __atexit_register_cleanup(void (*)(void));
 	(fp)->_lb._base = NULL; \
 }
 
-#define FLOCKFILE(fp)   if (_EXT(fp)->_stdio_handles_locking) flockfile(fp)
+#define FLOCKFILE(fp)   if (!_EXT(fp)->_caller_handles_locking) flockfile(fp)
-#define FUNLOCKFILE(fp) if (_EXT(fp)->_stdio_handles_locking) funlockfile(fp)
+#define FUNLOCKFILE(fp) if (!_EXT(fp)->_caller_handles_locking) funlockfile(fp)
 
 #define FLOATING_POINT
 #define PRINTF_WIDE_CHAR

libc/stdio/stdio_ext.cpp

@@ -74,7 +74,7 @@ void _flushlbf() {
 }
 
 int __fsetlocking(FILE* fp, int type) {
-  int old_state = _EXT(fp)->_stdio_handles_locking ? FSETLOCKING_INTERNAL : FSETLOCKING_BYCALLER;
+  int old_state = _EXT(fp)->_caller_handles_locking ? FSETLOCKING_BYCALLER : FSETLOCKING_INTERNAL;
   if (type == FSETLOCKING_QUERY) {
     return old_state;
   }
@@ -84,7 +84,7 @@ int __fsetlocking(FILE* fp, int type) {
     __libc_fatal("Bad type (%d) passed to __fsetlocking", type);
   }
 
-  _EXT(fp)->_stdio_handles_locking = (type == FSETLOCKING_INTERNAL);
+  _EXT(fp)->_caller_handles_locking = (type == FSETLOCKING_BYCALLER);
   return old_state;
 }
 

tests/pthread_test.cpp

@@ -30,12 +30,8 @@
 #include <unwind.h>
 
 #include <atomic>
-#include <regex>
 #include <vector>
 
-#include <base/file.h>
-#include <base/stringprintf.h>
-
 #include "private/bionic_macros.h"
 #include "private/ScopeGuard.h"
 #include "BionicDeathTest.h"
@@ -43,8 +39,6 @@
 
 #include "utils.h"
 
-extern "C" pid_t gettid();
-
 TEST(pthread, pthread_key_create) {
   pthread_key_t key;
   ASSERT_EQ(0, pthread_key_create(&key, NULL));
@@ -721,23 +715,6 @@ TEST(pthread, pthread_rwlock_smoke) {
   ASSERT_EQ(0, pthread_rwlock_destroy(&l));
 }
 
-static void WaitUntilThreadSleep(std::atomic<pid_t>& tid) {
-  while (tid == 0) {
-    usleep(1000);
-  }
-  std::string filename = android::base::StringPrintf("/proc/%d/stat", tid.load());
-  std::regex regex {R"(\s+S\s+)"};
-
-  while (true) {
-    std::string content;
-    ASSERT_TRUE(android::base::ReadFileToString(filename, &content));
-    if (std::regex_search(content, regex)) {
-      break;
-    }
-    usleep(1000);
-  }
-}
-
 struct RwlockWakeupHelperArg {
   pthread_rwlock_t lock;
   enum Progress {

tests/stdio_ext_test.cpp

@@ -30,6 +30,7 @@
 #include <locale.h>
 
 #include "TemporaryFile.h"
+#include "utils.h"
 
 TEST(stdio_ext, __fbufsize) {
   FILE* fp = fopen("/proc/version", "r");
@@ -140,3 +141,24 @@ TEST(stdio_ext, __fsetlocking) {
   ASSERT_EQ(FSETLOCKING_INTERNAL, __fsetlocking(fp, FSETLOCKING_QUERY));
   fclose(fp);
 }
+
+static void LockingByCallerHelper(std::atomic<pid_t>* pid) {
+  *pid = gettid();
+  flockfile(stdout);
+  funlockfile(stdout);
+}
+
+TEST(stdio_ext, __fsetlocking_BYCALLER) {
+  // Check if users can use flockfile/funlockfile to protect stdio operations.
+  int old_state = __fsetlocking(stdout, FSETLOCKING_BYCALLER);
+  flockfile(stdout);
+  pthread_t thread;
+  std::atomic<pid_t> pid(0);
+  ASSERT_EQ(0, pthread_create(&thread, nullptr,
+                              reinterpret_cast<void* (*)(void*)>(LockingByCallerHelper), &pid));
+  WaitUntilThreadSleep(pid);
+  funlockfile(stdout);
+
+  ASSERT_EQ(0, pthread_join(thread, nullptr));
+  __fsetlocking(stdout, old_state);
+}

tests/utils.h

@@ -18,6 +18,14 @@
 #define __TEST_UTILS_H
 #include <inttypes.h>
 #include <sys/mman.h>
+#include <unistd.h>
+
+#include <atomic>
+#include <string>
+#include <regex>
+
+#include <base/file.h>
+#include <base/stringprintf.h>
 
 #include "private/ScopeGuard.h"
 
@@ -82,4 +90,23 @@ class Maps {
   }
 };
 
+extern "C" pid_t gettid();
+
+static inline void WaitUntilThreadSleep(std::atomic<pid_t>& tid) {
+  while (tid == 0) {
+    usleep(1000);
+  }
+  std::string filename = android::base::StringPrintf("/proc/%d/stat", tid.load());
+  std::regex regex {R"(\s+S\s+)"};
+
+  while (true) {
+    std::string content;
+    ASSERT_TRUE(android::base::ReadFileToString(filename, &content));
+    if (std::regex_search(content, regex)) {
+      break;
+    }
+    usleep(1000);
+  }
+}
+
 #endif