Use cloexec everywhere

Signed-off-by: Andrew Gunnerson <[email protected]>
Dark98 · May 26, 2018 · a2c45e2 · a2c45e2
1 parent f64dfc4
commit a2c45e2
Show file tree

Hide file tree

Showing 39 changed files with 110 additions and 125 deletions.
diff --git a/bootimgtool/bootimgtool.cpp b/bootimgtool/bootimgtool.cpp
@@ -80,6 +80,12 @@
 #define IMAGE_RPM                       "rpm"
 #define IMAGE_APPSBL                    "appsbl"
 
+#ifdef _WIN32
+#  define CLOEXEC_FLAG "N"
+#else
+#  define CLOEXEC_FLAG "e"
+#endif
+
 
 namespace rj = rapidjson;
 
@@ -402,7 +408,7 @@ static bool read_header(const std::string &path, Header &header)
     static const char *fmt_unsupported =
             "Ignoring unsupported key for boot image type: '%s'\n";
 
-    ScopedFILE fp(fopen(path.c_str(), "rb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "rb" CLOEXEC_FLAG), fclose);
     if (!fp) {
         fprintf(stderr, "%s: Failed to open for reading: %s\n",
                 path.c_str(), strerror(errno));
@@ -506,7 +512,7 @@ static bool write_header(const std::string &path, const Header &header)
     absolute_to_offset(base, kernel_offset, ramdisk_offset, second_offset,
                        tags_offset);
 
-    ScopedFILE fp(fopen(path.c_str(), "wb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "wb" CLOEXEC_FLAG), fclose);
     if (!fp) {
         fprintf(stderr, "%s: Failed to open for writing: %s\n",
                 path.c_str(), strerror(errno));
@@ -573,7 +579,7 @@ static bool write_header(const std::string &path, const Header &header)
 
 static bool write_data_file_to_entry(const std::string &path, Writer &writer)
 {
-    ScopedFILE fp(fopen(path.c_str(), "rb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "rb" CLOEXEC_FLAG), fclose);
     if (!fp) {
         // Entries are optional
         if (errno == ENOENT) {
@@ -613,7 +619,7 @@ static bool write_data_file_to_entry(const std::string &path, Writer &writer)
 
 static bool write_data_entry_to_file(const std::string &path, Reader &reader)
 {
-    ScopedFILE fp(fopen(path.c_str(), "wb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "wb" CLOEXEC_FLAG), fclose);
     if (!fp) {
         fprintf(stderr, "%s: Failed to open for writing: %s\n",
                 path.c_str(), strerror(errno));

diff --git a/devicesgen/devicesgen.cpp b/devicesgen/devicesgen.cpp
@@ -219,7 +219,7 @@ int main(int argc, char *argv[])
     FILE *fp = stdout;
 
     if (output_file) {
-        fp = fopen(output_file, "w");
+        fp = fopen(output_file, "we");
         if (!fp) {
             fprintf(stderr, "%s: Failed to open file: %s\n",
                     output_file, strerror(errno));

diff --git a/examples/libmbpatcher_test.cpp b/examples/libmbpatcher_test.cpp
@@ -45,7 +45,7 @@ class BasicLogger : public mb::log::BaseLogger
 static bool file_read_all(const std::string &path,
                           std::vector<unsigned char> &data_out)
 {
-    FILE *fp = fopen(path.c_str(), "rb");
+    FILE *fp = fopen(path.c_str(), "rbe");
     if (!fp) {
         return false;
     }

diff --git a/libmbdevice/schemas2cpp.cpp b/libmbdevice/schemas2cpp.cpp
@@ -277,7 +277,7 @@ int main(int argc, char *argv[])
     std::vector<std::pair<std::string, std::string>> results;
 
     for (int i = optind; i < argc; ++i) {
-        ScopedFILE fp(fopen(argv[i], "r"), &fclose);
+        ScopedFILE fp(fopen(argv[i], "re"), &fclose);
         if (!fp) {
             fprintf(stderr, "%s: Failed to open for reading: %s\n",
                     argv[i], strerror(errno));
@@ -305,7 +305,7 @@ int main(int argc, char *argv[])
 
     // Write cpp file
     {
-        ScopedFILE fp(fopen(output_path, "w"), &fclose);
+        ScopedFILE fp(fopen(output_path, "we"), &fclose);
         if (!fp) {
             fprintf(stderr, "%s: Failed to open for writing: %s\n",
                     output_path, strerror(errno));
@@ -339,7 +339,7 @@ int main(int argc, char *argv[])
         }
         hpp_path += ".h";
 
-        ScopedFILE fp(fopen(hpp_path.c_str(), "w"), &fclose);
+        ScopedFILE fp(fopen(hpp_path.c_str(), "we"), &fclose);
         if (!fp) {
             fprintf(stderr, "%s: Failed to open for writing: %s\n",
                     hpp_path.c_str(), strerror(errno));

diff --git a/libmbutil/src/cmdline.cpp b/libmbutil/src/cmdline.cpp
@@ -19,58 +19,31 @@
 
 #include "mbutil/cmdline.h"
 
-#include <fcntl.h>
-#include <unistd.h>
-
-#include "mbcommon/error_code.h"
-#include "mbcommon/finally.h"
 #include "mbcommon/string.h"
 
+#include "mbutil/file.h"
+
 
 namespace mb::util
 {
 
 oc::result<KernelCmdlineArgs> kernel_cmdline()
 {
-    std::string args;
-
-    {
-        int fd = open("/proc/cmdline", O_RDONLY);
-        if (fd < 0) {
-            return ec_from_errno();
-        }
-
-        auto close_fd = finally([&fd] {
-            close(fd);
-        });
-
-        char buf[10240];
-
-        while (true) {
-            ssize_t n = read(fd, buf, sizeof(buf));
-            if (n < 0) {
-                return ec_from_errno();
-            } else if (n == 0) {
-                break;
-            } else {
-                args.insert(args.end(), buf, buf + n);
-            }
-        }
-    }
+    OUTCOME_TRY(data, file_read_all("/proc/cmdline"));
 
-    if (!args.empty() && args.back() == '\n') {
-        args.pop_back();
+    if (!data.empty() && data.back() == '\n') {
+        data.pop_back();
     }
 
+    std::string_view args(reinterpret_cast<char *>(data.data()), data.size());
     KernelCmdlineArgs result;
 
     for (auto const &item : split_sv(args, " ")) {
         if (item.empty()) {
             continue;
         }
 
-        auto pos = item.find('=');
-        if (pos != std::string_view::npos) {
+        if (auto pos = item.find('='); pos != std::string_view::npos) {
             result.emplace(item.substr(0, pos), item.substr(pos + 1));
         } else {
             result.emplace(item, std::nullopt);

diff --git a/libmbutil/src/copy.cpp b/libmbutil/src/copy.cpp
@@ -74,7 +74,7 @@ oc::result<void> copy_data_fd(int fd_source, int fd_target)
 static FileOpResult<void> copy_data(const std::string &source,
                                     const std::string &target)
 {
-    int fd_source = open(source.c_str(), O_RDONLY);
+    int fd_source = open(source.c_str(), O_RDONLY | O_CLOEXEC);
     if (fd_source < 0) {
         return FileOpErrorInfo{source, ec_from_errno()};
     }
@@ -83,7 +83,8 @@ static FileOpResult<void> copy_data(const std::string &source,
         close(fd_source);
     });
 
-    int fd_target = open(target.c_str(), O_WRONLY | O_CREAT | O_EXCL, 0666);
+    int fd_target = open(target.c_str(),
+                         O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0666);
     if (fd_target < 0) {
         return FileOpErrorInfo{target, ec_from_errno()};
     }
@@ -184,7 +185,7 @@ FileOpResult<void> copy_stat(const std::string &source,
 FileOpResult<void> copy_contents(const std::string &source,
                                  const std::string &target)
 {
-    int fd_source = open(source.c_str(), O_RDONLY);
+    int fd_source = open(source.c_str(), O_RDONLY | O_CLOEXEC);
     if (fd_source < 0) {
         return FileOpErrorInfo{source, ec_from_errno()};
     }
@@ -193,7 +194,8 @@ FileOpResult<void> copy_contents(const std::string &source,
         close(fd_source);
     });
 
-    int fd_target = open(target.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0666);
+    int fd_target = open(target.c_str(),
+                         O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0666);
     if (fd_target < 0) {
         return FileOpErrorInfo{target, ec_from_errno()};
     }

diff --git a/libmbutil/src/file.cpp b/libmbutil/src/file.cpp
@@ -52,15 +52,13 @@ using ScopedFILE = std::unique_ptr<FILE, decltype(fclose) *>;
  */
 oc::result<void> create_empty_file(const std::string &path)
 {
-    int fd;
-    if ((fd = open(path.c_str(), O_RDWR | O_CREAT, S_IRUSR | S_IWUSR |
-                                                   S_IRGRP | S_IWGRP |
-                                                   S_IROTH | S_IWOTH)) < 0) {
+    if (int fd = open(path.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0666);
+            fd >= 0) {
+        close(fd);
+        return oc::success();
+    } else {
         return ec_from_errno();
     }
-
-    close(fd);
-    return oc::success();
 }
 
 /*!
@@ -75,7 +73,7 @@ oc::result<void> create_empty_file(const std::string &path)
  */
 oc::result<std::string> file_first_line(const std::string &path)
 {
-    ScopedFILE fp(fopen(path.c_str(), "rb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "rbe"), fclose);
     if (!fp) {
         return ec_from_errno();
     }
@@ -118,7 +116,7 @@ oc::result<std::string> file_first_line(const std::string &path)
 oc::result<void> file_write_data(const std::string &path,
                                  const void *data, size_t size)
 {
-    ScopedFILE fp(fopen(path.c_str(), "wb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "wbe"), fclose);
     if (!fp) {
         return ec_from_errno();
     }
@@ -146,7 +144,7 @@ oc::result<bool> file_find_one_of(const std::string &path,
     void *map = MAP_FAILED;
     int fd = -1;
 
-    if ((fd = open(path.c_str(), O_RDONLY)) < 0) {
+    if ((fd = open(path.c_str(), O_RDONLY | O_CLOEXEC)) < 0) {
         return ec_from_errno();
     }
 
@@ -180,7 +178,7 @@ oc::result<bool> file_find_one_of(const std::string &path,
 
 oc::result<std::vector<unsigned char>> file_read_all(const std::string &path)
 {
-    ScopedFILE fp(fopen(path.c_str(), "rb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "rbe"), fclose);
     if (!fp) {
         return ec_from_errno();
     }
@@ -213,7 +211,7 @@ oc::result<std::vector<unsigned char>> file_read_all(const std::string &path)
 
 oc::result<uint64_t> get_blockdev_size(const std::string &path)
 {
-    int fd = open(path.c_str(), O_RDONLY);
+    int fd = open(path.c_str(), O_RDONLY | O_CLOEXEC);
     if (fd < 0) {
         return ec_from_errno();
     }

diff --git a/libmbutil/src/fstab.cpp b/libmbutil/src/fstab.cpp
@@ -211,7 +211,7 @@ parse_fs_mgr_options(std::string_view options)
 // Much simplified version of fs_mgr's fstab parsing code
 FstabResult<FstabRecs> read_fstab(const std::string &path)
 {
-    ScopedFILE fp(fopen(path.c_str(), "rb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "rbe"), fclose);
     if (!fp) {
         return FstabErrorInfo{{}, ec_from_errno()};
     }
@@ -289,7 +289,7 @@ FstabResult<FstabRecs> read_fstab(const std::string &path)
 
 FstabResult<TwrpFstabRecs> read_twrp_fstab(const std::string &path)
 {
-    ScopedFILE fp(fopen(path.c_str(), "rb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "rbe"), fclose);
     if (!fp) {
         return FstabErrorInfo{{}, ec_from_errno()};
     }

diff --git a/libmbutil/src/hash.cpp b/libmbutil/src/hash.cpp
@@ -40,7 +40,7 @@ using ScopedFILE = std::unique_ptr<FILE, decltype(fclose) *>;
  */
 oc::result<Sha512Digest> sha512_hash(const std::string &path)
 {
-    ScopedFILE fp(fopen(path.c_str(), "rb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "rbe"), fclose);
     if (!fp) {
         return ec_from_errno();
     }

diff --git a/libmbutil/src/loopdev.cpp b/libmbutil/src/loopdev.cpp
@@ -59,7 +59,7 @@ static oc::result<int> find_loopdev_by_loop_control()
 {
     int fd = -1;
 
-    if ((fd = open(LOOP_CONTROL, O_RDWR)) < 0) {
+    if ((fd = open(LOOP_CONTROL, O_RDWR | O_CLOEXEC)) < 0) {
         return ec_from_errno();
     }
 
@@ -168,7 +168,7 @@ oc::result<void> loopdev_set_up_device(const std::string &loopdev,
                                        const std::string &file,
                                        uint64_t offset, bool ro)
 {
-    int ffd = open(file.c_str(), ro ? O_RDONLY : O_RDWR);
+    int ffd = open(file.c_str(), (ro ? O_RDONLY : O_RDWR) | O_CLOEXEC);
     if (ffd < 0) {
         return ec_from_errno();
     }
@@ -177,7 +177,7 @@ oc::result<void> loopdev_set_up_device(const std::string &loopdev,
         close(ffd);
     });
 
-    int lfd = open(loopdev.c_str(), ro ? O_RDONLY : O_RDWR);
+    int lfd = open(loopdev.c_str(), (ro ? O_RDONLY : O_RDWR) | O_CLOEXEC);
     if (lfd < 0) {
         return ec_from_errno();
     }
@@ -207,7 +207,7 @@ oc::result<void> loopdev_set_up_device(const std::string &loopdev,
 
 oc::result<void> loopdev_remove_device(const std::string &loopdev)
 {
-    int lfd = open(loopdev.c_str(), O_RDONLY);
+    int lfd = open(loopdev.c_str(), O_RDONLY | O_CLOEXEC);
     if (lfd < 0) {
         return ec_from_errno();
     }

diff --git a/libmbutil/src/mount.cpp b/libmbutil/src/mount.cpp
@@ -168,7 +168,7 @@ oc::result<std::vector<MountEntry>> get_mount_entries()
     });
 
     {
-        ScopedFILE fp(fopen(PROC_MOUNTINFO, "r"), fclose);
+        ScopedFILE fp(fopen(PROC_MOUNTINFO, "re"), fclose);
         if (fp) {
             while (getline(&line, &len, fp.get()) != -1) {
                 MountEntry &entry = entries.emplace_back();
@@ -249,7 +249,7 @@ oc::result<std::vector<MountEntry>> get_mount_entries()
     }
 
     {
-        ScopedFILE fp(fopen(PROC_MOUNTS, "r"), fclose);
+        ScopedFILE fp(fopen(PROC_MOUNTS, "re"), fclose);
         if (fp) {
             while (getline(&line, &len, fp.get()) != -1) {
                 MountEntry &entry = entries.emplace_back();

diff --git a/libmbutil/src/properties.cpp b/libmbutil/src/properties.cpp
@@ -251,7 +251,7 @@ typedef PropIterAction (*PropIterCb)(const std::string &key,
 static bool iterate_property_file(const std::string &path, PropIterCb cb,
                                   void *cookie)
 {
-    ScopedFILE fp(fopen(path.c_str(), "r"), &fclose);
+    ScopedFILE fp(fopen(path.c_str(), "re"), &fclose);
     if (!fp) {
         return false;
     }
@@ -380,7 +380,7 @@ bool property_file_get_all(const std::string &path,
 bool property_file_write_all(const std::string &path,
                              const std::unordered_map<std::string, std::string> &map)
 {
-    ScopedFILE fp(fopen(path.c_str(), "wb"), fclose);
+    ScopedFILE fp(fopen(path.c_str(), "wbe"), fclose);
     if (!fp) {
         return false;
     }

diff --git a/libmbutil/src/selinux.cpp b/libmbutil/src/selinux.cpp
@@ -106,7 +106,7 @@ bool selinux_read_policy(const std::string &path, policydb_t *pdb)
     int fd;
 
     for (int i = 0; i < OPEN_ATTEMPTS; ++i) {
-        fd = open(path.c_str(), O_RDONLY);
+        fd = open(path.c_str(), O_RDONLY | O_CLOEXEC);
         if (fd < 0) {
             LOGE("[%d/%d] %s: Failed to open sepolicy: %s",
                  i + 1, OPEN_ATTEMPTS, path.c_str(), strerror(errno));
@@ -180,7 +180,7 @@ bool selinux_write_policy(const std::string &path, policydb_t *pdb)
     });
 
     for (int i = 0; i < OPEN_ATTEMPTS; ++i) {
-        fd = open(path.c_str(), O_CREAT | O_TRUNC | O_RDWR, 0644);
+        fd = open(path.c_str(), O_CREAT | O_TRUNC | O_RDWR | O_CLOEXEC, 0644);
         if (fd < 0) {
             LOGE("[%d/%d] %s: Failed to open sepolicy: %s",
                  i + 1, OPEN_ATTEMPTS, path.c_str(), strerror(errno));
@@ -344,7 +344,7 @@ oc::result<void> selinux_lset_context_recursive(const std::string &path,
 
 oc::result<bool> selinux_get_enforcing()
 {
-    int fd = open(SELINUX_ENFORCE_FILE, O_RDONLY);
+    int fd = open(SELINUX_ENFORCE_FILE, O_RDONLY | O_CLOEXEC);
     if (fd < 0) {
         return ec_from_errno();
     }
@@ -368,7 +368,7 @@ oc::result<bool> selinux_get_enforcing()
 
 oc::result<void> selinux_set_enforcing(bool value)
 {
-    int fd = open(SELINUX_ENFORCE_FILE, O_RDWR);
+    int fd = open(SELINUX_ENFORCE_FILE, O_RDWR | O_CLOEXEC);
     if (fd < 0) {
         return ec_from_errno();
     }