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
diff --git a/Utils.cpp b/Utils.cpp
index 33dbaf6..7f64d33 100644
--- a/Utils.cpp
+++ b/Utils.cpp
@@ -23,6 +23,7 @@
#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/properties.h>
+#include <android-base/scopeguard.h>
#include <android-base/stringprintf.h>
#include <android-base/strings.h>
#include <android-base/unique_fd.h>
@@ -100,13 +101,18 @@
status_t CreateDeviceNode(const std::string& path, dev_t dev) {
std::lock_guard<std::mutex> 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<char, void (*)(char*)>(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 @@
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 @@
unsigned int attrs) {
std::lock_guard<std::mutex> lock(kSecurityLock);
const char* cpath = path.c_str();
+ auto clearfscreatecon = android::base::make_scope_guard([] { setfscreatecon(nullptr); });
+ auto secontext = std::unique_ptr<char, void (*)(char*)>(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 @@
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 @@
const std::string& path, uid_t user_id) {
auto clearfscreatecon = android::base::make_scope_guard([] { setfscreatecon(nullptr); });
auto secontext = std::unique_ptr<char, void (*)(char*)>(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<char, void (*)(char*)>(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<char, void (*)(char*)>(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_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);
