init: service file command only opens existing files
Mixing open or create, along with attribute(MAC) and permissions(DAC)
is a security and confusion issue.
Fix an issue where fcntl F_SETFD was called to clear O_NONBLOCK, when
it should have been F_SETFL. Did not present a problem because the
current user of this feature does writes and control messages only.
Test: gTest logd-unit-tests and check dmesg for logd content.
Bug: 32450474
Bug: 33242020
Change-Id: I23cb9a9be5ddb7e8e9c58c79838bc07536e766e6
diff --git a/init/Android.mk b/init/Android.mk
index 442a5f3..ecdf5db 100644
--- a/init/Android.mk
+++ b/init/Android.mk
@@ -126,7 +126,6 @@
LOCAL_SHARED_LIBRARIES += \
libcutils \
libbase \
- libselinux \
LOCAL_STATIC_LIBRARIES := libinit
LOCAL_SANITIZE := integer
diff --git a/init/descriptors.cpp b/init/descriptors.cpp
index 429a76e..6e457cd 100644
--- a/init/descriptors.cpp
+++ b/init/descriptors.cpp
@@ -23,6 +23,7 @@
#include <unistd.h>
#include <android-base/stringprintf.h>
+#include <android-base/unique_fd.h>
#include <cutils/android_get_control_file.h>
#include <cutils/sockets.h>
@@ -89,14 +90,34 @@
FileInfo::FileInfo(const std::string& name, const std::string& type, uid_t uid,
gid_t gid, int perm, const std::string& context)
+ // defaults OK for uid,..., they are ignored for this class.
: DescriptorInfo(name, type, uid, gid, perm, context) {
}
-int FileInfo::Create(const std::string& context) const {
- int flags = ((type() == "r" ? O_RDONLY :
- (type() == "w" ? (O_WRONLY | O_CREAT) :
- (O_RDWR | O_CREAT))));
- return create_file(name().c_str(), flags, perm(), uid(), gid(), context.c_str());
+int FileInfo::Create(const std::string&) const {
+ int flags = (type() == "r") ? O_RDONLY :
+ (type() == "w") ? O_WRONLY :
+ O_RDWR;
+
+ // Make sure we do not block on open (eg: devices can chose to block on
+ // carrier detect). Our intention is never to delay launch of a service
+ // for such a condition. The service can perform its own blocking on
+ // carrier detect.
+ android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(name().c_str(),
+ flags | O_NONBLOCK)));
+
+ if (fd < 0) {
+ PLOG(ERROR) << "Failed to open file '" << name().c_str() << "'";
+ return -1;
+ }
+
+ // Fixup as we set O_NONBLOCK for open, the intent for fd is to block reads.
+ fcntl(fd, F_SETFL, flags);
+
+ LOG(INFO) << "Opened file '" << name().c_str() << "'"
+ << ", flags " << std::oct << flags << std::dec;
+
+ return fd.release();
}
const std::string FileInfo::key() const {
diff --git a/init/readme.txt b/init/readme.txt
index 7549e3c..36bf698 100644
--- a/init/readme.txt
+++ b/init/readme.txt
@@ -148,13 +148,10 @@
seclabel or computed based on the service executable file security context.
For native executables see libcutils android_get_control_socket().
-file <path> <type> <perm> [ <user> [ <group> [ <seclabel> ] ] ]
- Open/Create a file path and pass its fd to the launched process. <type> must
- be "r", "w" or "rw". User and group default to 0. 'seclabel' is the SELinux
- security context for the file if it must be created. It defaults to the
- service security context, as specified by seclabel or computed based on the
- service executable file security context. For native executables see
- libcutils android_get_control_file().
+file <path> <type>
+ Open a file path and pass its fd to the launched process. <type> must be
+ "r", "w" or "rw". For native executables see libcutils
+ android_get_control_file().
user <username>
Change to 'username' before exec'ing this service.
diff --git a/init/service.cpp b/init/service.cpp
index 4b9724d..a7eaf66 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -526,7 +526,7 @@
{"seclabel", {1, 1, &Service::ParseSeclabel}},
{"setenv", {2, 2, &Service::ParseSetenv}},
{"socket", {3, 6, &Service::ParseSocket}},
- {"file", {2, 6, &Service::ParseFile}},
+ {"file", {2, 2, &Service::ParseFile}},
{"user", {1, 1, &Service::ParseUser}},
{"writepid", {1, kMax, &Service::ParseWritepid}},
};
diff --git a/init/util.cpp b/init/util.cpp
index 38e3b8d..a79a419 100644
--- a/init/util.cpp
+++ b/init/util.cpp
@@ -160,69 +160,6 @@
return -1;
}
-/*
- * create_file - opens and creates a file as dictated in init.rc.
- * This file is inherited by the daemon. We communicate the file
- * descriptor's value via the environment variable ANDROID_FILE_<basename>
- */
-int create_file(const char *path, int flags, mode_t perm, uid_t uid,
- gid_t gid, const char *filecon)
-{
- char *secontext = NULL;
-
- if (filecon) {
- if (setsockcreatecon(filecon) == -1) {
- PLOG(ERROR) << "setsockcreatecon(\"" << filecon << "\") failed";
- return -1;
- }
- } else if (sehandle) {
- if (selabel_lookup(sehandle, &secontext, path, perm) != -1) {
- if (setfscreatecon(secontext) == -1) {
- freecon(secontext); // does not upset errno value
- PLOG(ERROR) << "setfscreatecon(\"" << secontext << "\") failed";
- return -1;
- }
- }
- }
-
- android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(path, flags | O_NDELAY, perm)));
- int savederrno = errno;
-
- if (filecon) {
- setsockcreatecon(NULL);
- lsetfilecon(path, filecon);
- } else {
- setfscreatecon(NULL);
- freecon(secontext);
- }
-
- if (fd < 0) {
- errno = savederrno;
- PLOG(ERROR) << "Failed to open/create file '" << path << "'";
- return -1;
- }
-
- if (!(flags & O_NDELAY)) fcntl(fd, F_SETFD, flags);
-
- if (lchown(path, uid, gid)) {
- PLOG(ERROR) << "Failed to lchown file '" << path << "'";
- return -1;
- }
- if (perm != static_cast<mode_t>(-1)) {
- if (fchmodat(AT_FDCWD, path, perm, AT_SYMLINK_NOFOLLOW)) {
- PLOG(ERROR) << "Failed to fchmodat file '" << path << "'";
- return -1;
- }
- }
-
- LOG(INFO) << "Created file '" << path << "'"
- << ", mode " << std::oct << perm << std::dec
- << ", user " << uid
- << ", group " << gid;
-
- return fd.release();
-}
-
bool read_file(const char* path, std::string* content) {
content->clear();
diff --git a/init/util.h b/init/util.h
index 4b54361..e63c469 100644
--- a/init/util.h
+++ b/init/util.h
@@ -31,8 +31,6 @@
int create_socket(const char *name, int type, mode_t perm,
uid_t uid, gid_t gid, const char *socketcon);
-int create_file(const char *path, int mode, mode_t perm,
- uid_t uid, gid_t gid, const char *filecon);
bool read_file(const char* path, std::string* content);
int write_file(const char* path, const char* content);
diff --git a/init/util_test.cpp b/init/util_test.cpp
index e9f164d..24c75c4 100644
--- a/init/util_test.cpp
+++ b/init/util_test.cpp
@@ -16,19 +16,9 @@
#include "util.h"
-#include <ctype.h>
#include <errno.h>
-#include <fcntl.h>
-#include <stdlib.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <unistd.h>
-#include <android-base/stringprintf.h>
-#include <android-base/test_utils.h>
-#include <cutils/android_get_control_file.h>
#include <gtest/gtest.h>
-#include <selinux/android.h>
TEST(util, read_file_ENOENT) {
std::string s("hello");
@@ -52,54 +42,3 @@
EXPECT_EQ(UINT_MAX, decode_uid("toot"));
EXPECT_EQ(123U, decode_uid("123"));
}
-
-struct selabel_handle *sehandle;
-
-TEST(util, create_file) {
- if (!sehandle) sehandle = selinux_android_file_context_handle();
-
- TemporaryFile tf;
- close(tf.fd);
- EXPECT_GE(unlink(tf.path), 0);
-
- std::string key(ANDROID_FILE_ENV_PREFIX);
- key += tf.path;
-
- std::for_each(key.begin(), key.end(), [] (char& c) { c = isalnum(c) ? c : '_'; });
-
- EXPECT_EQ(unsetenv(key.c_str()), 0);
-
- uid_t uid = decode_uid("logd");
- gid_t gid = decode_uid("system");
- mode_t perms = S_IRWXU | S_IWGRP | S_IRGRP | S_IROTH;
- static const char context[] = "u:object_r:misc_logd_file:s0";
- EXPECT_GE(tf.fd = create_file(tf.path, O_RDWR | O_CREAT, perms, uid, gid, context), 0);
- if (tf.fd < 0) return;
- static const char hello[] = "hello world\n";
- static const ssize_t len = strlen(hello);
- EXPECT_EQ(write(tf.fd, hello, len), len);
- char buffer[sizeof(hello) + 1];
- memset(buffer, 0, sizeof(buffer));
- EXPECT_GE(lseek(tf.fd, 0, SEEK_SET), 0);
- EXPECT_EQ(read(tf.fd, buffer, sizeof(buffer)), len);
- EXPECT_EQ(std::string(hello), buffer);
- EXPECT_EQ(android_get_control_file(tf.path), -1);
- EXPECT_EQ(setenv(key.c_str(), android::base::StringPrintf("%d", tf.fd).c_str(), true), 0);
- EXPECT_EQ(android_get_control_file(tf.path), tf.fd);
- close(tf.fd);
- EXPECT_EQ(android_get_control_file(tf.path), -1);
- EXPECT_EQ(unsetenv(key.c_str()), 0);
- struct stat st;
- EXPECT_EQ(stat(tf.path, &st), 0);
- EXPECT_EQ(st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO), perms);
- EXPECT_EQ(st.st_uid, uid);
- EXPECT_EQ(st.st_gid, gid);
- security_context_t con;
- EXPECT_GE(getfilecon(tf.path, &con), 0);
- EXPECT_NE(con, static_cast<security_context_t>(NULL));
- if (con) {
- EXPECT_EQ(context, std::string(con));
- }
- freecon(con);
- EXPECT_EQ(unlink(tf.path), 0);
-}