Merge "Add mode bits to open calls" am: cebf7ea40f am: 385dae25c5 am: 94f2d7653c
am: da162a77f1  -s ours

Change-Id: I37b921fcff9a5fa5104a2e59de3e1dad48c88e1d
diff --git a/Android.mk b/Android.mk
index 1b38393..1e20215 100644
--- a/Android.mk
+++ b/Android.mk
@@ -37,8 +37,6 @@
 	external/scrypt/lib/crypto \
 	frameworks/native/include \
 	system/security/keystore \
-	hardware/libhardware/include/hardware \
-	system/security/softkeymaster/include/keymaster
 
 common_shared_libraries := \
 	libsysutils \
@@ -55,9 +53,11 @@
 	libselinux \
 	libutils \
 	libhardware \
-	libsoftkeymaster \
 	libbase \
-	libkeymaster_messages \
+	libhwbinder \
+	libhidlbase \
+	android.hardware.keymaster@3.0 \
+	libkeystore_binder
 
 common_static_libraries := \
 	libbootloader_message \
@@ -149,3 +149,5 @@
 LOCAL_CONLYFLAGS := $(vold_conlyflags)
 
 include $(BUILD_EXECUTABLE)
+
+include $(LOCAL_PATH)/tests/Android.mk
diff --git a/CommandListener.cpp b/CommandListener.cpp
index b548a91..e80bdce 100644
--- a/CommandListener.cpp
+++ b/CommandListener.cpp
@@ -703,8 +703,9 @@
             PLOG(ERROR) << "Failed to stat /proc/" << pid;
             return -errno;
         }
-        if (sb.st_uid != uid) {
-            LOG(ERROR) << "Mismatch UID expected=" << uid << ", actual=" << sb.st_uid;
+        if (sb.st_uid != AID_SYSTEM) {
+            LOG(ERROR) << "Only system can mount appfuse. UID expected=" << AID_SYSTEM
+                    << ", actual=" << sb.st_uid;
             return -EPERM;
         }
     }
