From c7c4f5a9023d6366964e4f9025848bd61211ff76 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 1 Mar 2023 20:08:59 +0000 Subject: [PATCH] Check for SELinux labelling errors It's essential that files created by vold get the correct SELinux labels, so make sure to check for errors when setting them. ENOENT (no label defined) is expected on some files such as /mnt/appfuse/*, so allow ENOENT but log a DEBUG message. This will help debug b/269567270. This is not a fix for b/269567270. Bug: 269567270 Test: Created user and checked SELinux labels of user's directories Test: atest CtsBlobStoreHostTestCases Change-Id: Ife005bdd896952653943c57336deb33456f7c5d8 --- Utils.cpp | 62 ++++++++++++++++++++-------------------- main.cpp | 6 ++-- vold_prepare_subdirs.cpp | 40 ++++++++++++++------------ 3 files changed, 57 insertions(+), 51 deletions(-) diff --git a/Utils.cpp b/Utils.cpp index 33dbaf6..157b839 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -100,13 +101,21 @@ std::string GetFuseMountPathForUser(userid_t user_id, const std::string& relativ status_t CreateDeviceNode(const std::string& path, dev_t dev) { std::lock_guard lock(kSecurityLock); const char* cpath = path.c_str(); - status_t res = 0; + auto clearfscreatecon = android::base::make_scope_guard([] { setfscreatecon(nullptr); }); + auto secontext = std::unique_ptr(nullptr, freecon); + char* tmp_secontext; - char* secontext = nullptr; - if (sehandle) { - if (!selabel_lookup(sehandle, &secontext, cpath, S_IFBLK)) { - setfscreatecon(secontext); + if (selabel_lookup(sehandle, &tmp_secontext, cpath, S_IFBLK) == 0) { + secontext.reset(tmp_secontext); + if (setfscreatecon(secontext.get()) != 0) { + LOG(ERROR) << "Failed to setfscreatecon for device node " << path; + return -EINVAL; } + } else if (errno == ENOENT) { + LOG(DEBUG) << "No selabel defined for device node " << path; + } else { + PLOG(ERROR) << "Failed to look up selabel for device node " << path; + return -errno; } mode_t mode = 0660 | S_IFBLK; @@ -114,16 +123,10 @@ status_t CreateDeviceNode(const std::string& path, dev_t dev) { if (errno != EEXIST) { PLOG(ERROR) << "Failed to create device node for " << major(dev) << ":" << minor(dev) << " at " << path; - res = -errno; + return -errno; } } - - if (secontext) { - setfscreatecon(nullptr); - freecon(secontext); - } - - return res; + return OK; } status_t DestroyDeviceNode(const std::string& path) { @@ -449,29 +452,26 @@ status_t PrepareDir(const std::string& path, mode_t mode, uid_t uid, gid_t gid, unsigned int attrs) { std::lock_guard lock(kSecurityLock); const char* cpath = path.c_str(); + auto clearfscreatecon = android::base::make_scope_guard([] { setfscreatecon(nullptr); }); + auto secontext = std::unique_ptr(nullptr, freecon); + char* tmp_secontext; - char* secontext = nullptr; - if (sehandle) { - if (!selabel_lookup(sehandle, &secontext, cpath, S_IFDIR)) { - setfscreatecon(secontext); + if (selabel_lookup(sehandle, &tmp_secontext, cpath, S_IFDIR) == 0) { + secontext.reset(tmp_secontext); + if (setfscreatecon(secontext.get()) != 0) { + LOG(ERROR) << "Failed to setfscreatecon for directory " << path; + return -EINVAL; } - } - - int res = fs_prepare_dir(cpath, mode, uid, gid); - - if (secontext) { - setfscreatecon(nullptr); - freecon(secontext); - } - - if (res) return -errno; - if (attrs) res = SetAttrs(path, attrs); - - if (res == 0) { - return OK; + } else if (errno == ENOENT) { + LOG(DEBUG) << "No selabel defined for directory " << path; } else { + PLOG(ERROR) << "Failed to look up selabel for directory " << path; return -errno; } + + if (fs_prepare_dir(cpath, mode, uid, gid) != 0) return -errno; + if (attrs && SetAttrs(path, attrs) != 0) return -errno; + return OK; } status_t ForceUnmount(const std::string& path) { diff --git a/main.cpp b/main.cpp index b07ee68..078ee14 100644 --- a/main.cpp +++ b/main.cpp @@ -82,9 +82,11 @@ int main(int argc, char** argv) { parse_args(argc, argv); sehandle = selinux_android_file_context_handle(); - if (sehandle) { - selinux_android_set_sehandle(sehandle); + if (!sehandle) { + LOG(ERROR) << "Failed to get SELinux file contexts handle"; + exit(1); } + selinux_android_set_sehandle(sehandle); mkdir("/dev/block/vold", 0755); diff --git a/vold_prepare_subdirs.cpp b/vold_prepare_subdirs.cpp index 6ad3c6f..60e82f5 100644 --- a/vold_prepare_subdirs.cpp +++ b/vold_prepare_subdirs.cpp @@ -58,32 +58,32 @@ static bool prepare_dir_for_user(struct selabel_handle* sehandle, mode_t mode, u const std::string& path, uid_t user_id) { auto clearfscreatecon = android::base::make_scope_guard([] { setfscreatecon(nullptr); }); auto secontext = std::unique_ptr(nullptr, freecon); - if (sehandle) { - char* tmp_secontext; + char* tmp_secontext; - if (selabel_lookup(sehandle, &tmp_secontext, path.c_str(), S_IFDIR) == 0) { - secontext.reset(tmp_secontext); - - if (user_id != (uid_t)-1) { - if (selinux_android_context_with_level(secontext.get(), &tmp_secontext, user_id, - (uid_t)-1) != 0) { - PLOG(ERROR) << "Unable to create context with level for: " << path; - return false; - } - secontext.reset(tmp_secontext); // Free the context + if (selabel_lookup(sehandle, &tmp_secontext, path.c_str(), S_IFDIR) == 0) { + secontext.reset(tmp_secontext); + if (user_id != (uid_t)-1) { + if (selinux_android_context_with_level(secontext.get(), &tmp_secontext, user_id, + (uid_t)-1) != 0) { + PLOG(ERROR) << "Unable to create context with level for: " << path; + return false; } + secontext.reset(tmp_secontext); } + if (setfscreatecon(secontext.get()) != 0) { + LOG(ERROR) << "Failed to setfscreatecon for directory " << path; + return false; + } + } else if (errno == ENOENT) { + LOG(DEBUG) << "No selabel defined for directory " << path; + } else { + PLOG(ERROR) << "Failed to look up selabel for directory " << path; + return false; } LOG(DEBUG) << "Setting up mode " << std::oct << mode << std::dec << " uid " << uid << " gid " << gid << " context " << (secontext ? secontext.get() : "null") << " on path: " << path; - if (secontext) { - if (setfscreatecon(secontext.get()) != 0) { - PLOG(ERROR) << "Unable to setfscreatecon for: " << path; - return false; - } - } if (fs_prepare_dir(path.c_str(), mode, uid, gid) != 0) { return false; } @@ -168,6 +168,10 @@ static bool prepare_apex_subdirs(struct selabel_handle* sehandle, const std::str static bool prepare_subdirs(const std::string& volume_uuid, int user_id, int flags) { struct selabel_handle* sehandle = selinux_android_file_context_handle(); + if (!sehandle) { + LOG(ERROR) << "Failed to get SELinux file contexts handle"; + return false; + } if (flags & android::os::IVold::STORAGE_FLAG_DE) { auto user_de_path = android::vold::BuildDataUserDePath(volume_uuid, user_id);