commit db8a266aea6c12b0fb8ee3587d72333662b05266
author Mark Salyzyn <salyzyn@google.com> Mon Oct 10 07:27:42 2016 -0700
committer Mark Salyzyn <salyzyn@google.com> Thu Oct 13 21:43:42 2016 +0000
tree c10491cebcf0b69c99aa0e8ed56611a50d0080b4
parent bf0cf4e22d594db03c1dcef57fd18d3ae038eefb

liblog: __android_log_pmsg_file_write() cleanup

__android_log_pmsg_file_write() will open /dev/pmsg0 if not
already, and will close it if we opened it.

Added atomic access to the android_log_context as insurance.

Fortify and correct pmsg tests.

Test: gTest liblog-unit-tests --gtest_filter=liblog.__android_log_pmsg_file_*
Bug: 31958686
Change-Id: I2cf6f971b6968938f471fda67367efe20dae3004

liblog/logd_writer.c

diff --git a/liblog/logd_writer.c b/liblog/logd_writer.c
index e8e392d..2913507 100644
--- a/liblog/logd_writer.c
+++ b/liblog/logd_writer.c
@@ -31,7 +31,6 @@
 #include <time.h>
 #include <unistd.h>
 
-#include <android/log.h>
 #include <cutils/sockets.h>
 #include <log/logger.h>
 #include <private/android_filesystem_config.h>
@@ -65,7 +64,8 @@
 {
     int i, ret = 0;
 
-    if (logdLoggerWrite.context.sock < 0) {
+    i = atomic_load(&logdLoggerWrite.context.sock);
+    if (i < 0) {
         i = TEMP_FAILURE_RETRY(socket(PF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0));
         if (i < 0) {
             ret = -errno;
@@ -80,7 +80,11 @@
                 ret = -errno;
                 close(i);
             } else {
-                logdLoggerWrite.context.sock = i;
+                ret = atomic_exchange(&logdLoggerWrite.context.sock, i);
+                if ((ret >= 0) && (ret != i)) {
+                    close(ret);
+                }
+                ret = 0;
             }
         }
     }
@@ -90,9 +94,9 @@
 
 static void logdClose()
 {
-    if (logdLoggerWrite.context.sock >= 0) {
-        close(logdLoggerWrite.context.sock);
-        logdLoggerWrite.context.sock = -1;
+    int sock = atomic_exchange(&logdLoggerWrite.context.sock, -1);
+    if (sock >= 0) {
+        close(sock);
     }
 }
 
@@ -101,7 +105,7 @@
     if (logId > LOG_ID_SECURITY) {
         return -EINVAL;
     }
-    if (logdLoggerWrite.context.sock < 0) {
+    if (atomic_load(&logdLoggerWrite.context.sock) < 0) {
         if (access("/dev/socket/logdw", W_OK) == 0) {
             return 0;
         }
@@ -121,7 +125,7 @@
     static atomic_int_fast32_t dropped;
     static atomic_int_fast32_t droppedSecurity;
 
-    if (logdLoggerWrite.context.sock < 0) {
+    if (atomic_load(&logdLoggerWrite.context.sock) < 0) {
         return -EBADF;
     }
 
@@ -160,7 +164,7 @@
     newVec[0].iov_base = (unsigned char *)&header;
     newVec[0].iov_len  = sizeof(header);
 
-    if (logdLoggerWrite.context.sock > 0) {
+    if (atomic_load(&logdLoggerWrite.context.sock) > 0) {
         int32_t snapshot = atomic_exchange_explicit(&droppedSecurity, 0,
                                                     memory_order_relaxed);
         if (snapshot) {
@@ -174,7 +178,8 @@
             newVec[headerLength].iov_base = &buffer;
             newVec[headerLength].iov_len  = sizeof(buffer);
 
-            ret = TEMP_FAILURE_RETRY(writev(logdLoggerWrite.context.sock, newVec, 2));
+            ret = TEMP_FAILURE_RETRY(writev(
+                    atomic_load(&logdLoggerWrite.context.sock), newVec, 2));
             if (ret != (ssize_t)(sizeof(header) + sizeof(buffer))) {
                 atomic_fetch_add_explicit(&droppedSecurity, snapshot,
                                           memory_order_relaxed);
@@ -194,7 +199,8 @@
             newVec[headerLength].iov_base = &buffer;
             newVec[headerLength].iov_len  = sizeof(buffer);
 
-            ret = TEMP_FAILURE_RETRY(writev(logdLoggerWrite.context.sock, newVec, 2));
+            ret = TEMP_FAILURE_RETRY(writev(
+                      atomic_load(&logdLoggerWrite.context.sock), newVec, 2));
             if (ret != (ssize_t)(sizeof(header) + sizeof(buffer))) {
                 atomic_fetch_add_explicit(&dropped, snapshot,
                                           memory_order_relaxed);
@@ -223,7 +229,8 @@
      * ENOTCONN occurs if logd dies.
      * EAGAIN occurs if logd is overloaded.
      */
-    ret = TEMP_FAILURE_RETRY(writev(logdLoggerWrite.context.sock, newVec, i));
+    ret = TEMP_FAILURE_RETRY(writev(
+            atomic_load(&logdLoggerWrite.context.sock), newVec, i));
     if (ret < 0) {
         ret = -errno;
         if (ret == -ENOTCONN) {
@@ -236,7 +243,8 @@
                 return ret;
             }
 
-            ret = TEMP_FAILURE_RETRY(writev(logdLoggerWrite.context.sock, newVec, i));
+            ret = TEMP_FAILURE_RETRY(writev(
+                    atomic_load(&logdLoggerWrite.context.sock), newVec, i));
             if (ret < 0) {
                 ret = -errno;
             }