Merge "libc: __system_property_set uses writev to write atomically"

libc/bionic/system_properties.cpp

@@ -34,7 +34,6 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
-#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
@@ -47,6 +46,7 @@
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <sys/uio.h>
 #include <sys/un.h>
 #include <sys/xattr.h>
 
@@ -487,8 +487,8 @@ const prop_info* prop_area::find_property(prop_bt* const trie, const char* name,
 class PropertyServiceConnection {
  public:
   PropertyServiceConnection() : last_error_(0) {
-    fd_ = socket(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0);
+    socket_ = ::socket(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0);
-    if (fd_ == -1) {
+    if (socket_ == -1) {
       last_error_ = errno;
       return;
     }
@@ -500,54 +500,33 @@ class PropertyServiceConnection {
     addr.sun_family = AF_LOCAL;
     socklen_t alen = namelen + offsetof(sockaddr_un, sun_path) + 1;
 
-    if (TEMP_FAILURE_RETRY(connect(fd_, reinterpret_cast<sockaddr*>(&addr), alen)) == -1) {
+    if (TEMP_FAILURE_RETRY(connect(socket_, reinterpret_cast<sockaddr*>(&addr), alen)) == -1) {
-      close(fd_);
+      close(socket_);
-      fd_ = -1;
+      socket_ = -1;
       last_error_ = errno;
     }
   }
 
   bool IsValid() {
-    return fd_ != -1;
+    return socket_ != -1;
   }
 
   int GetLastError() {
     return last_error_;
   }
 
-  bool SendUint32(uint32_t value) {
-    int result = TEMP_FAILURE_RETRY(send(fd_, &value, sizeof(value), 0));
-    return CheckSendRecvResult(result, sizeof(value));
-  }
-
-  bool SendString(const char* value) {
-    uint32_t valuelen = strlen(value);
-    if (!SendUint32(valuelen)) {
-      return false;
-    }
-
-    // Trying to send even 0 bytes to closed socket may lead to
-    // broken pipe (http://b/34670529).
-    if (valuelen == 0) {
-      return true;
-    }
-
-    int result = TEMP_FAILURE_RETRY(send(fd_, value, valuelen, 0));
-    return CheckSendRecvResult(result, valuelen);
-  }
-
   bool RecvInt32(int32_t* value) {
-    int result = TEMP_FAILURE_RETRY(recv(fd_, value, sizeof(*value), MSG_WAITALL));
+    int result = TEMP_FAILURE_RETRY(recv(socket_, value, sizeof(*value), MSG_WAITALL));
     return CheckSendRecvResult(result, sizeof(*value));
   }
 
-  int GetFd() {
+  int socket() {
-    return fd_;
+    return socket_;
   }
 
   ~PropertyServiceConnection() {
-    if (fd_ != -1) {
+    if (socket_ != -1) {
-      close(fd_);
+      close(socket_);
     }
   }
 
@@ -564,8 +543,69 @@ class PropertyServiceConnection {
     return last_error_ == 0;
   }
 
-  int fd_;
+  int socket_;
   int last_error_;
+
+  friend class SocketWriter;
+};
+
+class SocketWriter {
+ public:
+  explicit SocketWriter(PropertyServiceConnection* connection)
+      : connection_(connection), iov_index_(0), uint_buf_index_(0)
+  {}
+
+  SocketWriter& WriteUint32(uint32_t value) {
+    CHECK(uint_buf_index_ < kUintBufSize);
+    CHECK(iov_index_ < kIovSize);
+    uint32_t* ptr = uint_buf_ + uint_buf_index_;
+    uint_buf_[uint_buf_index_++] = value;
+    iov_[iov_index_].iov_base = ptr;
+    iov_[iov_index_].iov_len = sizeof(*ptr);
+    ++iov_index_;
+    return *this;
+  }
+
+  SocketWriter& WriteString(const char* value) {
+    uint32_t valuelen = strlen(value);
+    WriteUint32(valuelen);
+    if (valuelen == 0) {
+      return *this;
+    }
+
+    CHECK(iov_index_ < kIovSize);
+    iov_[iov_index_].iov_base = const_cast<char*>(value);
+    iov_[iov_index_].iov_len = valuelen;
+    ++iov_index_;
+
+    return *this;
+  }
+
+  bool Send() {
+    if (!connection_->IsValid()) {
+      return false;
+    }
+
+    if (writev(connection_->socket(), iov_, iov_index_) == -1) {
+      connection_->last_error_ = errno;
+      return false;
+    }
+
+    iov_index_ = uint_buf_index_ = 0;
+    return true;
+  }
+
+ private:
+  static constexpr size_t kUintBufSize = 8;
+  static constexpr size_t kIovSize = 8;
+
+  PropertyServiceConnection* connection_;
+  iovec iov_[kIovSize];
+  size_t iov_index_;
+  uint32_t uint_buf_[kUintBufSize];
+  size_t uint_buf_index_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(SocketWriter);
 };
 
 struct prop_msg {
@@ -581,9 +621,9 @@ static int send_prop_msg(const prop_msg* msg) {
   }
 
   int result = -1;
-  int fd = connection.GetFd();
+  int s = connection.socket();
 
-  const int num_bytes = TEMP_FAILURE_RETRY(send(fd, msg, sizeof(prop_msg), 0));
+  const int num_bytes = TEMP_FAILURE_RETRY(send(s, msg, sizeof(prop_msg), 0));
   if (num_bytes == sizeof(prop_msg)) {
     // We successfully wrote to the property server but now we
     // wait for the property server to finish its work.  It
@@ -593,7 +633,7 @@ static int send_prop_msg(const prop_msg* msg) {
     // once the socket closes.  Out of paranoia we cap our poll
     // at 250 ms.
     pollfd pollfds[1];
-    pollfds[0].fd = fd;
+    pollfds[0].fd = s;
     pollfds[0].events = 0;
     const int poll_result = TEMP_FAILURE_RETRY(poll(pollfds, 1, 250 /* ms */));
     if (poll_result == 1 && (pollfds[0].revents & POLLHUP) != 0) {
@@ -1230,7 +1270,6 @@ int __system_property_set(const char* key, const char* value) {
     detect_protocol_version();
   }
 
-  int result = -1;
   if (g_propservice_protocol_version == kProtocolVersion1) {
     // Old protocol does not support long names
     if (strlen(key) >= PROP_NAME_MAX) return -1;
@@ -1241,27 +1280,60 @@ int __system_property_set(const char* key, const char* value) {
     strlcpy(msg.name, key, sizeof msg.name);
     strlcpy(msg.value, value, sizeof msg.value);
 
-    result = send_prop_msg(&msg);
+    return send_prop_msg(&msg);
   } else {
     // Use proper protocol
     PropertyServiceConnection connection;
-    if (connection.IsValid() && connection.SendUint32(PROP_MSG_SETPROP2) &&
+    if (!connection.IsValid()) {
-        connection.SendString(key) && connection.SendString(value) &&
+      errno = connection.GetLastError();
-        connection.RecvInt32(&result)) {
+      __libc_format_log(ANDROID_LOG_WARN,
-      if (result != PROP_SUCCESS) {
+                        "libc",
-        __libc_format_log(ANDROID_LOG_WARN, "libc",
+                        "Unable to set property \"%s\" to \"%s\": connection failed; errno=%d (%s)",
-                          "Unable to set property \"%s\" to \"%s\": error code: 0x%x", key, value,
+                        key,
-                          result);
+                        value,
-      }
+                        errno,
-    } else {
+                        strerror(errno));
-      result = connection.GetLastError();
+      return -1;
-      __libc_format_log(ANDROID_LOG_WARN, "libc",
-                        "Unable to set property \"%s\" to \"%s\": error code: 0x%x (%s)", key,
-                        value, result, strerror(result));
     }
-  }
 
-  return result != 0 ? -1 : 0;
+    SocketWriter writer(&connection);
+    if (!writer.WriteUint32(PROP_MSG_SETPROP2).WriteString(key).WriteString(value).Send()) {
+      errno = connection.GetLastError();
+      __libc_format_log(ANDROID_LOG_WARN,
+                        "libc",
+                        "Unable to set property \"%s\" to \"%s\": write failed; errno=%d (%s)",
+                        key,
+                        value,
+                        errno,
+                        strerror(errno));
+      return -1;
+    }
+
+    int result = -1;
+    if (!connection.RecvInt32(&result)) {
+      errno = connection.GetLastError();
+      __libc_format_log(ANDROID_LOG_WARN,
+                        "libc",
+                        "Unable to set property \"%s\" to \"%s\": recv failed; errno=%d (%s)",
+                        key,
+                        value,
+                        errno,
+                        strerror(errno));
+      return -1;
+    }
+
+    if (result != PROP_SUCCESS) {
+      __libc_format_log(ANDROID_LOG_WARN,
+                        "libc",
+                        "Unable to set property \"%s\" to \"%s\": error code: 0x%x",
+                        key,
+                        value,
+                        result);
+      return -1;
+    }
+
+    return 0;
+  }
 }
 
 int __system_property_update(prop_info* pi, const char* value, unsigned int len) {

libc/private/libc_logging.h

@@ -97,6 +97,14 @@ int __libc_format_log_va_list(int pri, const char* _Nonnull tag, const char* _No
 #endif
 int __libc_write_log(int pri, const char* _Nonnull tag, const char* _Nonnull msg);
 
+#define CHECK(predicate) \
+  do { \
+    if (!(predicate)) { \
+      __libc_fatal("%s:%d: %s CHECK '" #predicate "' failed", \
+          __FILE__, __LINE__, __FUNCTION__); \
+    } \
+  } while(0)
+
 __END_DECLS
 
 #endif

linker/linker_debug.h

@@ -59,13 +59,6 @@
 
 __LIBC_HIDDEN__ extern int g_ld_debug_verbosity;
 
-#define CHECK(predicate) { \
-    if (!(predicate)) { \
-      __libc_fatal("%s:%d: %s CHECK '" #predicate "' failed", \
-          __FILE__, __LINE__, __FUNCTION__); \
-    } \
-  }
-
 #if LINKER_DEBUG_TO_LOG
 #define _PRINTVF(v, x...) \
     do { \