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
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 @@
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
diff --git a/libutils/tests/file_test.cpp b/libutils/tests/file_test.cpp
index 3703a49..cea18b6 100644
--- a/libutils/tests/file_test.cpp
+++ b/libutils/tests/file_test.cpp
@@ -71,6 +71,20 @@
EXPECT_EQ("abc", s);
}
+TEST(file, WriteStringToFile2) {
+ TemporaryFile tf;
+ ASSERT_TRUE(tf.fd != -1);
+ ASSERT_TRUE(android::WriteStringToFile("abc", tf.filename, 0660, getuid(), getgid())) << errno;
+ struct stat sb;
+ ASSERT_EQ(0, stat(tf.filename, &sb));
+ ASSERT_EQ(0660U, (sb.st_mode & ~S_IFMT));
+ ASSERT_EQ(getuid(), sb.st_uid);
+ ASSERT_EQ(getgid(), sb.st_gid);
+ std::string s;
+ ASSERT_TRUE(android::ReadFileToString(tf.filename, &s)) << errno;
+ EXPECT_EQ("abc", s);
+}
+
TEST(file, WriteStringToFd) {
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);