From 67f6a530badb4bcc92ac6bf6766e22aca851cea2 Mon Sep 17 00:00:00 2001 From: Yongqin Liu Date: Wed, 28 Dec 2016 16:06:19 +0800 Subject: [PATCH] init: use read_file and write_file to implement do_copy builtin this will make the implementation more cleaner, and has error message output when failed on some operations also add the O_TRUNC flag explicitly for the open function called in write_file. And add more test on read_file and write_file functions Bug: 36726045 Bug: 36576280 Test: manual with hikey Test: boot and init tests on bullhead Test: cast with fugu, per b/36726045 Merged-In: If3c30a2fff58cfece2fcd27e69c30382146e6808 Change-Id: If3c30a2fff58cfece2fcd27e69c30382146e6808 Signed-off-by: Yongqin Liu (cherry picked from commit dbe88e7953ed53961056c7f5531d91d229293462) --- init/README.md | 5 ++++ init/builtins.cpp | 48 ++++--------------------------------- init/util.cpp | 4 ++-- init/util_test.cpp | 60 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 46 deletions(-) diff --git a/init/README.md b/init/README.md index 97681c2b8..8cb1e52a3 100644 --- a/init/README.md +++ b/init/README.md @@ -293,6 +293,11 @@ Commands `copy ` > Copies a file. Similar to write, but useful for binary/large amounts of data. + Regarding to the src file, copying from symbolic link file and world-writable + or group-writable files are not allowed. + Regarding to the dst file, the default mode created is 0600 if it does not + exist. And it will be truncated if dst file is a normal regular file and + already exists. `domainname ` > Set the domain name. diff --git a/init/builtins.cpp b/init/builtins.cpp index 2260358c8..7ee02d0c1 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -652,51 +652,11 @@ static int do_write(const std::vector& args) { } static int do_copy(const std::vector& args) { - char* buffer = NULL; - int rc = 0; - int fd1 = -1, fd2 = -1; - struct stat info; - int brtw, brtr; - char* p; - - if (stat(args[1].c_str(), &info) < 0) return -1; - - if ((fd1 = open(args[1].c_str(), O_RDONLY | O_CLOEXEC)) < 0) goto out_err; - - if ((fd2 = open(args[2].c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0660)) < 0) - goto out_err; - - if (!(buffer = (char*)malloc(info.st_size))) goto out_err; - - p = buffer; - brtr = info.st_size; - while (brtr) { - rc = read(fd1, p, brtr); - if (rc < 0) goto out_err; - if (rc == 0) break; - p += rc; - brtr -= rc; + std::string data; + if (read_file(args[1], &data)) { + return write_file(args[2], data) ? 0 : 1; } - - p = buffer; - brtw = info.st_size; - while (brtw) { - rc = write(fd2, p, brtw); - if (rc < 0) goto out_err; - if (rc == 0) break; - p += rc; - brtw -= rc; - } - - rc = 0; - goto out; -out_err: - rc = -1; -out: - if (buffer) free(buffer); - if (fd1 >= 0) close(fd1); - if (fd2 >= 0) close(fd2); - return rc; + return 1; } static int do_chown(const std::vector& args) { diff --git a/init/util.cpp b/init/util.cpp index c06d51b89..895e78a5a 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -186,8 +186,8 @@ bool read_file(const std::string& path, std::string* content) { } bool write_file(const std::string& path, const std::string& content) { - android::base::unique_fd fd( - TEMP_FAILURE_RETRY(open(path.c_str(), O_WRONLY | O_CREAT | O_NOFOLLOW | O_CLOEXEC, 0600))); + android::base::unique_fd fd(TEMP_FAILURE_RETRY( + open(path.c_str(), O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC | O_CLOEXEC, 0600))); if (fd == -1) { PLOG(ERROR) << "write_file: Unable to open '" << path << "'"; return false; diff --git a/init/util_test.cpp b/init/util_test.cpp index 613499813..0c0350a60 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -17,7 +17,10 @@ #include "util.h" #include +#include +#include +#include #include #include @@ -29,6 +32,35 @@ TEST(util, read_file_ENOENT) { EXPECT_EQ("", s); // s was cleared. } +TEST(util, read_file_group_writeable) { + std::string s("hello"); + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + EXPECT_TRUE(write_file(tf.path, s)) << strerror(errno); + EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0620, AT_SYMLINK_NOFOLLOW)) << strerror(errno); + EXPECT_FALSE(read_file(tf.path, &s)) << strerror(errno); + EXPECT_EQ("", s); // s was cleared. +} + +TEST(util, read_file_world_writeable) { + std::string s("hello"); + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + EXPECT_TRUE(write_file(tf.path, s.c_str())) << strerror(errno); + EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0602, AT_SYMLINK_NOFOLLOW)) << strerror(errno); + EXPECT_FALSE(read_file(tf.path, &s)) << strerror(errno); + EXPECT_EQ("", s); // s was cleared. +} + +TEST(util, read_file_symbolic_link) { + std::string s("hello"); + errno = 0; + // lrwxrwxrwx 1 root root 13 1970-01-01 00:00 charger -> /sbin/healthd + EXPECT_FALSE(read_file("/charger", &s)); + EXPECT_EQ(ELOOP, errno); + EXPECT_EQ("", s); // s was cleared. +} + TEST(util, read_file_success) { std::string s("hello"); EXPECT_TRUE(read_file("/proc/version", &s)); @@ -55,6 +87,34 @@ TEST(util, write_file_binary) { EXPECT_EQ(10u, read_back_contents.size()); } +TEST(util, write_file_not_exist) { + std::string s("hello"); + std::string s2("hello"); + TemporaryDir test_dir; + std::string path = android::base::StringPrintf("%s/does-not-exist", test_dir.path); + EXPECT_TRUE(write_file(path, s)); + EXPECT_TRUE(read_file(path, &s2)); + EXPECT_EQ(s, s2); + struct stat sb; + int fd = open(path.c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC); + EXPECT_NE(-1, fd); + EXPECT_EQ(0, fstat(fd, &sb)); + EXPECT_EQ((const unsigned int)(S_IRUSR | S_IWUSR), sb.st_mode & 0777); + EXPECT_EQ(0, unlink(path.c_str())); +} + +TEST(util, write_file_exist) { + std::string s2(""); + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + EXPECT_TRUE(write_file(tf.path, "1hello1")) << strerror(errno); + EXPECT_TRUE(read_file(tf.path, &s2)); + EXPECT_STREQ("1hello1", s2.c_str()); + EXPECT_TRUE(write_file(tf.path, "2ll2")); + EXPECT_TRUE(read_file(tf.path, &s2)); + EXPECT_STREQ("2ll2", s2.c_str()); +} + TEST(util, decode_uid) { EXPECT_EQ(0U, decode_uid("root")); EXPECT_EQ(UINT_MAX, decode_uid("toot"));