@@ -748,7 +749,17 @@
         if (command == "mount") {
             _exit(mountInNamespace(uid, device_fd, path));
         } else if (command == "unmount") {
-            android::vold::ForceUnmount(path);
+            // If it's just after all FD opened on mount point are closed, umount2 can fail with
+            // EBUSY. To avoid the case, specify MNT_DETACH.
+            if (umount2(path.c_str(), UMOUNT_NOFOLLOW | MNT_DETACH) != 0 &&
+                    errno != EINVAL && errno != ENOENT) {
+                PLOG(ERROR) << "Failed to unmount directory.";
+                _exit(-errno);
+            }
+            if (rmdir(path.c_str()) != 0) {
+                PLOG(ERROR) << "Failed to remove the mount directory.";
+                _exit(-errno);
+            }
             _exit(android::OK);
         } else {
             LOG(ERROR) << "Unknown appfuse command " << command;
diff --git a/Ext4Crypt.cpp b/Ext4Crypt.cpp
index 0063bef..682b34c 100644
--- a/Ext4Crypt.cpp
+++ b/Ext4Crypt.cpp
@@ -20,11 +20,14 @@
 #include "Utils.h"
 
 #include <algorithm>
+#include <chrono>
 #include <iomanip>
 #include <map>
+#include <mutex>
 #include <set>
 #include <sstream>
 #include <string>
+#include <thread>
 
 #include <dirent.h>
 #include <errno.h>
@@ -60,6 +63,8 @@
 static constexpr int FLAG_STORAGE_CE = 1 << 1;
 
 namespace {
+const std::chrono::seconds s_key_eviction_sleep_time(20);
+
 const std::string device_key_dir = std::string() + DATA_MNT_POINT + e4crypt_unencrypted_folder;
 const std::string device_key_path = device_key_dir + "/key";
 const std::string device_key_temp = device_key_dir + "/temp";
@@ -72,6 +77,10 @@
 // Some users are ephemeral, don't try to wipe their keys from disk
 std::set<userid_t> s_ephemeral_users;
 
+// Allow evictions to be cancelled.
+std::map<std::string, std::thread::id> s_desired_eviction_threads;
+std::mutex s_desired_eviction_threads_mutex;
+
 // Map user ids to key references
 std::map<userid_t, std::string> s_de_key_raw_refs;
 std::map<userid_t, std::string> s_ce_key_raw_refs;
@@ -158,6 +167,9 @@
     ext4_encryption_key ext4_key;
     if (!fill_key(key, &ext4_key)) return false;
     *raw_ref = generate_key_ref(ext4_key.raw, ext4_key.size);
+    // Ensure that no thread is waiting to evict this ref
+    std::lock_guard<std::mutex> lock(s_desired_eviction_threads_mutex);
+    s_desired_eviction_threads.erase(*raw_ref);
     auto ref = keyname(*raw_ref);
     key_serial_t device_keyring;
     if (!e4crypt_keyring(&device_keyring)) return false;
@@ -516,15 +528,66 @@
     return true;
 }
 
+static void evict_key_after_delay(const std::string raw_ref) {
+    LOG(DEBUG) << "Waiting to evict key in thread " << std::this_thread::get_id();
+    std::this_thread::sleep_for(s_key_eviction_sleep_time);
+    LOG(DEBUG) << "Done waiting to evict key in thread " << std::this_thread::get_id();
+
+    std::lock_guard<std::mutex> lock(s_desired_eviction_threads_mutex);
+    // Check the key should be evicted by this thread
+    auto search = s_desired_eviction_threads.find(raw_ref);
+    if (search == s_desired_eviction_threads.end()) {
+        LOG(DEBUG) << "Not evicting renewed-desirability key";
+        return;
+    }
+    if (search->second != std::this_thread::get_id()) {
+        LOG(DEBUG) << "We are not the thread to evict this key";
+        return;
+    }
+
+    // Remove the key from the keyring
+    auto ref = keyname(raw_ref);
+    key_serial_t device_keyring;
+    if (!e4crypt_keyring(&device_keyring)) return;
+    auto key_serial = keyctl_search(device_keyring, "logon", ref.c_str(), 0);
+    if (keyctl_revoke(key_serial) != 0) {
+        PLOG(ERROR) << "Failed to revoke key with serial " << key_serial;
+        return;
+    }
+    LOG(DEBUG) << "Revoked key with serial " << key_serial;
+}
+
+static bool evict_key(const std::string &raw_ref) {
+    // FIXME the use of a thread with delay is a work around for a kernel bug
+    std::lock_guard<std::mutex> lock(s_desired_eviction_threads_mutex);
+    std::thread t(evict_key_after_delay, raw_ref);
+    s_desired_eviction_threads[raw_ref] = t.get_id();
+    LOG(DEBUG) << "Scheduled key eviction in thread " << t.get_id();
+    t.detach();
+    return true; // Sadly no way to know if we were successful :(
+}
+
+static bool evict_ce_key(userid_t user_id) {
+    s_ce_keys.erase(user_id);
+    bool success = true;
+    std::string raw_ref;
+    // If we haven't loaded the CE key, no need to evict it.
+    if (lookup_key_ref(s_ce_key_raw_refs, user_id, &raw_ref)) {
+        success &= evict_key(raw_ref);
+    }
+    s_ce_key_raw_refs.erase(user_id);
+    return success;
+}
+
 bool e4crypt_destroy_user_key(userid_t user_id) {
     LOG(DEBUG) << "e4crypt_destroy_user_key(" << user_id << ")";
     if (!e4crypt_is_native()) {
         return true;
     }
     bool success = true;
-    s_ce_keys.erase(user_id);
     std::string raw_ref;
-    s_ce_key_raw_refs.erase(user_id);
+    success &= evict_ce_key(user_id);
+    success &= lookup_key_ref(s_de_key_raw_refs, user_id, &raw_ref) && evict_key(raw_ref);
     s_de_key_raw_refs.erase(user_id);
     auto it = s_ephemeral_users.find(user_id);
     if (it != s_ephemeral_users.end()) {
@@ -659,8 +722,9 @@
 
 // TODO: rename to 'evict' for consistency
 bool e4crypt_lock_user_key(userid_t user_id) {
+    LOG(DEBUG) << "e4crypt_lock_user_key " << user_id;
     if (e4crypt_is_native()) {
-        // TODO: remove from kernel keyring
+        return evict_ce_key(user_id);
     } else if (e4crypt_is_emulated()) {
         // When in emulation mode, we just use chmod
         if (!emulated_lock(android::vold::BuildDataSystemCePath(user_id)) ||
diff --git a/KeyStorage.cpp b/KeyStorage.cpp
index 986f403..9cb8ca0 100644
--- a/KeyStorage.cpp
+++ b/KeyStorage.cpp
@@ -23,6 +23,7 @@
 #include <vector>
 
 #include <errno.h>
+#include <stdio.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -37,7 +38,8 @@
 
 #include <hardware/hw_auth_token.h>
 
-#include <keymaster/authorization_set.h>
+#include <keystore/authorization_set.h>
+#include <keystore/keystore_hidl_support.h>
 
 extern "C" {
 
@@ -46,6 +48,7 @@
 
 namespace android {
 namespace vold {
+using namespace keystore;
 
 const KeyAuthentication kEmptyAuthentication{"", ""};
 
@@ -66,6 +69,7 @@
 static const std::string kStretchPrefix_scrypt = "scrypt ";
 static const char* kFn_encrypted_key = "encrypted_key";
 static const char* kFn_keymaster_key_blob = "keymaster_key_blob";
+static const char* kFn_keymaster_key_blob_upgraded = "keymaster_key_blob_upgraded";
 static const char* kFn_salt = "salt";
 static const char* kFn_secdiscardable = "secdiscardable";
 static const char* kFn_stretching = "stretching";
@@ -98,15 +102,15 @@
 
 static bool generateKeymasterKey(Keymaster& keymaster, const KeyAuthentication& auth,
                                  const std::string& appId, std::string* key) {
-    auto paramBuilder = keymaster::AuthorizationSetBuilder()
+    auto paramBuilder = AuthorizationSetBuilder()
                             .AesEncryptionKey(AES_KEY_BYTES * 8)
-                            .Authorization(keymaster::TAG_BLOCK_MODE, KM_MODE_GCM)
-                            .Authorization(keymaster::TAG_MIN_MAC_LENGTH, GCM_MAC_BYTES * 8)
-                            .Authorization(keymaster::TAG_PADDING, KM_PAD_NONE);
-    addStringParam(&paramBuilder, keymaster::TAG_APPLICATION_ID, appId);
+                            .Authorization(TAG_BLOCK_MODE, BlockMode::GCM)
+                            .Authorization(TAG_MIN_MAC_LENGTH, GCM_MAC_BYTES * 8)
+                            .Authorization(TAG_PADDING, PaddingMode::NONE)
+                            .Authorization(TAG_APPLICATION_ID, blob2hidlVec(appId));
     if (auth.token.empty()) {
         LOG(DEBUG) << "Creating key that doesn't need auth token";
-        paramBuilder.Authorization(keymaster::TAG_NO_AUTH_REQUIRED);
+        paramBuilder.Authorization(TAG_NO_AUTH_REQUIRED);
     } else {
         LOG(DEBUG) << "Auth token required for key";
         if (auth.token.size() != sizeof(hw_auth_token_t)) {
@@ -115,65 +119,27 @@
             return false;
         }
         const hw_auth_token_t* at = reinterpret_cast<const hw_auth_token_t*>(auth.token.data());
-        paramBuilder.Authorization(keymaster::TAG_USER_SECURE_ID, at->user_id);
-        paramBuilder.Authorization(keymaster::TAG_USER_AUTH_TYPE, HW_AUTH_PASSWORD);
-        paramBuilder.Authorization(keymaster::TAG_AUTH_TIMEOUT, AUTH_TIMEOUT);
+        paramBuilder.Authorization(TAG_USER_SECURE_ID, at->user_id);
+        paramBuilder.Authorization(TAG_USER_AUTH_TYPE, HardwareAuthenticatorType::PASSWORD);
+        paramBuilder.Authorization(TAG_AUTH_TIMEOUT, AUTH_TIMEOUT);
     }
-    return keymaster.generateKey(paramBuilder.build(), key);
+    return keymaster.generateKey(paramBuilder, key);
 }
 
-static keymaster::AuthorizationSetBuilder beginParams(const KeyAuthentication& auth,
-                                                      const std::string& appId) {
-    auto paramBuilder = keymaster::AuthorizationSetBuilder()
-                            .Authorization(keymaster::TAG_BLOCK_MODE, KM_MODE_GCM)
-                            .Authorization(keymaster::TAG_MAC_LENGTH, GCM_MAC_BYTES * 8)
-                            .Authorization(keymaster::TAG_PADDING, KM_PAD_NONE);
-    addStringParam(&paramBuilder, keymaster::TAG_APPLICATION_ID, appId);
+static AuthorizationSet beginParams(const KeyAuthentication& auth,
+                                               const std::string& appId) {
+    auto paramBuilder = AuthorizationSetBuilder()
+                            .Authorization(TAG_BLOCK_MODE, BlockMode::GCM)
+                            .Authorization(TAG_MAC_LENGTH, GCM_MAC_BYTES * 8)
+                            .Authorization(TAG_PADDING, PaddingMode::NONE)
+                            .Authorization(TAG_APPLICATION_ID, blob2hidlVec(appId));
     if (!auth.token.empty()) {
         LOG(DEBUG) << "Supplying auth token to Keymaster";
-        addStringParam(&paramBuilder, keymaster::TAG_AUTH_TOKEN, auth.token);
+        paramBuilder.Authorization(TAG_AUTH_TOKEN, blob2hidlVec(auth.token));
     }
     return paramBuilder;
 }
 
-static bool encryptWithKeymasterKey(Keymaster& keymaster, const std::string& key,
-                                    const KeyAuthentication& auth, const std::string& appId,
-                                    const std::string& message, std::string* ciphertext) {
-    auto params = beginParams(auth, appId).build();
-    keymaster::AuthorizationSet outParams;
-    auto opHandle = keymaster.begin(KM_PURPOSE_ENCRYPT, key, params, &outParams);
-    if (!opHandle) return false;
-    keymaster_blob_t nonceBlob;
-    if (!outParams.GetTagValue(keymaster::TAG_NONCE, &nonceBlob)) {
-        LOG(ERROR) << "GCM encryption but no nonce generated";
-        return false;
-    }
-    // nonceBlob here is just a pointer into existing data, must not be freed
-    std::string nonce(reinterpret_cast<const char*>(nonceBlob.data), nonceBlob.data_length);
-    if (!checkSize("nonce", nonce.size(), GCM_NONCE_BYTES)) return false;
-    std::string body;
-    if (!opHandle.updateCompletely(message, &body)) return false;
-
-    std::string mac;
-    if (!opHandle.finishWithOutput(&mac)) return false;
-    if (!checkSize("mac", mac.size(), GCM_MAC_BYTES)) return false;
-    *ciphertext = nonce + body + mac;
-    return true;
-}
-
-static bool decryptWithKeymasterKey(Keymaster& keymaster, const std::string& key,
-                                    const KeyAuthentication& auth, const std::string& appId,
-                                    const std::string& ciphertext, std::string* message) {
-    auto nonce = ciphertext.substr(0, GCM_NONCE_BYTES);
-    auto bodyAndMac = ciphertext.substr(GCM_NONCE_BYTES);
-    auto params = addStringParam(beginParams(auth, appId), keymaster::TAG_NONCE, nonce).build();
-    auto opHandle = keymaster.begin(KM_PURPOSE_DECRYPT, key, params);
-    if (!opHandle) return false;
-    if (!opHandle.updateCompletely(bodyAndMac, message)) return false;
-    if (!opHandle.finish()) return false;
-    return true;
-}
-
 static bool readFileToString(const std::string& filename, std::string* result) {
     if (!android::base::ReadFileToString(filename, result)) {
         PLOG(ERROR) << "Failed to read from " << filename;
@@ -190,6 +156,78 @@
     return true;
 }
 
+static KeymasterOperation begin(Keymaster& keymaster, const std::string& dir,
+                                KeyPurpose purpose,
+                                const AuthorizationSet &keyParams,
+                                const AuthorizationSet &opParams,
+                                AuthorizationSet* outParams) {
+    auto kmKeyPath = dir + "/" + kFn_keymaster_key_blob;
+    std::string kmKey;
+    if (!readFileToString(kmKeyPath, &kmKey)) return KeymasterOperation();
+    AuthorizationSet inParams(keyParams);
+    inParams.append(opParams.begin(), opParams.end());
+    for (;;) {
+        auto opHandle = keymaster.begin(purpose, kmKey, inParams, outParams);
+        if (opHandle) {
+            return opHandle;
+        }
+        if (opHandle.error() != ErrorCode::KEY_REQUIRES_UPGRADE) return opHandle;
+        LOG(DEBUG) << "Upgrading key: " << dir;
+        std::string newKey;
+        if (!keymaster.upgradeKey(kmKey, keyParams, &newKey)) return KeymasterOperation();
+        auto newKeyPath = dir + "/" + kFn_keymaster_key_blob_upgraded;
+        if (!writeStringToFile(newKey, newKeyPath)) return KeymasterOperation();
+        if (rename(newKeyPath.c_str(), kmKeyPath.c_str()) != 0) {
+            PLOG(ERROR) << "Unable to move upgraded key to location: " << kmKeyPath;
+            return KeymasterOperation();
+        }
+        if (!keymaster.deleteKey(kmKey)) {
+            LOG(ERROR) << "Key deletion failed during upgrade, continuing anyway: " << dir;
+        }
+        kmKey = newKey;
+        LOG(INFO) << "Key upgraded: " << dir;
+    }
+}
+
+static bool encryptWithKeymasterKey(Keymaster& keymaster, const std::string& dir,
+                                    const AuthorizationSet &keyParams,
+                                    const std::string& message, std::string* ciphertext) {
+    AuthorizationSet opParams;
+    AuthorizationSet outParams;
+    auto opHandle = begin(keymaster, dir, KeyPurpose::ENCRYPT, keyParams, opParams, &outParams);
+    if (!opHandle) return false;
+    auto nonceBlob = outParams.GetTagValue(TAG_NONCE);
+    if (!nonceBlob.isOk()) {
+        LOG(ERROR) << "GCM encryption but no nonce generated";
+        return false;
+    }
+    // nonceBlob here is just a pointer into existing data, must not be freed
+    std::string nonce(reinterpret_cast<const char*>(&nonceBlob.value()[0]), nonceBlob.value().size());
+    if (!checkSize("nonce", nonce.size(), GCM_NONCE_BYTES)) return false;
+    std::string body;
+    if (!opHandle.updateCompletely(message, &body)) return false;
+
+    std::string mac;
+    if (!opHandle.finish(&mac)) return false;
+    if (!checkSize("mac", mac.size(), GCM_MAC_BYTES)) return false;
+    *ciphertext = nonce + body + mac;
+    return true;
+}
+
+static bool decryptWithKeymasterKey(Keymaster& keymaster, const std::string& dir,
+                                    const AuthorizationSet &keyParams,
+                                    const std::string& ciphertext, std::string* message) {
+    auto nonce = ciphertext.substr(0, GCM_NONCE_BYTES);
+    auto bodyAndMac = ciphertext.substr(GCM_NONCE_BYTES);
+    auto opParams = AuthorizationSetBuilder()
+            .Authorization(TAG_NONCE, blob2hidlVec(nonce));
+    auto opHandle = begin(keymaster, dir, KeyPurpose::DECRYPT, keyParams, opParams, nullptr);
+    if (!opHandle) return false;
+    if (!opHandle.updateCompletely(bodyAndMac, message)) return false;
+    if (!opHandle.finish(nullptr)) return false;
+    return true;
+}
+
 static std::string getStretching() {
     char paramstr[PROPERTY_VALUE_MAX];
 
@@ -273,8 +311,9 @@
     std::string kmKey;
     if (!generateKeymasterKey(keymaster, auth, appId, &kmKey)) return false;
     if (!writeStringToFile(kmKey, dir + "/" + kFn_keymaster_key_blob)) return false;
+    auto keyParams = beginParams(auth, appId);
     std::string encryptedKey;
-    if (!encryptWithKeymasterKey(keymaster, kmKey, auth, appId, key, &encryptedKey)) return false;
+    if (!encryptWithKeymasterKey(keymaster, dir, keyParams, key, &encryptedKey)) return false;
     if (!writeStringToFile(encryptedKey, dir + "/" + kFn_encrypted_key)) return false;
     return true;
 }
@@ -296,13 +335,12 @@
     }
     std::string appId;
     if (!generateAppId(auth, stretching, salt, secdiscardable, &appId)) return false;
-    std::string kmKey;
-    if (!readFileToString(dir + "/" + kFn_keymaster_key_blob, &kmKey)) return false;
     std::string encryptedMessage;
     if (!readFileToString(dir + "/" + kFn_encrypted_key, &encryptedMessage)) return false;
     Keymaster keymaster;
     if (!keymaster) return false;
-    return decryptWithKeymasterKey(keymaster, kmKey, auth, appId, encryptedMessage, key);
+    auto keyParams = beginParams(auth, appId);
+    return decryptWithKeymasterKey(keymaster, dir, keyParams, encryptedMessage, key);
 }
 
 static bool deleteKey(const std::string& dir) {
diff --git a/Keymaster.cpp b/Keymaster.cpp
index d271b6a..bb99cde 100644
--- a/Keymaster.cpp
+++ b/Keymaster.cpp
@@ -17,121 +17,48 @@
 #include "Keymaster.h"
 
 #include <android-base/logging.h>
-#include <hardware/hardware.h>
-#include <hardware/keymaster1.h>
-#include <hardware/keymaster2.h>
+#include <keystore/keymaster_tags.h>
+#include <keystore/authorization_set.h>
+#include <keystore/keystore_hidl_support.h>
+
+using namespace ::keystore;
 
 namespace android {
 namespace vold {
 
-class IKeymasterDevice {
-  public:
-    IKeymasterDevice() {}
-    virtual ~IKeymasterDevice() {}
-    virtual keymaster_error_t generate_key(const keymaster_key_param_set_t* params,
-                                           keymaster_key_blob_t* key_blob) const = 0;
-    virtual keymaster_error_t delete_key(const keymaster_key_blob_t* key) const = 0;
-    virtual keymaster_error_t begin(keymaster_purpose_t purpose, const keymaster_key_blob_t* key,
-                                    const keymaster_key_param_set_t* in_params,
-                                    keymaster_key_param_set_t* out_params,
-                                    keymaster_operation_handle_t* operation_handle) const = 0;
-    virtual keymaster_error_t update(keymaster_operation_handle_t operation_handle,
-                                     const keymaster_key_param_set_t* in_params,
-                                     const keymaster_blob_t* input, size_t* input_consumed,
-                                     keymaster_key_param_set_t* out_params,
-                                     keymaster_blob_t* output) const = 0;
-    virtual keymaster_error_t finish(keymaster_operation_handle_t operation_handle,
-                                     const keymaster_key_param_set_t* in_params,
-                                     const keymaster_blob_t* signature,
-                                     keymaster_key_param_set_t* out_params,
-                                     keymaster_blob_t* output) const = 0;
-    virtual keymaster_error_t abort(keymaster_operation_handle_t operation_handle) const = 0;
-
-  protected:
-    DISALLOW_COPY_AND_ASSIGN(IKeymasterDevice);
-};
-
-template <typename T> class KeymasterDevice : public IKeymasterDevice {
-  public:
-    KeymasterDevice(T* d) : mDevice{d} {}
-    keymaster_error_t generate_key(const keymaster_key_param_set_t* params,
-                                   keymaster_key_blob_t* key_blob) const override final {
-        return mDevice->generate_key(mDevice, params, key_blob, nullptr);
-    }
-    keymaster_error_t delete_key(const keymaster_key_blob_t* key) const override final {
-        if (mDevice->delete_key == nullptr) return KM_ERROR_OK;
-        return mDevice->delete_key(mDevice, key);
-    }
-    keymaster_error_t begin(keymaster_purpose_t purpose, const keymaster_key_blob_t* key,
-                            const keymaster_key_param_set_t* in_params,
-                            keymaster_key_param_set_t* out_params,
-                            keymaster_operation_handle_t* operation_handle) const override final {
-        return mDevice->begin(mDevice, purpose, key, in_params, out_params, operation_handle);
-    }
-    keymaster_error_t update(keymaster_operation_handle_t operation_handle,
-                             const keymaster_key_param_set_t* in_params,
-                             const keymaster_blob_t* input, size_t* input_consumed,
-                             keymaster_key_param_set_t* out_params,
-                             keymaster_blob_t* output) const override final {
-        return mDevice->update(mDevice, operation_handle, in_params, input, input_consumed,
-                               out_params, output);
-    }
-    keymaster_error_t abort(keymaster_operation_handle_t operation_handle) const override final {
-        return mDevice->abort(mDevice, operation_handle);
-    }
-
-  protected:
-    T* const mDevice;
-};
-
-class Keymaster1Device : public KeymasterDevice<keymaster1_device_t> {
-  public:
-    Keymaster1Device(keymaster1_device_t* d) : KeymasterDevice<keymaster1_device_t>{d} {}
-    ~Keymaster1Device() override final { keymaster1_close(mDevice); }
-    keymaster_error_t finish(keymaster_operation_handle_t operation_handle,
-                             const keymaster_key_param_set_t* in_params,
-                             const keymaster_blob_t* signature,
-                             keymaster_key_param_set_t* out_params,
-                             keymaster_blob_t* output) const override final {
-        return mDevice->finish(mDevice, operation_handle, in_params, signature, out_params, output);
-    }
-};
-
-class Keymaster2Device : public KeymasterDevice<keymaster2_device_t> {
-  public:
-    Keymaster2Device(keymaster2_device_t* d) : KeymasterDevice<keymaster2_device_t>{d} {}
-    ~Keymaster2Device() override final { keymaster2_close(mDevice); }
-    keymaster_error_t finish(keymaster_operation_handle_t operation_handle,
-                             const keymaster_key_param_set_t* in_params,
-                             const keymaster_blob_t* signature,
-                             keymaster_key_param_set_t* out_params,
-                             keymaster_blob_t* output) const override final {
-        return mDevice->finish(mDevice, operation_handle, in_params, nullptr, signature, out_params,
-                               output);
-    }
-};
-
 KeymasterOperation::~KeymasterOperation() {
-    if (mDevice) mDevice->abort(mOpHandle);
+    if (mDevice.get()) mDevice->abort(mOpHandle);
 }
 
 bool KeymasterOperation::updateCompletely(const std::string& input, std::string* output) {
     output->clear();
     auto it = input.begin();
+    uint32_t inputConsumed;
+
+    ErrorCode km_error;
+    auto hidlCB = [&] (ErrorCode ret, uint32_t _inputConsumed,
+            const hidl_vec<KeyParameter>& /*ignored*/, const hidl_vec<uint8_t>& _output) {
+        km_error = ret;
+        if (km_error != ErrorCode::OK) return;
+        inputConsumed = _inputConsumed;
+        if (output)
+            output->append(reinterpret_cast<const char*>(&_output[0]), _output.size());
+    };
+
     while (it != input.end()) {
         size_t toRead = static_cast<size_t>(input.end() - it);
-        keymaster_blob_t inputBlob{reinterpret_cast<const uint8_t*>(&*it), toRead};
-        keymaster_blob_t outputBlob;
-        size_t inputConsumed;
-        auto error =
-            mDevice->update(mOpHandle, nullptr, &inputBlob, &inputConsumed, nullptr, &outputBlob);
-        if (error != KM_ERROR_OK) {
-            LOG(ERROR) << "update failed, code " << error;
+        auto inputBlob = blob2hidlVec(reinterpret_cast<const uint8_t*>(&*it), toRead);
+        auto error = mDevice->update(mOpHandle, hidl_vec<KeyParameter>(), inputBlob, hidlCB);
+        if (!error.isOk()) {
+            LOG(ERROR) << "update failed: " << error.description();
             mDevice = nullptr;
             return false;
         }
-        output->append(reinterpret_cast<const char*>(outputBlob.data), outputBlob.data_length);
-        free(const_cast<uint8_t*>(outputBlob.data));
+        if (km_error != ErrorCode::OK) {
+            LOG(ERROR) << "update failed, code " << uint32_t(km_error);
+            mDevice = nullptr;
+            return false;
+        }
         if (inputConsumed > toRead) {
             LOG(ERROR) << "update reported too much input consumed";
             mDevice = nullptr;
@@ -142,106 +69,116 @@
     return true;
 }
 
-bool KeymasterOperation::finish() {
-    auto error = mDevice->finish(mOpHandle, nullptr, nullptr, nullptr, nullptr);
+bool KeymasterOperation::finish(std::string* output) {
+    ErrorCode km_error;
+    auto hidlCb = [&] (ErrorCode ret, const hidl_vec<KeyParameter>& /*ignored*/,
+            const hidl_vec<uint8_t>& _output) {
+        km_error = ret;
+        if (km_error != ErrorCode::OK) return;
+        if (output)
+            output->assign(reinterpret_cast<const char*>(&_output[0]), _output.size());
+    };
+    auto error = mDevice->finish(mOpHandle, hidl_vec<KeyParameter>(), hidl_vec<uint8_t>(),
+            hidl_vec<uint8_t>(), hidlCb);
     mDevice = nullptr;
-    if (error != KM_ERROR_OK) {
-        LOG(ERROR) << "finish failed, code " << error;
+    if (!error.isOk()) {
+        LOG(ERROR) << "finish failed: " << error.description();
         return false;
     }
-    return true;
-}
-
-bool KeymasterOperation::finishWithOutput(std::string* output) {
-    keymaster_blob_t outputBlob;
-    auto error = mDevice->finish(mOpHandle, nullptr, nullptr, nullptr, &outputBlob);
-    mDevice = nullptr;
-    if (error != KM_ERROR_OK) {
-        LOG(ERROR) << "finish failed, code " << error;
+    if (km_error != ErrorCode::OK) {
+        LOG(ERROR) << "finish failed, code " << uint32_t(km_error);
         return false;
     }
-    output->assign(reinterpret_cast<const char*>(outputBlob.data), outputBlob.data_length);
-    free(const_cast<uint8_t*>(outputBlob.data));
     return true;
 }
 
 Keymaster::Keymaster() {
-    mDevice = nullptr;
-    const hw_module_t* module;
-    int ret = hw_get_module_by_class(KEYSTORE_HARDWARE_MODULE_ID, NULL, &module);
-    if (ret != 0) {
-        LOG(ERROR) << "hw_get_module_by_class returned " << ret;
-        return;
-    }
-    if (module->module_api_version == KEYMASTER_MODULE_API_VERSION_1_0) {
-        keymaster1_device_t* device;
-        ret = keymaster1_open(module, &device);
-        if (ret != 0) {
-            LOG(ERROR) << "keymaster1_open returned " << ret;
-            return;
-        }
-        mDevice = std::make_shared<Keymaster1Device>(device);
-    } else if (module->module_api_version == KEYMASTER_MODULE_API_VERSION_2_0) {
-        keymaster2_device_t* device;
-        ret = keymaster2_open(module, &device);
-        if (ret != 0) {
-            LOG(ERROR) << "keymaster2_open returned " << ret;
-            return;
-        }
-        mDevice = std::make_shared<Keymaster2Device>(device);
-    } else {
-        LOG(ERROR) << "module_api_version is " << module->module_api_version;
-        return;
-    }
+    mDevice = ::android::hardware::keymaster::V3_0::IKeymasterDevice::getService("keymaster");
 }
 
-bool Keymaster::generateKey(const keymaster::AuthorizationSet& inParams, std::string* key) {
-    keymaster_key_blob_t keyBlob;
-    auto error = mDevice->generate_key(&inParams, &keyBlob);
-    if (error != KM_ERROR_OK) {
-        LOG(ERROR) << "generate_key failed, code " << error;
+bool Keymaster::generateKey(const AuthorizationSet& inParams, std::string* key) {
+    ErrorCode km_error;
+    auto hidlCb = [&] (ErrorCode ret, const hidl_vec<uint8_t>& keyBlob,
+            const KeyCharacteristics& /*ignored*/) {
+        km_error = ret;
+        if (km_error != ErrorCode::OK) return;
+        if (key)
+            key->assign(reinterpret_cast<const char*>(&keyBlob[0]), keyBlob.size());
+    };
+
+    auto error = mDevice->generateKey(inParams.hidl_data(), hidlCb);
+    if (!error.isOk()) {
+        LOG(ERROR) << "generate_key failed: " << error.description();
         return false;
     }
-    key->assign(reinterpret_cast<const char*>(keyBlob.key_material), keyBlob.key_material_size);
-    free(const_cast<uint8_t*>(keyBlob.key_material));
+    if (km_error != ErrorCode::OK) {
+        LOG(ERROR) << "generate_key failed, code " << int32_t(km_error);
+        return false;
+    }
     return true;
 }
 
 bool Keymaster::deleteKey(const std::string& key) {
-    keymaster_key_blob_t keyBlob{reinterpret_cast<const uint8_t*>(key.data()), key.size()};
-    auto error = mDevice->delete_key(&keyBlob);
-    if (error != KM_ERROR_OK) {
-        LOG(ERROR) << "delete_key failed, code " << error;
+    auto keyBlob = blob2hidlVec(key);
+    auto error = mDevice->deleteKey(keyBlob);
+    if (!error.isOk()) {
+        LOG(ERROR) << "delete_key failed: " << error.description();
+        return false;
+    }
+    if (ErrorCode(error) != ErrorCode::OK) {
+        LOG(ERROR) << "delete_key failed, code " << uint32_t(ErrorCode(error));
         return false;
     }
     return true;
 }
 
-KeymasterOperation Keymaster::begin(keymaster_purpose_t purpose, const std::string& key,
-                                    const keymaster::AuthorizationSet& inParams,
-                                    keymaster::AuthorizationSet* outParams) {
-    keymaster_key_blob_t keyBlob{reinterpret_cast<const uint8_t*>(key.data()), key.size()};
-    keymaster_operation_handle_t mOpHandle;
-    keymaster_key_param_set_t outParams_set;
-    auto error = mDevice->begin(purpose, &keyBlob, &inParams, &outParams_set, &mOpHandle);
-    if (error != KM_ERROR_OK) {
-        LOG(ERROR) << "begin failed, code " << error;
-        return KeymasterOperation(nullptr, mOpHandle);
+bool Keymaster::upgradeKey(const std::string& oldKey, const AuthorizationSet& inParams,
+                           std::string* newKey) {
+    auto oldKeyBlob = blob2hidlVec(oldKey);
+    ErrorCode km_error;
+    auto hidlCb = [&] (ErrorCode ret, const hidl_vec<uint8_t>& upgradedKeyBlob) {
+        km_error = ret;
+        if (km_error != ErrorCode::OK) return;
+        if (newKey)
+            newKey->assign(reinterpret_cast<const char*>(&upgradedKeyBlob[0]),
+                    upgradedKeyBlob.size());
+    };
+    auto error = mDevice->upgradeKey(oldKeyBlob, inParams.hidl_data(), hidlCb);
+    if (!error.isOk()) {
+        LOG(ERROR) << "upgrade_key failed: " << error.description();
+        return false;
     }
-    outParams->Clear();
-    outParams->push_back(outParams_set);
-    keymaster_free_param_set(&outParams_set);
-    return KeymasterOperation(mDevice, mOpHandle);
+    if (km_error != ErrorCode::OK) {
+        LOG(ERROR) << "upgrade_key failed, code " << int32_t(km_error);
+        return false;
+    }
+    return true;
 }
 
-KeymasterOperation Keymaster::begin(keymaster_purpose_t purpose, const std::string& key,
-                                    const keymaster::AuthorizationSet& inParams) {
-    keymaster_key_blob_t keyBlob{reinterpret_cast<const uint8_t*>(key.data()), key.size()};
-    keymaster_operation_handle_t mOpHandle;
-    auto error = mDevice->begin(purpose, &keyBlob, &inParams, nullptr, &mOpHandle);
-    if (error != KM_ERROR_OK) {
-        LOG(ERROR) << "begin failed, code " << error;
-        return KeymasterOperation(nullptr, mOpHandle);
+KeymasterOperation Keymaster::begin(KeyPurpose purpose, const std::string& key,
+                                    const AuthorizationSet& inParams,
+                                    AuthorizationSet* outParams) {
+    auto keyBlob = blob2hidlVec(key);
+    uint64_t mOpHandle;
+    ErrorCode km_error;
+
+    auto hidlCb = [&] (ErrorCode ret, const hidl_vec<KeyParameter>& _outParams,
+            uint64_t operationHandle) {
+        km_error = ret;
+        if (km_error != ErrorCode::OK) return;
+        if (outParams)
+            *outParams = _outParams;
+        mOpHandle = operationHandle;
+    };
+
+    auto error = mDevice->begin(purpose, keyBlob, inParams.hidl_data(), hidlCb);
+    if (!error.isOk()) {
+        LOG(ERROR) << "begin failed: " << error.description();
+        return KeymasterOperation(ErrorCode::UNKNOWN_ERROR);
+    }
+    if (km_error != ErrorCode::OK) {
+        LOG(ERROR) << "begin failed, code " << uint32_t(km_error);
+        return KeymasterOperation(km_error);
     }
     return KeymasterOperation(mDevice, mOpHandle);
 }
diff --git a/Keymaster.h b/Keymaster.h
index 412110c..893a6d1 100644
--- a/Keymaster.h
+++ b/Keymaster.h
@@ -21,22 +21,21 @@
 #include <string>
 #include <utility>
 
-#include <keymaster/authorization_set.h>
+#include <android/hardware/keymaster/3.0/IKeymasterDevice.h>
+#include <keystore/authorization_set.h>
 
 namespace android {
 namespace vold {
+using ::android::hardware::keymaster::V3_0::IKeymasterDevice;
+using ::keystore::ErrorCode;
+using ::keystore::KeyPurpose;
+using ::keystore::AuthorizationSet;
 
-using namespace keymaster;
-
-// C++ wrappers to the Keymaster C interface.
+// C++ wrappers to the Keymaster hidl interface.
 // This is tailored to the needs of KeyStorage, but could be extended to be
 // a more general interface.
 
-// Class that wraps a keymaster1_device_t or keymaster2_device_t and provides methods
-// they have in common. Also closes the device on destruction.
-class IKeymasterDevice;
-
-// Wrapper for a keymaster_operation_handle_t representing an
+// Wrapper for a Keymaster operation handle representing an
 // ongoing Keymaster operation.  Aborts the operation
 // in the destructor if it is unfinished. Methods log failures
 // to LOG(ERROR).
@@ -45,25 +44,33 @@
     ~KeymasterOperation();
     // Is this instance valid? This is false if creation fails, and becomes
     // false on finish or if an update fails.
-    explicit operator bool() { return mDevice != nullptr; }
+    explicit operator bool() { return mError == ErrorCode::OK; }
+    ErrorCode error() { return mError; }
     // Call "update" repeatedly until all of the input is consumed, and
     // concatenate the output. Return true on success.
     bool updateCompletely(const std::string& input, std::string* output);
-    // Finish; pass nullptr for the "output" param.
-    bool finish();
-    // Finish and write the output to this string.
-    bool finishWithOutput(std::string* output);
+    // Finish and write the output to this string, unless pointer is null.
+    bool finish(std::string* output);
     // Move constructor
     KeymasterOperation(KeymasterOperation&& rhs) {
-        mOpHandle = std::move(rhs.mOpHandle);
         mDevice = std::move(rhs.mDevice);
+        mOpHandle = std::move(rhs.mOpHandle);
+        mError = std::move(rhs.mError);
     }
+    // Construct an object in an error state for error returns
+    KeymasterOperation()
+        : mDevice{nullptr}, mOpHandle{static_cast<uint64_t>(0)},
+          mError {ErrorCode::UNKNOWN_ERROR} {}
 
   private:
-    KeymasterOperation(std::shared_ptr<IKeymasterDevice> d, keymaster_operation_handle_t h)
-        : mDevice{d}, mOpHandle{h} {}
-    std::shared_ptr<IKeymasterDevice> mDevice;
-    keymaster_operation_handle_t mOpHandle;
+    KeymasterOperation(const sp<IKeymasterDevice>& d, uint64_t h)
+        : mDevice{d}, mOpHandle{h}, mError {ErrorCode::OK} {}
+    KeymasterOperation(ErrorCode error)
+        : mDevice{nullptr}, mOpHandle{0},
+          mError {error} {}
+    sp<IKeymasterDevice> mDevice;
+    uint64_t mOpHandle;
+    ErrorCode mError;
     DISALLOW_COPY_AND_ASSIGN(KeymasterOperation);
     friend class Keymaster;
 };
@@ -74,36 +81,23 @@
   public:
     Keymaster();
     // false if we failed to open the keymaster device.
-    explicit operator bool() { return mDevice != nullptr; }
+    explicit operator bool() { return mDevice.get() != nullptr; }
     // Generate a key in the keymaster from the given params.
     bool generateKey(const AuthorizationSet& inParams, std::string* key);
     // If the keymaster supports it, permanently delete a key.
     bool deleteKey(const std::string& key);
-    // Begin a new cryptographic operation, collecting output parameters.
-    KeymasterOperation begin(keymaster_purpose_t purpose, const std::string& key,
+    // Replace stored key blob in response to KM_ERROR_KEY_REQUIRES_UPGRADE.
+    bool upgradeKey(const std::string& oldKey, const AuthorizationSet& inParams,
+                    std::string* newKey);
+    // Begin a new cryptographic operation, collecting output parameters if pointer is non-null
+    KeymasterOperation begin(KeyPurpose purpose, const std::string& key,
                              const AuthorizationSet& inParams, AuthorizationSet* outParams);
-    // Begin a new cryptographic operation; don't collect output parameters.
-    KeymasterOperation begin(keymaster_purpose_t purpose, const std::string& key,
-                             const AuthorizationSet& inParams);
 
   private:
-    std::shared_ptr<IKeymasterDevice> mDevice;
+    sp<hardware::keymaster::V3_0::IKeymasterDevice> mDevice;
     DISALLOW_COPY_AND_ASSIGN(Keymaster);
 };
 
-template <keymaster_tag_t Tag>
-inline AuthorizationSetBuilder& addStringParam(AuthorizationSetBuilder&& params,
-                                               TypedTag<KM_BYTES, Tag> tag,
-                                               const std::string& val) {
-    return params.Authorization(tag, val.data(), val.size());
-}
-
-template <keymaster_tag_t Tag>
-inline void addStringParam(AuthorizationSetBuilder* params, TypedTag<KM_BYTES, Tag> tag,
-                           const std::string& val) {
-    params->Authorization(tag, val.data(), val.size());
-}
-
 }  // namespace vold
 }  // namespace android
 
diff --git a/Loop.cpp b/Loop.cpp
index a5863b3..1127817 100644
--- a/Loop.cpp
+++ b/Loop.cpp
@@ -47,7 +47,7 @@
         struct loop_info64 li;
         int rc;
 
-        sprintf(filename, "/dev/block/loop%d", i);
+        snprintf(filename, sizeof(filename), "/dev/block/loop%d", i);
 
         if ((fd = open(filename, O_RDWR | O_CLOEXEC)) < 0) {
             if (errno != ENOENT) {
@@ -91,7 +91,7 @@
         struct loop_info64 li;
         int rc;
 
-        sprintf(filename, "/dev/block/loop%d", i);
+        snprintf(filename, sizeof(filename), "/dev/block/loop%d", i);
 
         if ((fd = open(filename, O_RDWR | O_CLOEXEC)) < 0) {
             if (errno != ENOENT) {
@@ -137,7 +137,7 @@
         int rc;
         char *secontext = NULL;
 
-        sprintf(filename, "/dev/block/loop%d", i);
+        snprintf(filename, sizeof(filename), "/dev/block/loop%d", i);
 
         /*
          * The kernel starts us off with 8 loop nodes, but more
diff --git a/MoveTask.cpp b/MoveTask.cpp
index 0a60c4e..38cca04 100644
--- a/MoveTask.cpp
+++ b/MoveTask.cpp
@@ -29,6 +29,8 @@
 
 #define CONSTRAIN(amount, low, high) ((amount) < (low) ? (low) : ((amount) > (high) ? (high) : (amount)))
 
+#define EXEC_BLOCKING 0
+
 using android::base::StringPrintf;
 
 namespace android {
@@ -93,6 +95,9 @@
         return OK;
     }
 
+#if EXEC_BLOCKING
+    return ForkExecvp(cmd);
+#else
     pid_t pid = ForkExecvpAsync(cmd);
     if (pid == -1) return -1;
 
@@ -113,6 +118,7 @@
                 ((deltaFreeBytes * stepProgress) / expectedBytes), 0, stepProgress));
     }
     return -1;
+#endif
 }
 
 static status_t execCp(const std::string& fromPath, const std::string& toPath,
@@ -134,6 +140,9 @@
     }
     cmd.push_back(toPath.c_str());
 
+#if EXEC_BLOCKING
+    return ForkExecvp(cmd);
+#else
     pid_t pid = ForkExecvpAsync(cmd);
     if (pid == -1) return -1;
 
@@ -154,6 +163,7 @@
                 ((deltaFreeBytes * stepProgress) / expectedBytes), 0, stepProgress));
     }
     return -1;
+#endif
 }
 
 static void bringOffline(const std::shared_ptr<VolumeBase>& vol) {
diff --git a/Process.cpp b/Process.cpp
index 962a460..7dc0144 100644
--- a/Process.cpp
+++ b/Process.cpp
@@ -85,7 +85,7 @@
 
     // compute path to process's directory of open files
     char    path[PATH_MAX];
-    sprintf(path, "/proc/%d/fd", pid);
+    snprintf(path, sizeof(path), "/proc/%d/fd", pid);
     DIR *dir = opendir(path);
     if (!dir)
         return 0;
@@ -129,7 +129,7 @@
     FILE *file;
     char buffer[PATH_MAX + 100];
 
-    sprintf(buffer, "/proc/%d/maps", pid);
+    snprintf(buffer, sizeof(buffer), "/proc/%d/maps", pid);
     file = fopen(buffer, "r");
     if (!file)
         return 0;
@@ -155,7 +155,7 @@
     char    path[PATH_MAX];
     char    link[PATH_MAX];
 
-    sprintf(path, "/proc/%d/%s", pid, name);
+    snprintf(path, sizeof(path), "/proc/%d/%s", pid, name);
     if (readSymLink(path, link, sizeof(link)) && pathMatchesMountPoint(link, mountPoint)) 
         return 1;
     return 0;
diff --git a/VolumeManager.cpp b/VolumeManager.cpp
index 4a649bb..9dc250b 100644
--- a/VolumeManager.cpp
+++ b/VolumeManager.cpp
@@ -585,11 +585,17 @@
                 _exit(0);
             }
             if (TEMP_FAILURE_RETRY(mount(storageSource.c_str(), "/storage",
-                    NULL, MS_BIND | MS_REC | MS_SLAVE, NULL)) == -1) {
+                    NULL, MS_BIND | MS_REC, NULL)) == -1) {
                 PLOG(ERROR) << "Failed to mount " << storageSource << " for "
                         << de->d_name;
                 _exit(1);
             }
+            if (TEMP_FAILURE_RETRY(mount(NULL, "/storage", NULL,
+                    MS_REC | MS_SLAVE, NULL)) == -1) {
+                PLOG(ERROR) << "Failed to set MS_SLAVE to /storage for "
+                        << de->d_name;
+                _exit(1);
+            }
 
             // Mount user-specific symlink helper into place
             userid_t user_id = multiuser_get_user_id(uid);
@@ -1002,24 +1008,6 @@
 
     oldNumSec = info.st_size / 512;
 
-    unsigned long numImgSectors;
-    if (sb.c_opts & ASEC_SB_C_OPTS_EXT4)
-        numImgSectors = adjustSectorNumExt4(numSectors);
-    else
-        numImgSectors = adjustSectorNumFAT(numSectors);
-    /*
-     *  add one block for the superblock
-     */
-    SLOGD("Resizing from %lu sectors to %lu sectors", oldNumSec, numImgSectors + 1);
-    if (oldNumSec == numImgSectors + 1) {
-        SLOGW("Size unchanged; ignoring resize request");
-        return 0;
-    } else if (oldNumSec > numImgSectors + 1) {
-        SLOGE("Only growing is currently supported.");
-        close(fd);
-        return -1;
-    }
-
     /*
      * Try to read superblock.
      */
@@ -1045,10 +1033,26 @@
         return -1;
     }
 
+    unsigned long numImgSectors;
     if (!(sb.c_opts & ASEC_SB_C_OPTS_EXT4)) {
         SLOGE("Only ext4 partitions are supported for resize");
         errno = EINVAL;
         return -1;
+    } else {
+        numImgSectors = adjustSectorNumExt4(numSectors);
+    }
+
+    /*
+     *  add one block for the superblock
+     */
+    SLOGD("Resizing from %lu sectors to %lu sectors", oldNumSec, numImgSectors + 1);
+    if (oldNumSec == numImgSectors + 1) {
+        SLOGW("Size unchanged; ignoring resize request");
+        return 0;
+    } else if (oldNumSec > numImgSectors + 1) {
+        SLOGE("Only growing is currently supported.");
+        close(fd);
+        return -1;
     }
 
     if (Loop::resizeImageFile(asecFileName, numImgSectors + 1)) {
diff --git a/cryptfs.c b/cryptfs.c
index f6698f6..e2606ec 100644
--- a/cryptfs.c
+++ b/cryptfs.c
@@ -1068,6 +1068,7 @@
   struct dm_target_spec *tgt;
   char *crypt_params;
   char master_key_ascii[129]; /* Large enough to hold 512 bit key and null */
+  size_t buff_offset;
   int i;
 
   io = (struct dm_ioctl *) buffer;
@@ -1093,8 +1094,11 @@
 
   crypt_params = buffer + sizeof(struct dm_ioctl) + sizeof(struct dm_target_spec);
   convert_key_to_hex_ascii(master_key, crypt_ftr->keysize, master_key_ascii);
-  sprintf(crypt_params, "%s %s 0 %s 0 %s", crypt_ftr->crypto_type_name,
-          master_key_ascii, real_blk_name, extra_params);
+
+  buff_offset = crypt_params - buffer;
+  snprintf(crypt_params, sizeof(buffer) - buff_offset, "%s %s 0 %s 0 %s",
+           crypt_ftr->crypto_type_name, master_key_ascii, real_blk_name,
+           extra_params);
   crypt_params += strlen(crypt_params) + 1;
   crypt_params = (char *) (((unsigned long)crypt_params + 7) & ~8); /* Align to an 8 byte boundary */
   tgt->next = crypt_params - buffer;
@@ -1405,6 +1409,8 @@
       SLOGE("encrypt_master_key: crypto_scrypt failed");
     }
 
+    EVP_CIPHER_CTX_cleanup(&e_ctx);
+
     return 0;
 }
 
@@ -1448,12 +1454,14 @@
   /* Copy intermediate key if needed by params */
   if (intermediate_key && intermediate_key_size) {
     *intermediate_key = (unsigned char*) malloc(KEY_LEN_BYTES);
-    if (intermediate_key) {
+    if (*intermediate_key) {
       memcpy(*intermediate_key, ikey, KEY_LEN_BYTES);
       *intermediate_key_size = KEY_LEN_BYTES;
     }
   }
 
+  EVP_CIPHER_CTX_cleanup(&d_ctx);
+
   return 0;
 }
 
@@ -1733,11 +1741,11 @@
             return -1;
         }
 
-        property_set("vold.decrypt", "trigger_load_persist_props");
         /* Create necessary paths on /data */
         if (prep_data_fs()) {
             return -1;
         }
+        property_set("vold.decrypt", "trigger_load_persist_props");
 
         /* startup service classes main and late_start */
         property_set("vold.decrypt", "trigger_restart_framework");
@@ -1889,7 +1897,8 @@
   } else {
     /* Try mounting the file system anyway, just in case the problem's with
      * the footer, not the key. */
-    sprintf(tmp_mount_point, "%s/tmp_mnt", mount_point);
+    snprintf(tmp_mount_point, sizeof(tmp_mount_point), "%s/tmp_mnt",
+             mount_point);
     mkdir(tmp_mount_point, 0755);
     if (fs_mgr_do_mount(fstab, DATA_MNT_POINT, crypto_blkdev, tmp_mount_point)) {
       SLOGE("Error temp mounting decrypted block device\n");
diff --git a/fs/Vfat.cpp b/fs/Vfat.cpp
index 38681c9..1803c4b 100644
--- a/fs/Vfat.cpp
+++ b/fs/Vfat.cpp
@@ -139,7 +139,7 @@
     flags |= (ro ? MS_RDONLY : 0);
     flags |= (remount ? MS_REMOUNT : 0);
 
-    sprintf(mountData,
+    snprintf(mountData, sizeof(mountData),
             "utf8,uid=%d,gid=%d,fmask=%o,dmask=%o,shortname=mixed",
             ownerUid, ownerGid, permMask, permMask);
 
diff --git a/tests/Android.mk b/tests/Android.mk
index f974e7f..416e621 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -7,11 +7,7 @@
 LOCAL_C_INCLUDES := \
     system/core/fs_mgr/include
 
-LOCAL_SHARED_LIBRARIES := \
-    liblog \
-    libcrypto \
-
-LOCAL_STATIC_LIBRARIES := libvold
+LOCAL_STATIC_LIBRARIES := libselinux libvold liblog libcrypto
 LOCAL_SRC_FILES := VolumeManager_test.cpp
 LOCAL_MODULE := vold_tests
 LOCAL_MODULE_TAGS := eng tests