Fix the WriteStringToFile overload that takes mode/owner/group.

The actual bug is == instead of !=, but the real cause was me trying to be
too clever. This patch switches to much simpler code, and -- since the
intended use of this code is security anyway -- adds logging if anything
goes wrong.

Bug: 19361774
Change-Id: If2af07d31a5002f9010b838247b691f6b28bdfb1

1 files changed, 17 insertions, 2 deletions

diff --git a/libutils/file.cpp b/libutils/file.cpp
index 577df78..0690bc2 100644
--- a/libutils/file.cpp
+++ b/libutils/file.cpp
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+#define LOG_TAG "utils.file"
+#include <cutils/log.h>
+
 #include "utils/file.h"
 
 #include <errno.h>
@@ -75,14 +78,26 @@ bool android::WriteStringToFile(const std::string& content, const std::string& p
                                    O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
                                    mode));
   if (fd == -1) {
+    ALOGE("android::WriteStringToFile open failed: %s", strerror(errno));
     return false;
   }
 
   // We do an explicit fchmod here because we assume that the caller really meant what they
   // said and doesn't want the umask-influenced mode.
-  bool result = (fchmod(fd, mode) != -1 && fchown(fd, owner, group) == -1 && WriteStringToFd(content, fd));
+  if (fchmod(fd, mode) == -1) {
+    ALOGE("android::WriteStringToFile fchmod failed: %s", strerror(errno));
+    return CleanUpAfterFailedWrite(path);
+  }
+  if (fchown(fd, owner, group) == -1) {
+    ALOGE("android::WriteStringToFile fchown failed: %s", strerror(errno));
+    return CleanUpAfterFailedWrite(path);
+  }
+  if (!WriteStringToFd(content, fd)) {
+    ALOGE("android::WriteStringToFile write failed: %s", strerror(errno));
+    return CleanUpAfterFailedWrite(path);
+  }
   TEMP_FAILURE_RETRY(close(fd));
-  return result || CleanUpAfterFailedWrite(path);
+  return true;
 }
 #endif