Merge "Strictly check for SELinux labelling errors" am: 5554b80afb

Original change: https://android-review.googlesource.com/c/platform/system/vold/+/2456274

Change-Id: I7c4ab5c3fdfbda22b24a8875915f3acfef1b8003
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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 @@ std::string GetFuseMountPathForUser(userid_t user_id, const std::string& relativ
 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 (selabel_lookup(sehandle, &tmp_secontext, cpath, S_IFBLK) != 0) {
-    if (sehandle) {
+        PLOG(ERROR) << "Failed to look up selabel for device node " << path;
-        if (!selabel_lookup(sehandle, &secontext, cpath, S_IFBLK)) {
+        return -errno;
-            setfscreatecon(secontext);
     }
+    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;
         }
     }
-
+    return OK;
-    if (secontext) {
-        setfscreatecon(nullptr);
-        freecon(secontext);
-    }
-
-    return res;
 }
 
 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<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 (selabel_lookup(sehandle, &tmp_secontext, cpath, S_IFDIR) != 0) {
-    if (sehandle) {
+        PLOG(ERROR) << "Failed to look up selabel for directory " << path;
-        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 {
         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) {

main.cpp

@@ -82,9 +82,11 @@ int main(int argc, char** argv) {
     parse_args(argc, argv);
 
     sehandle = selinux_android_file_context_handle();
-    if (sehandle) {
+    if (!sehandle) {
-        selinux_android_set_sehandle(sehandle);
+        LOG(ERROR) << "Failed to get SELinux file contexts handle";
+        exit(1);
     }
+    selinux_android_set_sehandle(sehandle);
 
     mkdir("/dev/block/vold", 0755);
 

vold_prepare_subdirs.cpp

@@ -58,10 +58,12 @@ 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<char, void (*)(char*)>(nullptr, freecon);
-    if (sehandle) {
     char* tmp_secontext;
 
-        if (selabel_lookup(sehandle, &tmp_secontext, path.c_str(), S_IFDIR) == 0) {
+    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) {
@@ -70,24 +72,18 @@ static bool prepare_dir_for_user(struct selabel_handle* sehandle, mode_t mode, u
             PLOG(ERROR) << "Unable to create context with level for: " << path;
             return false;
         }
-                secontext.reset(tmp_secontext);  // Free the context
+        secontext.reset(tmp_secontext);
-            }
-        }
     }
 
     LOG(DEBUG) << "Setting up mode " << std::oct << mode << std::dec << " uid " << uid << " gid "
-               << gid << " context " << (secontext ? secontext.get() : "null")
+               << gid << " context " << secontext.get() << " on path: " << path;
-               << " on path: " << path;
-    if (secontext) {
     if (setfscreatecon(secontext.get()) != 0) {
-            PLOG(ERROR) << "Unable to setfscreatecon for: " << path;
+        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;
@@ -102,7 +98,6 @@ static bool prepare_dir_for_user(struct selabel_handle* sehandle, mode_t mode, u
             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);