From 2ef4e85448df5767b98e0d1dc793b7805249393f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 24 Feb 2023 18:09:25 +0000 Subject: [PATCH] Strictly 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. 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 Change-Id: I99e4d530a00f9401532c9cb0990df254b7a12a80 --- Utils.cpp | 62 ++++++++++++++++++--------------------- main.cpp | 6 ++-- vold_prepare_subdirs.cpp | 63 ++++++++++++++++++++-------------------- 3 files changed, 63 insertions(+), 68 deletions(-) diff --git a/Utils.cpp b/Utils.cpp index 33dbaf6..7f64d33 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -100,13 +101,18 @@ 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) { + PLOG(ERROR) << "Failed to look up selabel for device node " << path; + return -errno; + } + secontext.reset(tmp_secontext); + if (setfscreatecon(secontext.get()) != 0) { + LOG(ERROR) << "Failed to setfscreatecon for device node " << path; + return -EINVAL; } mode_t mode = 0660 | S_IFBLK; @@ -114,16 +120,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 +449,23 @@ 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); - } - } - - 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 (selabel_lookup(sehandle, &tmp_secontext, cpath, S_IFDIR) != 0) { + PLOG(ERROR) << "Failed to look up selabel for directory " << path; return -errno; } + secontext.reset(tmp_secontext); + if (setfscreatecon(secontext.get()) != 0) { + LOG(ERROR) << "Failed to setfscreatecon for directory " << path; + return -EINVAL; + } + + 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..f4d4309 100644 --- a/vold_prepare_subdirs.cpp +++ b/vold_prepare_subdirs.cpp @@ -58,50 +58,45 @@ 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 (selabel_lookup(sehandle, &tmp_secontext, path.c_str(), S_IFDIR) != 0) { + PLOG(ERROR) << "Failed to look up selabel for directory " << path; + return false; + } + 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 (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); } 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; - } + << gid << " context " << secontext.get() << " on path: " << path; + if (setfscreatecon(secontext.get()) != 0) { + LOG(ERROR) << "Failed to setfscreatecon for directory " << path; + return false; } if (fs_prepare_dir(path.c_str(), mode, uid, gid) != 0) { return false; } - if (secontext) { - char* tmp_oldsecontext = nullptr; - if (lgetfilecon(path.c_str(), &tmp_oldsecontext) < 0) { - PLOG(ERROR) << "Unable to read secontext for: " << path; + char* tmp_oldsecontext = nullptr; + if (lgetfilecon(path.c_str(), &tmp_oldsecontext) < 0) { + PLOG(ERROR) << "Unable to read secontext for: " << path; + return false; + } + auto oldsecontext = std::unique_ptr(tmp_oldsecontext, freecon); + if (strcmp(secontext.get(), oldsecontext.get()) != 0) { + LOG(INFO) << "Relabelling from " << ((char*)oldsecontext.get()) << " to " + << ((char*)secontext.get()) << ": " << path; + if (lsetfilecon(path.c_str(), secontext.get()) != 0) { + PLOG(ERROR) << "Relabelling failed for: " << path; return false; } - auto oldsecontext = std::unique_ptr(tmp_oldsecontext, freecon); - if (strcmp(secontext.get(), oldsecontext.get()) != 0) { - LOG(INFO) << "Relabelling from " << ((char*)oldsecontext.get()) << " to " - << ((char*)secontext.get()) << ": " << path; - if (lsetfilecon(path.c_str(), secontext.get()) != 0) { - PLOG(ERROR) << "Relabelling failed for: " << path; - return false; - } - } } return true; } @@ -168,6 +163,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);