From 4217f8ecfe8f884288104262000275c1633b983e Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Mon, 30 Oct 2017 14:31:41 -0700 Subject: Update shell open command to have a mode. We need to open files for reading sometimes, so now we can. Test: manual Change-Id: If089c1e7a4aac1d85784e070d2fccb04b9d84391 --- cmds/cmd/cmd.cpp | 53 +++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 10 deletions(-) (limited to 'cmds/cmd/cmd.cpp') diff --git a/cmds/cmd/cmd.cpp b/cmds/cmd/cmd.cpp index 014c99593c..40dbbf551c 100644 --- a/cmds/cmd/cmd.cpp +++ b/cmds/cmd/cmd.cpp @@ -61,7 +61,8 @@ class MyShellCallback : public BnShellCallback public: bool mActive = true; - virtual int openOutputFile(const String16& path, const String16& seLinuxContext) { + virtual int openFile(const String16& path, const String16& seLinuxContext, + const String16& mode) { String8 path8(path); char cwd[256]; getcwd(cwd, 256); @@ -71,7 +72,26 @@ public: aerr << "Open attempt after active for: " << fullPath << endl; return -EPERM; } - int fd = open(fullPath.string(), O_WRONLY|O_CREAT|O_TRUNC, S_IRWXU|S_IRWXG); + int flags = 0; + bool checkRead = false; + bool checkWrite = false; + if (mode == String16("w")) { + flags = O_WRONLY|O_CREAT|O_TRUNC; + checkWrite = true; + } else if (mode == String16("w+")) { + flags = O_RDWR|O_CREAT|O_TRUNC; + checkRead = checkWrite = true; + } else if (mode == String16("r")) { + flags = O_RDONLY; + checkRead = true; + } else if (mode == String16("r+")) { + flags = O_RDWR; + checkRead = checkWrite = true; + } else { + aerr << "Invalid mode requested: " << mode.string() << endl; + return -EINVAL; + } + int fd = open(fullPath.string(), flags, S_IRWXU|S_IRWXG); if (fd < 0) { return fd; } @@ -80,14 +100,27 @@ public: security_context_t tmp = NULL; int ret = getfilecon(fullPath.string(), &tmp); Unique_SecurityContext context(tmp); - int accessGranted = selinux_check_access(seLinuxContext8.string(), context.get(), - "file", "write", NULL); - if (accessGranted != 0) { - close(fd); - aerr << "System server has no access to file context " << context.get() - << " (from path " << fullPath.string() << ", context " - << seLinuxContext8.string() << ")" << endl; - return -EPERM; + if (checkWrite) { + int accessGranted = selinux_check_access(seLinuxContext8.string(), context.get(), + "file", "write", NULL); + if (accessGranted != 0) { + close(fd); + aerr << "System server has no access to write file context " << context.get() + << " (from path " << fullPath.string() << ", context " + << seLinuxContext8.string() << ")" << endl; + return -EPERM; + } + } + if (checkRead) { + int accessGranted = selinux_check_access(seLinuxContext8.string(), context.get(), + "file", "read", NULL); + if (accessGranted != 0) { + close(fd); + aerr << "System server has no access to read file context " << context.get() + << " (from path " << fullPath.string() << ", context " + << seLinuxContext8.string() << ")" << endl; + return -EPERM; + } } } return fd; -- cgit v1.2.3-59-g8ed1b From 228f2f66318e97da7c9d13fdc9f28ebddaa1fb46 Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Tue, 14 Nov 2017 11:18:08 -0800 Subject: Add more debugging to cmd. Test: manual Change-Id: Ibb6c6dd99446504452921cb148d64461a6f0ef85 --- cmds/cmd/cmd.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'cmds/cmd/cmd.cpp') diff --git a/cmds/cmd/cmd.cpp b/cmds/cmd/cmd.cpp index 40dbbf551c..6511b446c1 100644 --- a/cmds/cmd/cmd.cpp +++ b/cmds/cmd/cmd.cpp @@ -72,6 +72,9 @@ public: aerr << "Open attempt after active for: " << fullPath << endl; return -EPERM; } +#if DEBUG + ALOGD("openFile: %s, full=%s", path8.string(), fullPath.string()); +#endif int flags = 0; bool checkRead = false; bool checkWrite = false; @@ -92,6 +95,9 @@ public: return -EINVAL; } int fd = open(fullPath.string(), flags, S_IRWXU|S_IRWXG); +#if DEBUG + ALOGD("openFile: fd=%d", fd); +#endif if (fd < 0) { return fd; } @@ -104,6 +110,9 @@ public: int accessGranted = selinux_check_access(seLinuxContext8.string(), context.get(), "file", "write", NULL); if (accessGranted != 0) { +#if DEBUG + ALOGD("openFile: failed selinux write check!"); +#endif close(fd); aerr << "System server has no access to write file context " << context.get() << " (from path " << fullPath.string() << ", context " @@ -115,6 +124,9 @@ public: int accessGranted = selinux_check_access(seLinuxContext8.string(), context.get(), "file", "read", NULL); if (accessGranted != 0) { +#if DEBUG + ALOGD("openFile: failed selinux read check!"); +#endif close(fd); aerr << "System server has no access to read file context " << context.get() << " (from path " << fullPath.string() << ", context " @@ -164,6 +176,9 @@ int main(int argc, char* const argv[]) proc->setThreadPoolMaxThreadCount(0); proc->startThreadPool(); +#if DEBUG + ALOGD("cmd: starting"); +#endif sp sm = defaultServiceManager(); fflush(stdout); if (sm == NULL) { -- cgit v1.2.3-59-g8ed1b From f463e180e5a69b4a43acd41c41c8518fa8c22b2d Mon Sep 17 00:00:00 2001 From: Chih-Hung Hsieh Date: Tue, 5 Dec 2017 10:20:12 -0800 Subject: Use -Werror in frameworks/native/cmds/cmd * Ignore return value of getfilecon? Bug: 66996870 Test: build with WITH_TIDY=1 Change-Id: I341e6047dbb5b49a92f69df74b073d2f54a7754d --- cmds/cmd/Android.mk | 2 ++ cmds/cmd/cmd.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'cmds/cmd/cmd.cpp') diff --git a/cmds/cmd/Android.mk b/cmds/cmd/Android.mk index d565e57f43..4868555b34 100644 --- a/cmds/cmd/Android.mk +++ b/cmds/cmd/Android.mk @@ -10,6 +10,8 @@ LOCAL_SHARED_LIBRARIES := \ libselinux \ libbinder +LOCAL_CFLAGS := -Wall -Werror + LOCAL_C_INCLUDES += \ $(JNI_H_INCLUDE) diff --git a/cmds/cmd/cmd.cpp b/cmds/cmd/cmd.cpp index 6511b446c1..48d5d4aed7 100644 --- a/cmds/cmd/cmd.cpp +++ b/cmds/cmd/cmd.cpp @@ -104,7 +104,7 @@ public: if (is_selinux_enabled() && seLinuxContext.size() > 0) { String8 seLinuxContext8(seLinuxContext); security_context_t tmp = NULL; - int ret = getfilecon(fullPath.string(), &tmp); + getfilecon(fullPath.string(), &tmp); Unique_SecurityContext context(tmp); if (checkWrite) { int accessGranted = selinux_check_access(seLinuxContext8.string(), context.get(), -- cgit v1.2.3-59-g8ed1b From 8e39ddd81e43e0a5be6c1916649c4a840518ca45 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 24 May 2018 17:57:31 -0700 Subject: Revert "Only spawn one binder thread for cmd." This was being used as a work around for a static destructor issue that has been fixed. This reverts commit c21bc9afe403f52c189d2d6b79dedaf9dce6217b. Bug: 36066697 Bug: 77934844 Test: cmd still works Change-Id: Id13589c5c12de0a36b70b8d16216e21345eb986e --- cmds/cmd/cmd.cpp | 7 ------- 1 file changed, 7 deletions(-) (limited to 'cmds/cmd/cmd.cpp') diff --git a/cmds/cmd/cmd.cpp b/cmds/cmd/cmd.cpp index 48d5d4aed7..423853175b 100644 --- a/cmds/cmd/cmd.cpp +++ b/cmds/cmd/cmd.cpp @@ -167,13 +167,6 @@ int main(int argc, char* const argv[]) { signal(SIGPIPE, SIG_IGN); sp proc = ProcessState::self(); - // setThreadPoolMaxThreadCount(0) actually tells the kernel it's - // not allowed to spawn any additional threads, but we still spawn - // a binder thread from userspace when we call startThreadPool(). - // This is safe because we only have 2 callbacks, neither of which - // block. - // See b/36066697 for rationale - proc->setThreadPoolMaxThreadCount(0); proc->startThreadPool(); #if DEBUG -- cgit v1.2.3-59-g8ed1b