From 41ea424413c0381ef2aa15fc5bd5d4b88abd23c4 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 9 Apr 2015 11:34:03 -0700 Subject: [PATCH] More volume UUID awareness. Teach free_cache() and restorecon_data() about building per-volume paths. Also clean up restorecon_data() by using std::string when building paths. Clearer names for path building utility methods, and tests to verify. Bug: 19993667 Change-Id: Iacfbcdaa5b901cc2490bc8eba366dfdeb44f1d93 --- cmds/installd/Android.mk | 2 + cmds/installd/commands.cpp | 96 ++++++++++----------- cmds/installd/installd.cpp | 6 +- cmds/installd/installd.h | 18 ++-- cmds/installd/tests/Android.mk | 1 + cmds/installd/tests/installd_utils_test.cpp | 46 +++++----- cmds/installd/utils.cpp | 94 +++++++------------- 7 files changed, 115 insertions(+), 148 deletions(-) diff --git a/cmds/installd/Android.mk b/cmds/installd/Android.mk index c66414801..2171f4db8 100644 --- a/cmds/installd/Android.mk +++ b/cmds/installd/Android.mk @@ -14,6 +14,7 @@ LOCAL_SRC_FILES := $(common_src_files) LOCAL_CFLAGS := $(common_cflags) LOCAL_SHARED_LIBRARIES := libbase LOCAL_ADDITIONAL_DEPENDENCIES += $(LOCAL_PATH)/Android.mk +LOCAL_CLANG := true include $(BUILD_STATIC_LIBRARY) # @@ -33,4 +34,5 @@ LOCAL_SHARED_LIBRARIES := \ LOCAL_STATIC_LIBRARIES := libdiskusage LOCAL_ADDITIONAL_DEPENDENCIES += $(LOCAL_PATH)/Android.mk +LOCAL_CLANG := true include $(BUILD_EXECUTABLE) diff --git a/cmds/installd/commands.cpp b/cmds/installd/commands.cpp index 2d1965d92..ea59f5b68 100644 --- a/cmds/installd/commands.cpp +++ b/cmds/installd/commands.cpp @@ -14,14 +14,19 @@ ** limitations under the License. */ +#include "installd.h" + #include #include #include -#include "installd.h" #include #include #include #include +#include +#include + +using android::base::StringPrintf; /* Directory records that are used in execution of commands. */ dir_rec_t android_data_dir; @@ -176,26 +181,27 @@ int make_user_config(userid_t userid) return 0; } -int delete_user(userid_t userid) +int delete_user(const char *uuid, userid_t userid) { int status = 0; - char data_path[PKG_PATH_MAX]; - if ((create_user_path(data_path, userid) != 0) - || (delete_dir_contents(data_path, 1, NULL) != 0)) { + std::string data_path(create_data_user_path(uuid, userid)); + if (delete_dir_contents(data_path.c_str(), 1, NULL) != 0) { status = -1; } - char media_path[PATH_MAX]; - if ((create_user_media_path(media_path, userid) != 0) - || (delete_dir_contents(media_path, 1, NULL) != 0)) { + std::string media_path(create_data_media_path(uuid, userid)); + if (delete_dir_contents(media_path.c_str(), 1, NULL) != 0) { status = -1; } - char config_path[PATH_MAX]; - if ((create_user_config_path(config_path, userid) != 0) - || (delete_dir_contents(config_path, 1, NULL) != 0)) { - status = -1; + // Config paths only exist on internal storage + if (uuid == nullptr) { + char config_path[PATH_MAX]; + if ((create_user_config_path(config_path, userid) != 0) + || (delete_dir_contents(config_path, 1, NULL) != 0)) { + status = -1; + } } return status; @@ -235,8 +241,7 @@ int delete_code_cache(const char *uuid, const char *pkgname, userid_t userid) * also require that apps constantly modify file metadata even * when just reading from the cache, which is pretty awful. */ -// TODO: extend to know about other volumes -int free_cache(int64_t free_size) +int free_cache(const char *uuid, int64_t free_size) { cache_t* cache; int64_t avail; @@ -245,7 +250,9 @@ int free_cache(int64_t free_size) char tmpdir[PATH_MAX]; char *dirpos; - avail = data_disk_free(); + std::string data_path(create_data_path(uuid)); + + avail = data_disk_free(data_path); if (avail < 0) return -1; ALOGI("free_cache(%" PRId64 ") avail %" PRId64 "\n", free_size, avail); @@ -253,15 +260,16 @@ int free_cache(int64_t free_size) cache = start_cache_collection(); - // Collect cache files for primary user. - if (create_user_path(tmpdir, 0) == 0) { - //ALOGI("adding cache files from %s\n", tmpdir); - add_cache_files(cache, tmpdir, "cache"); + // Special case for owner on internal storage + if (uuid == nullptr) { + std::string _tmpdir(create_data_user_path(nullptr, 0)); + add_cache_files(cache, _tmpdir.c_str(), "cache"); } // Search for other users and add any cache files from them. - snprintf(tmpdir, sizeof(tmpdir), "%s%s", android_data_dir.path, - SECONDARY_USER_PREFIX); + std::string _tmpdir(create_data_path(uuid) + "/" + SECONDARY_USER_PREFIX); + strcpy(tmpdir, _tmpdir.c_str()); + dirpos = tmpdir + strlen(tmpdir); d = opendir(tmpdir); if (d != NULL) { @@ -313,10 +321,10 @@ int free_cache(int64_t free_size) closedir(d); } - clear_cache_files(cache, free_size); + clear_cache_files(data_path, cache, free_size); finish_cache_collection(cache); - return data_disk_free() >= free_size ? 0 : -1; + return data_disk_free(data_path) >= free_size ? 0 : -1; } int move_dex(const char *src, const char *dst, const char *instruction_set) @@ -1541,9 +1549,6 @@ int restorecon_data(const char* uuid __attribute__((unused)), const char* pkgNam struct dirent *entry; DIR *d; struct stat s; - char *userdir; - char *primarydir; - char *pkgdir; int ret = 0; // SELINUX_ANDROID_RESTORECON_DATADATA flag is set by libselinux. Not needed here. @@ -1554,26 +1559,20 @@ int restorecon_data(const char* uuid __attribute__((unused)), const char* pkgNam return -1; } - if (asprintf(&primarydir, "%s%s%s", android_data_dir.path, PRIMARY_USER_PREFIX, pkgName) < 0) { - return -1; - } + // Special case for owner on internal storage + if (uuid == nullptr) { + std::string path(create_package_data_path(nullptr, pkgName, 0)); - // Relabel for primary user. - if (selinux_android_restorecon_pkgdir(primarydir, seinfo, uid, flags) < 0) { - ALOGE("restorecon failed for %s: %s\n", primarydir, strerror(errno)); - ret |= -1; - } - - if (asprintf(&userdir, "%s%s", android_data_dir.path, SECONDARY_USER_PREFIX) < 0) { - free(primarydir); - return -1; + if (selinux_android_restorecon_pkgdir(path.c_str(), seinfo, uid, flags) < 0) { + PLOG(ERROR) << "restorecon failed for " << path; + ret |= -1; + } } // Relabel package directory for all secondary users. - d = opendir(userdir); + std::string userdir(create_data_path(uuid) + "/" + SECONDARY_USER_PREFIX); + d = opendir(userdir.c_str()); if (d == NULL) { - free(primarydir); - free(userdir); return -1; } @@ -1594,25 +1593,18 @@ int restorecon_data(const char* uuid __attribute__((unused)), const char* pkgNam continue; } - if (asprintf(&pkgdir, "%s%s/%s", userdir, user, pkgName) < 0) { + std::string pkgdir(StringPrintf("%s/%s/%s", userdir.c_str(), user, pkgName)); + if (stat(pkgdir.c_str(), &s) < 0) { continue; } - if (stat(pkgdir, &s) < 0) { - free(pkgdir); - continue; - } - - if (selinux_android_restorecon_pkgdir(pkgdir, seinfo, s.st_uid, flags) < 0) { - ALOGE("restorecon failed for %s: %s\n", pkgdir, strerror(errno)); + if (selinux_android_restorecon_pkgdir(pkgdir.c_str(), seinfo, s.st_uid, flags) < 0) { + PLOG(ERROR) << "restorecon failed for " << pkgdir; ret |= -1; } - free(pkgdir); } closedir(d); - free(primarydir); - free(userdir); return ret; } diff --git a/cmds/installd/installd.cpp b/cmds/installd/installd.cpp index 211b5b842..b98fd0517 100644 --- a/cmds/installd/installd.cpp +++ b/cmds/installd/installd.cpp @@ -76,7 +76,7 @@ static int do_fixuid(char **arg, char reply[REPLY_MAX] __unused) static int do_free_cache(char **arg, char reply[REPLY_MAX] __unused) /* TODO int:free_size */ { - return free_cache((int64_t)atoll(arg[0])); /* free_size */ + return free_cache(nullptr, (int64_t)atoll(arg[0])); /* free_size */ } static int do_rm_cache(char **arg, char reply[REPLY_MAX] __unused) @@ -128,7 +128,7 @@ static int do_mk_user_config(char **arg, char reply[REPLY_MAX] __unused) static int do_rm_user(char **arg, char reply[REPLY_MAX] __unused) { - return delete_user(atoi(arg[0])); /* userid */ + return delete_user(nullptr, atoi(arg[0])); /* userid */ } static int do_movefiles(char **arg __unused, char reply[REPLY_MAX] __unused) @@ -518,7 +518,7 @@ int initialize_directories() { version = 2; } - if (ensure_media_user_dirs(0) == -1) { + if (ensure_media_user_dirs(nullptr, 0) == -1) { ALOGE("Failed to setup media for user 0"); goto fail; } diff --git a/cmds/installd/installd.h b/cmds/installd/installd.h index 439594189..053de260a 100644 --- a/cmds/installd/installd.h +++ b/cmds/installd/installd.h @@ -139,6 +139,7 @@ typedef struct { /* util.c */ +// TODO: rename to create_data_user_package_path std::string create_package_data_path(const char* volume_uuid, const char* package_name, userid_t user); @@ -147,10 +148,11 @@ int create_pkg_path(char path[PKG_PATH_MAX], const char *postfix, userid_t userid); -int create_user_path(char path[PKG_PATH_MAX], - userid_t userid); +std::string create_data_path(const char* volume_uuid); -int create_user_media_path(char path[PKG_PATH_MAX], userid_t userid); +std::string create_data_user_path(const char* volume_uuid, userid_t userid); + +std::string create_data_media_path(const char* volume_uuid, userid_t userid); int create_user_config_path(char path[PKG_PATH_MAX], userid_t userid); @@ -174,13 +176,13 @@ int copy_dir_files(const char *srcname, const char *dstname, uid_t owner, gid_t int lookup_media_dir(char basepath[PATH_MAX], const char *dir); -int64_t data_disk_free(); +int64_t data_disk_free(const std::string& data_path); cache_t* start_cache_collection(); void add_cache_files(cache_t* cache, const char *basepath, const char *cachedir); -void clear_cache_files(cache_t* cache, int64_t free_size); +void clear_cache_files(const std::string& data_path, cache_t* cache, int64_t free_size); void finish_cache_collection(cache_t* cache); @@ -200,7 +202,7 @@ char *build_string2(const char *s1, const char *s2); char *build_string3(const char *s1, const char *s2, const char *s3); int ensure_dir(const char* path, mode_t mode, uid_t uid, gid_t gid); -int ensure_media_user_dirs(userid_t userid); +int ensure_media_user_dirs(const char* uuid, userid_t userid); int ensure_config_user_dirs(userid_t userid); int create_profile_file(const char *pkgname, gid_t gid); void remove_profile_file(const char *pkgname); @@ -214,7 +216,7 @@ int fix_uid(const char *uuid, const char *pkgname, uid_t uid, gid_t gid); int delete_user_data(const char *uuid, const char *pkgname, userid_t userid); int make_user_data(const char *uuid, const char *pkgname, uid_t uid, userid_t userid, const char* seinfo); int make_user_config(userid_t userid); -int delete_user(userid_t userid); +int delete_user(const char *uuid, userid_t userid); int delete_cache(const char *uuid, const char *pkgname, userid_t userid); int delete_code_cache(const char *uuid, const char *pkgname, userid_t userid); int move_dex(const char *src, const char *dst, const char *instruction_set); @@ -223,7 +225,7 @@ int protect(char *pkgname, gid_t gid); int get_size(const char *uuid, const char *pkgname, userid_t userid, const char *apkpath, const char *libdirpath, const char *fwdlock_apkpath, const char *asecpath, const char *instruction_set, int64_t *codesize, int64_t *datasize, int64_t *cachesize, int64_t *asecsize); -int free_cache(int64_t free_size); +int free_cache(const char *uuid, int64_t free_size); int dexopt(const char *apk_path, uid_t uid, bool is_public, const char *pkgName, const char *instruction_set, int dexopt_needed, bool vm_safe_mode, bool debuggable, const char* oat_dir); diff --git a/cmds/installd/tests/Android.mk b/cmds/installd/tests/Android.mk index c16375ae3..38a9f6996 100644 --- a/cmds/installd/tests/Android.mk +++ b/cmds/installd/tests/Android.mk @@ -26,5 +26,6 @@ $(foreach file,$(test_src_files), \ $(eval LOCAL_SRC_FILES := $(file)) \ $(eval LOCAL_C_INCLUDES := $(c_includes)) \ $(eval LOCAL_MODULE := $(notdir $(file:%.cpp=%))) \ + $(eval LOCAL_CLANG := true) \ $(eval include $(BUILD_NATIVE_TEST)) \ ) diff --git a/cmds/installd/tests/installd_utils_test.cpp b/cmds/installd/tests/installd_utils_test.cpp index ebf705324..4ce559d3c 100644 --- a/cmds/installd/tests/installd_utils_test.cpp +++ b/cmds/installd/tests/installd_utils_test.cpp @@ -372,26 +372,6 @@ TEST_F(UtilsTest, CreatePkgPath_SecondaryUser) { << "Package path should be in /data/user/"; } -TEST_F(UtilsTest, CreatePersonaPath_Primary) { - char path[PKG_PATH_MAX]; - - EXPECT_EQ(0, create_user_path(path, 0)) - << "Should successfully build primary user path."; - - EXPECT_STREQ("/data/data/", path) - << "Primary user should have correct path"; -} - -TEST_F(UtilsTest, CreatePersonaPath_Secondary) { - char path[PKG_PATH_MAX]; - - EXPECT_EQ(0, create_user_path(path, 1)) - << "Should successfully build primary user path."; - - EXPECT_STREQ("/data/user/1/", path) - << "Primary user should have correct path"; -} - TEST_F(UtilsTest, CreateMovePath_Primary) { char path[PKG_PATH_MAX]; @@ -469,6 +449,32 @@ TEST_F(UtilsTest, AppendAndIncrement_TooBig) { << "String should fail because it's too large to fit"; } +TEST_F(UtilsTest, CreateDataPath) { + EXPECT_EQ("/data", create_data_path(nullptr)); + EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b", + create_data_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b")); +} + +TEST_F(UtilsTest, CreateDataUserPath) { + EXPECT_EQ("/data/data", create_data_user_path(nullptr, 0)); + EXPECT_EQ("/data/user/10", create_data_user_path(nullptr, 10)); + + EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b/user/0", + create_data_user_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b", 0)); + EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b/user/10", + create_data_user_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b", 10)); +} + +TEST_F(UtilsTest, CreateDataMediaPath) { + EXPECT_EQ("/data/media/0", create_data_media_path(nullptr, 0)); + EXPECT_EQ("/data/media/10", create_data_media_path(nullptr, 10)); + + EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b/media/0", + create_data_media_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b", 0)); + EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b/media/10", + create_data_media_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b", 10)); +} + TEST_F(UtilsTest, CreatePackageDataPath) { EXPECT_EQ("/data/data/com.example", create_package_data_path(nullptr, "com.example", 0)); EXPECT_EQ("/data/user/10/com.example", create_package_data_path(nullptr, "com.example", 10)); diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp index 763cb4283..e10116eb8 100644 --- a/cmds/installd/utils.cpp +++ b/cmds/installd/utils.cpp @@ -46,21 +46,7 @@ std::string create_package_data_path(const char* volume_uuid, CHECK(is_valid_filename(package_name)); CHECK(is_valid_package_name(package_name) == 0); - if (volume_uuid == nullptr) { - if (user == 0) { - // /data/data/com.example - return StringPrintf("%sdata/%s", android_data_dir.path, package_name); - } else { - // /data/user/0/com.example - return StringPrintf("%suser/%u/%s", android_data_dir.path, user, package_name); - } - } else { - CHECK(is_valid_filename(volume_uuid)); - - // /mnt/expand/uuid/user/0/com.example - return StringPrintf("%s%s/user/%u/%s", android_mnt_expand_dir.path, - volume_uuid, user, package_name); - } + return StringPrintf("%s/%s", create_data_user_path(volume_uuid, user).c_str(), package_name); } int create_pkg_path(char path[PKG_PATH_MAX], const char *pkgname, @@ -81,55 +67,36 @@ int create_pkg_path(char path[PKG_PATH_MAX], const char *pkgname, } } +std::string create_data_path(const char* volume_uuid) { + if (volume_uuid == nullptr) { + return "/data"; + } else { + CHECK(is_valid_filename(volume_uuid)); + return StringPrintf("/mnt/expand/%s", volume_uuid); + } +} + /** * Create the path name for user data for a certain userid. - * Returns 0 on success, and -1 on failure. */ -int create_user_path(char path[PKG_PATH_MAX], - userid_t userid) -{ - size_t userid_len; - const char* userid_prefix; - if (userid == 0) { - userid_prefix = PRIMARY_USER_PREFIX; - userid_len = 0; +std::string create_data_user_path(const char* volume_uuid, userid_t userid) { + std::string data(create_data_path(volume_uuid)); + if (volume_uuid == nullptr) { + if (userid == 0) { + return StringPrintf("%s/data", data.c_str()); + } else { + return StringPrintf("%s/user/%u", data.c_str(), userid); + } } else { - userid_prefix = SECONDARY_USER_PREFIX; - userid_len = snprintf(NULL, 0, "%d/", userid); + return StringPrintf("%s/user/%u", data.c_str(), userid); } - - char *dst = path; - size_t dst_size = PKG_PATH_MAX; - - if (append_and_increment(&dst, android_data_dir.path, &dst_size) < 0 - || append_and_increment(&dst, userid_prefix, &dst_size) < 0) { - ALOGE("Error building prefix for user path"); - return -1; - } - - if (userid != 0) { - if (dst_size < userid_len + 1) { - ALOGE("Error building user path"); - return -1; - } - int ret = snprintf(dst, dst_size, "%d/", userid); - if (ret < 0 || (size_t) ret != userid_len) { - ALOGE("Error appending userid to path"); - return -1; - } - } - return 0; } /** * Create the path name for media for a certain userid. - * Returns 0 on success, and -1 on failure. */ -int create_user_media_path(char path[PATH_MAX], userid_t userid) { - if (snprintf(path, PATH_MAX, "%s%d", android_media_dir.path, userid) > PATH_MAX) { - return -1; - } - return 0; +std::string create_data_media_path(const char* volume_uuid, userid_t userid) { + return StringPrintf("%s/media/%u", create_data_path(volume_uuid).c_str(), userid); } /** @@ -459,13 +426,13 @@ int lookup_media_dir(char basepath[PATH_MAX], const char *dir) return -1; } -int64_t data_disk_free() +int64_t data_disk_free(const std::string& data_path) { struct statfs sfs; - if (statfs(android_data_dir.path, &sfs) == 0) { + if (statfs(data_path.c_str(), &sfs) == 0) { return sfs.f_bavail * sfs.f_bsize; } else { - ALOGE("Couldn't statfs %s: %s\n", android_data_dir.path, strerror(errno)); + PLOG(ERROR) << "Couldn't statfs " << data_path; return -1; } } @@ -823,7 +790,7 @@ static int cache_modtime_sort(const void *lhsP, const void *rhsP) return lhs->modTime < rhs->modTime ? -1 : (lhs->modTime > rhs->modTime ? 1 : 0); } -void clear_cache_files(cache_t* cache, int64_t free_size) +void clear_cache_files(const std::string& data_path, cache_t* cache, int64_t free_size) { size_t i; int skip = 0; @@ -848,7 +815,7 @@ void clear_cache_files(cache_t* cache, int64_t free_size) for (i=0; inumFiles; i++) { skip++; if (skip > 10) { - if (data_disk_free() > free_size) { + if (data_disk_free(data_path) > free_size) { return; } skip = 0; @@ -1090,12 +1057,9 @@ char *build_string3(const char *s1, const char *s2, const char *s3) { } /* Ensure that /data/media directories are prepared for given user. */ -int ensure_media_user_dirs(userid_t userid) { - char media_user_path[PATH_MAX]; - - // Ensure /data/media/ exists - create_user_media_path(media_user_path, userid); - if (fs_prepare_dir(media_user_path, 0770, AID_MEDIA_RW, AID_MEDIA_RW) == -1) { +int ensure_media_user_dirs(const char* uuid, userid_t userid) { + std::string media_user_path(create_data_media_path(uuid, userid)); + if (fs_prepare_dir(media_user_path.c_str(), 0770, AID_MEDIA_RW, AID_MEDIA_RW) == -1) { return -1; }