From 569649ff1d6d76f89982c391a5b0e119050250e4 Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Wed, 9 Sep 2015 12:13:00 -0700 Subject: [PATCH 1/5] Don't show UI on default encryption Bug: 22989588 Change-Id: I21403233d84031869d929c46c3c7b2ebefb3caff --- CryptCommandListener.cpp | 54 ++++++++++++++++++++++++++++++++++------ cryptfs.c | 18 ++++++-------- cryptfs.h | 4 +-- 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/CryptCommandListener.cpp b/CryptCommandListener.cpp index 173be63..3132a82 100644 --- a/CryptCommandListener.cpp +++ b/CryptCommandListener.cpp @@ -147,26 +147,66 @@ int CryptCommandListener::CryptfsCmd::runCommand(SocketClient *cli, rc = cryptfs_crypto_complete(); } else if (!strcmp(argv[1], "enablecrypto")) { const char* syntax = "Usage: cryptfs enablecrypto " - "default|password|pin|pattern [passwd]"; - if ( (argc != 4 && argc != 5) - || (strcmp(argv[2], "wipe") && strcmp(argv[2], "inplace")) ) { + "default|password|pin|pattern [passwd] [noui]"; + + // This should be replaced with a command line parser if more options + // are added + bool valid = true; + bool no_ui = false; + int type = CRYPT_TYPE_DEFAULT; + int options = 4; // Optional parameters are at this offset + if (argc < 4) { + // Minimum 4 parameters + valid = false; + } else if (strcmp(argv[2], "wipe") && strcmp(argv[2], "inplace") ) { + // Second parameter must be wipe or inplace + valid = false; + } else { + // Third parameter must be valid type + type = getType(argv[3]); + if (type == -1) { + valid = false; + } else if (type != CRYPT_TYPE_DEFAULT) { + options++; + } + } + + if (valid) { + if(argc < options) { + // Too few parameters + valid = false; + } else if (argc == options) { + // No more, done + } else if (argc == options + 1) { + // One option, must be noui + if (!strcmp(argv[options], "noui")) { + no_ui = true; + } else { + valid = false; + } + } else { + // Too many options + valid = false; + } + } + + if (!valid ) { cli->sendMsg(ResponseCode::CommandSyntaxError, syntax, false); return 0; } + dumpArgs(argc, argv, 4); int tries; for (tries = 0; tries < 2; ++tries) { - int type = getType(argv[3]); if (type == -1) { cli->sendMsg(ResponseCode::CommandSyntaxError, syntax, false); return 0; } else if (type == CRYPT_TYPE_DEFAULT) { - rc = cryptfs_enable_default(argv[2], /*allow_reboot*/false); + rc = cryptfs_enable_default(argv[2], no_ui); } else { - rc = cryptfs_enable(argv[2], type, argv[4], - /*allow_reboot*/false); + rc = cryptfs_enable(argv[2], type, argv[4], no_ui); } if (rc == 0) { diff --git a/cryptfs.c b/cryptfs.c index 1a63a5b..1828ece 100644 --- a/cryptfs.c +++ b/cryptfs.c @@ -2893,7 +2893,7 @@ static int cryptfs_enable_all_volumes(struct crypt_mnt_ftr *crypt_ftr, int how, } int cryptfs_enable_internal(char *howarg, int crypt_type, char *passwd, - int allow_reboot) + int no_ui) { int how = 0; char crypto_blkdev[MAXPATHLEN], real_blkdev[MAXPATHLEN]; @@ -2992,11 +2992,7 @@ int cryptfs_enable_internal(char *howarg, int crypt_type, char *passwd, /* Now unmount the /data partition. */ if (wait_and_unmount(DATA_MNT_POINT, false)) { - if (allow_reboot) { - goto error_shutting_down; - } else { - goto error_unencrypted; - } + goto error_unencrypted; } /* Do extra work for a better UX when doing the long inplace encryption */ @@ -3086,7 +3082,7 @@ int cryptfs_enable_internal(char *howarg, int crypt_type, char *passwd, } } - if (how == CRYPTO_ENABLE_INPLACE) { + if (how == CRYPTO_ENABLE_INPLACE && !no_ui) { /* startup service classes main and late_start */ property_set("vold.decrypt", "trigger_restart_min_framework"); SLOGD("Just triggered restart_min_framework\n"); @@ -3223,15 +3219,15 @@ error_shutting_down: return -1; } -int cryptfs_enable(char *howarg, int type, char *passwd, int allow_reboot) +int cryptfs_enable(char *howarg, int type, char *passwd, int no_ui) { - return cryptfs_enable_internal(howarg, type, passwd, allow_reboot); + return cryptfs_enable_internal(howarg, type, passwd, no_ui); } -int cryptfs_enable_default(char *howarg, int allow_reboot) +int cryptfs_enable_default(char *howarg, int no_ui) { return cryptfs_enable_internal(howarg, CRYPT_TYPE_DEFAULT, - DEFAULT_PASSWORD, allow_reboot); + DEFAULT_PASSWORD, no_ui); } int cryptfs_changepw(int crypt_type, const char *newpw) diff --git a/cryptfs.h b/cryptfs.h index 94684e2..fd6f3da 100644 --- a/cryptfs.h +++ b/cryptfs.h @@ -218,9 +218,9 @@ extern "C" { int cryptfs_check_passwd(char *pw); int cryptfs_verify_passwd(char *newpw); int cryptfs_restart(void); - int cryptfs_enable(char *flag, int type, char *passwd, int allow_reboot); + int cryptfs_enable(char *flag, int type, char *passwd, int no_ui); int cryptfs_changepw(int type, const char *newpw); - int cryptfs_enable_default(char *flag, int allow_reboot); + int cryptfs_enable_default(char *flag, int no_ui); int cryptfs_setup_ext_volume(const char* label, const char* real_blkdev, const unsigned char* key, int keysize, char* out_crypto_blkdev); int cryptfs_revert_ext_volume(const char* label); From dadcceea7a8e931eaf970d95a297d8c89a0ebe9d Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 23 Sep 2015 14:13:45 -0700 Subject: [PATCH 2/5] Clean up any/all stale partition tables. When formatting media as a public volume, we write an MBR, but we might leave a stale GPT floating around. Some devices are configured to aggressively prefer GPT when detected, even if the checksums between primary/secondary don't match. To work around this, nuke both MBR and GPT tables from the media before we lay down our new MBR. Bug: 24112219 Change-Id: Ibf1be466a6877cbab925a24db5e5dbab0613bea7 --- Disk.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Disk.cpp b/Disk.cpp index 1e76bee..d5f3e5d 100644 --- a/Disk.cpp +++ b/Disk.cpp @@ -340,10 +340,24 @@ status_t Disk::unmountAll() { } status_t Disk::partitionPublic() { + int res; + // TODO: improve this code destroyAllVolumes(); mJustPartitioned = true; + // First nuke any existing partition table + std::vector cmd; + cmd.push_back(kSgdiskPath); + cmd.push_back("--zap-all"); + cmd.push_back(mDevPath); + + // Zap sometimes returns an error when it actually succeeded, so + // just log as warning and keep rolling forward. + if ((res = ForkExecvp(cmd)) != 0) { + LOG(WARNING) << "Failed to zap; status " << res; + } + struct disk_info dinfo; memset(&dinfo, 0, sizeof(dinfo)); From 6ae8604c1006e6ba0f687680387062d4acf1e41b Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Wed, 21 Oct 2015 09:28:39 -0700 Subject: [PATCH 3/5] Don't show UI on default encryption Merge of https://googleplex-android-review.git.corp.google.com/#/c/764976 Bug: 22989588 Change-Id: I3a6b01ee80446e0955e2039f88a627d37ee6caef --- vdc.rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vdc.rc b/vdc.rc index e842869..d5483d0 100644 --- a/vdc.rc +++ b/vdc.rc @@ -6,7 +6,7 @@ service defaultcrypto /system/bin/vdc --wait cryptfs mountdefaultencrypted # encryption) or trigger_restart_min_framework (other encryption) # One shot invocation to encrypt unencrypted volumes -service encrypt /system/bin/vdc --wait cryptfs enablecrypto inplace default +service encrypt /system/bin/vdc --wait cryptfs enablecrypto inplace default noui disabled oneshot # vold will set vold.decrypt to trigger_restart_framework (default From 89f74fbf2529d708534c041d2b711af0f1feff9f Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 21 Oct 2015 12:16:12 -0700 Subject: [PATCH 4/5] Kill apps using storage through bind mounts. When unmounting an emulated volume, look for apps with open files using the final published volume path. Without this change, we were only looking at the internal paths used for runtime permissions, which apps never use directly. This meant we'd always fail to unmount the volume if apps didn't respect the EJECTING broadcast, and volume migration would end up wedged until the device rebooted. Bug: 24863778 Change-Id: Ibda484e66ab95744c304c344b226caa5b10b7e2e --- EmulatedVolume.cpp | 1 + Process.cpp | 9 ++++++--- Process.h | 2 +- Utils.cpp | 42 ++++++++++++++++++++++++++++++++---------- Utils.h | 3 +++ 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/EmulatedVolume.cpp b/EmulatedVolume.cpp index 6e440cc..80ef3e2 100644 --- a/EmulatedVolume.cpp +++ b/EmulatedVolume.cpp @@ -113,6 +113,7 @@ status_t EmulatedVolume::doUnmount() { mFusePid = 0; } + KillProcessesUsingPath(getPath()); ForceUnmount(mFuseDefault); ForceUnmount(mFuseRead); ForceUnmount(mFuseWrite); diff --git a/Process.cpp b/Process.cpp index a6f0cc6..962a460 100644 --- a/Process.cpp +++ b/Process.cpp @@ -177,13 +177,14 @@ extern "C" void vold_killProcessesWithOpenFiles(const char *path, int signal) { /* * Hunt down processes that have files open at the given mount point. */ -void Process::killProcessesWithOpenFiles(const char *path, int signal) { - DIR* dir; +int Process::killProcessesWithOpenFiles(const char *path, int signal) { + int count = 0; + DIR* dir; struct dirent* de; if (!(dir = opendir("/proc"))) { SLOGE("opendir failed (%s)", strerror(errno)); - return; + return count; } while ((de = readdir(dir))) { @@ -213,7 +214,9 @@ void Process::killProcessesWithOpenFiles(const char *path, int signal) { if (signal != 0) { SLOGW("Sending %s to process %d", strsignal(signal), pid); kill(pid, signal); + count++; } } closedir(dir); + return count; } diff --git a/Process.h b/Process.h index 81b5f18..62a9313 100644 --- a/Process.h +++ b/Process.h @@ -21,7 +21,7 @@ class Process { public: - static void killProcessesWithOpenFiles(const char *path, int signal); + static int killProcessesWithOpenFiles(const char *path, int signal); static int getPid(const char *s); static int checkSymLink(int pid, const char *path, const char *name); static int checkFileMaps(int pid, const char *path); diff --git a/Utils.cpp b/Utils.cpp index e19c9df..f352e84 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -123,35 +123,57 @@ status_t ForceUnmount(const std::string& path) { if (!umount2(cpath, UMOUNT_NOFOLLOW) || errno == EINVAL || errno == ENOENT) { return OK; } - PLOG(WARNING) << "Failed to unmount " << path; - + // Apps might still be handling eject request, so wait before + // we start sending signals sleep(5); - Process::killProcessesWithOpenFiles(cpath, SIGINT); + Process::killProcessesWithOpenFiles(cpath, SIGINT); + sleep(5); if (!umount2(cpath, UMOUNT_NOFOLLOW) || errno == EINVAL || errno == ENOENT) { return OK; } - PLOG(WARNING) << "Failed to unmount " << path; - sleep(5); Process::killProcessesWithOpenFiles(cpath, SIGTERM); - + sleep(5); if (!umount2(cpath, UMOUNT_NOFOLLOW) || errno == EINVAL || errno == ENOENT) { return OK; } - PLOG(WARNING) << "Failed to unmount " << path; - sleep(5); Process::killProcessesWithOpenFiles(cpath, SIGKILL); - + sleep(5); if (!umount2(cpath, UMOUNT_NOFOLLOW) || errno == EINVAL || errno == ENOENT) { return OK; } - PLOG(ERROR) << "Failed to unmount " << path; return -errno; } +status_t KillProcessesUsingPath(const std::string& path) { + const char* cpath = path.c_str(); + if (Process::killProcessesWithOpenFiles(cpath, SIGINT) == 0) { + return OK; + } + sleep(5); + + if (Process::killProcessesWithOpenFiles(cpath, SIGTERM) == 0) { + return OK; + } + sleep(5); + + if (Process::killProcessesWithOpenFiles(cpath, SIGKILL) == 0) { + return OK; + } + sleep(5); + + // Send SIGKILL a second time to determine if we've + // actually killed everyone with open files + if (Process::killProcessesWithOpenFiles(cpath, SIGKILL) == 0) { + return OK; + } + PLOG(ERROR) << "Failed to kill processes using " << path; + return -EBUSY; +} + status_t BindMount(const std::string& source, const std::string& target) { if (::mount(source.c_str(), target.c_str(), "", MS_BIND, NULL)) { PLOG(ERROR) << "Failed to bind mount " << source << " to " << target; diff --git a/Utils.h b/Utils.h index f33a379..228727a 100644 --- a/Utils.h +++ b/Utils.h @@ -49,6 +49,9 @@ status_t PrepareDir(const std::string& path, mode_t mode, uid_t uid, gid_t gid); /* Really unmounts the path, killing active processes along the way */ status_t ForceUnmount(const std::string& path); +/* Kills any processes using given path */ +status_t KillProcessesUsingPath(const std::string& path); + /* Creates bind mount from source to target */ status_t BindMount(const std::string& source, const std::string& target); From 2403b4d0561c756ed5102aaf6048a80c9993f6f8 Mon Sep 17 00:00:00 2001 From: Oleksiy Avramchenko Date: Thu, 1 Oct 2015 12:44:46 +0200 Subject: [PATCH 5/5] Promote free bytes calculation to 64 bits The expression otherwise overflows for large devices. It's fsblkcnt_t -> unsigned long, which is 32 bit on ARMv7. Bug: 25162062 Change-Id: I46c5e00558b7dbd6abd50fae4727396079044df2 --- Utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Utils.cpp b/Utils.cpp index f352e84..279c990 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -432,7 +432,7 @@ status_t NormalizeHex(const std::string& in, std::string& out) { uint64_t GetFreeBytes(const std::string& path) { struct statvfs sb; if (statvfs(path.c_str(), &sb) == 0) { - return sb.f_bfree * sb.f_bsize; + return (uint64_t)sb.f_bfree * sb.f_bsize; } else { return -1; }