From 40578ab91f48f0654ca71cd147d0ab9359478b4b Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Mon, 20 Mar 2017 14:24:25 -0700 Subject: [PATCH] init: Fix leaking fd and error print read_file() does not close its fd if either stat() fails or the file has group/world writable permissions. Use unique_fd to ensure that all return paths close the fd and make the same change to write_file() for consistency. Replace PLOG() with LOG() after a simple if conditional, that does not set errno. Old: init: skipping insecure file '/data/bootchart/header': No such device or address New: init: skipping insecure file '/data/bootchart/header' Test: Cause an invalid file read and check the error log Test: Ensure non-error read_file() and write_file() work Change-Id: Ib15d94e38362e335d671d30b36aa5605254ec7ab --- init/util.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/init/util.cpp b/init/util.cpp index 888a36652..c98718101 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -163,7 +163,7 @@ out_unlink: bool read_file(const char* path, std::string* content) { content->clear(); - int fd = TEMP_FAILURE_RETRY(open(path, O_RDONLY|O_NOFOLLOW|O_CLOEXEC)); + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(path, O_RDONLY | O_NOFOLLOW | O_CLOEXEC))); if (fd == -1) { return false; } @@ -176,17 +176,16 @@ bool read_file(const char* path, std::string* content) { return false; } if ((sb.st_mode & (S_IWGRP | S_IWOTH)) != 0) { - PLOG(ERROR) << "skipping insecure file '" << path << "'"; + LOG(ERROR) << "skipping insecure file '" << path << "'"; return false; } - bool okay = android::base::ReadFdToString(fd, content); - close(fd); - return okay; + return android::base::ReadFdToString(fd, content); } bool write_file(const char* path, const char* content) { - int fd = TEMP_FAILURE_RETRY(open(path, O_WRONLY|O_CREAT|O_NOFOLLOW|O_CLOEXEC, 0600)); + android::base::unique_fd fd( + TEMP_FAILURE_RETRY(open(path, O_WRONLY | O_CREAT | O_NOFOLLOW | O_CLOEXEC, 0600))); if (fd == -1) { PLOG(ERROR) << "write_file: Unable to open '" << path << "'"; return false; @@ -195,7 +194,6 @@ bool write_file(const char* path, const char* content) { if (!success) { PLOG(ERROR) << "write_file: Unable to write to '" << path << "'"; } - close(fd); return success; }