Merge changes I096b7c98,I1d7f46cc am: 50e9c3a684 am: 772a5baddf
am: ed825a0ef9
Change-Id: I50dce009c4104ecf77cadedc9842c33e95b7bd03
diff --git a/keystore/KeyStore.cpp b/keystore/KeyStore.cpp
index 7530243..ac3ab5f 100644
--- a/keystore/KeyStore.cpp
+++ b/keystore/KeyStore.cpp
@@ -138,21 +138,13 @@
android::String8 prefix("");
android::Vector<android::String16> aliases;
- // DO NOT
- // move
- // auto userState = userStateDB_.getUserState(userId);
- // here, in an attempt to replace userStateDB_.getUserState(userId) with userState.
- // userState is a proxy that holds a lock which may required by a worker.
- // LockedKeyBlobEntry::list has a fence that waits until all workers have finished which may
- // not happen if a user state lock is held. The following line only briefly grabs the lock.
- // Grabbing the user state lock after the list call is also save since workers cannot grab
- // blob entry locks.
-
auto userState = mUserStateDB.getUserState(userId);
std::string userDirName = userState->getUserDirName();
auto encryptionKey = userState->getEncryptionKey();
auto state = userState->getState();
- // unlock the user state
+ // userState is a proxy that holds a lock which may be required by a worker.
+ // LockedKeyBlobEntry::list has a fence that waits until all workers have finished which may
+ // not happen if a user state lock is held. The following line relinquishes the lock.
userState = {};
ResponseCode rc;
@@ -217,7 +209,7 @@
bool KeyStore::isEmpty(uid_t userId) const {
std::string userDirName;
{
- // userState hold a lock which must be relinqhished before list is called. This scope
+ // userState holds a lock which must be relinquished before list is called. This scope
// prevents deadlocks.
auto userState = mUserStateDB.getUserState(userId);
if (!userState) {
diff --git a/keystore/blob.h b/keystore/blob.h
index 5cd1b90..a7f9fd0 100644
--- a/keystore/blob.h
+++ b/keystore/blob.h
@@ -196,10 +196,10 @@
bool hasCharacteristicsBlob() const;
bool operator<(const KeyBlobEntry& rhs) const {
- return std::tie(alias_, user_dir_, uid_) < std::tie(rhs.alias_, rhs.user_dir_, uid_);
+ return std::tie(uid_, alias_, user_dir_) < std::tie(rhs.uid_, rhs.alias_, rhs.user_dir_);
}
bool operator==(const KeyBlobEntry& rhs) const {
- return std::tie(alias_, user_dir_, uid_) == std::tie(rhs.alias_, rhs.user_dir_, uid_);
+ return std::tie(uid_, alias_, user_dir_) == std::tie(rhs.uid_, rhs.alias_, rhs.user_dir_);
}
bool operator!=(const KeyBlobEntry& rhs) const { return !(*this == rhs); }
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index 24bf331..35530e1 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -256,10 +256,10 @@
ResponseCode rc;
std::list<LockedKeyBlobEntry> internal_matches;
+ auto userDirName = mKeyStore->getUserStateDB().getUserStateByUid(targetUid)->getUserDirName();
- std::tie(rc, internal_matches) = LockedKeyBlobEntry::list(
- mKeyStore->getUserStateDB().getUserStateByUid(targetUid)->getUserDirName(),
- [&](uid_t uid, const std::string& alias) {
+ std::tie(rc, internal_matches) =
+ LockedKeyBlobEntry::list(userDirName, [&](uid_t uid, const std::string& alias) {
std::mismatch(stdPrefix.begin(), stdPrefix.end(), alias.begin(), alias.end());
return uid == static_cast<uid_t>(targetUid) &&
std::mismatch(stdPrefix.begin(), stdPrefix.end(), alias.begin(), alias.end())
@@ -582,11 +582,10 @@
return Status::ok();
}
-Status KeyStoreService::clear_uid(int64_t targetUid64, int32_t* aidl_return) {
+Status KeyStoreService::clear_uid(int64_t targetUid64, int32_t* _aidl_return) {
uid_t targetUid = getEffectiveUid(targetUid64);
if (!checkBinderPermissionSelfOrSystem(P_CLEAR_UID, targetUid)) {
- *aidl_return = static_cast<int32_t>(ResponseCode::PERMISSION_DENIED);
- return Status::ok();
+ return AIDL_RETURN(ResponseCode::PERMISSION_DENIED);
}
ALOGI("clear_uid %" PRId64, targetUid64);
@@ -594,16 +593,15 @@
ResponseCode rc;
std::list<LockedKeyBlobEntry> entries;
+ auto userDirName = mKeyStore->getUserStateDB().getUserStateByUid(targetUid)->getUserDirName();
// list has a fence making sure no workers are modifying blob files before iterating the
// data base. All returned entries are locked.
std::tie(rc, entries) = LockedKeyBlobEntry::list(
- mKeyStore->getUserStateDB().getUserStateByUid(targetUid)->getUserDirName(),
- [&](uid_t uid, const std::string&) -> bool { return uid == targetUid; });
+ userDirName, [&](uid_t uid, const std::string&) -> bool { return uid == targetUid; });
if (rc != ResponseCode::NO_ERROR) {
- *aidl_return = static_cast<int32_t>(rc);
- return Status::ok();
+ return AIDL_RETURN(rc);
}
for (LockedKeyBlobEntry& lockedEntry : entries) {
@@ -618,8 +616,7 @@
}
mKeyStore->del(lockedEntry);
}
- *aidl_return = static_cast<int32_t>(ResponseCode::NO_ERROR);
- return Status::ok();
+ return AIDL_RETURN(ResponseCode::NO_ERROR);
}
Status KeyStoreService::addRngEntropy(
@@ -821,7 +818,7 @@
std::tie(rc, keyBlob, charBlob, lockedEntry) =
mKeyStore->getKeyForName(name8, targetUid, TYPE_KEYMASTER_10);
- if (!rc) {
+ if (!rc.isOk()) {
return AIDL_RETURN(rc);
}
diff --git a/keystore/keymaster_worker.cpp b/keystore/keymaster_worker.cpp
index f3bf71f..6dc055f 100644
--- a/keystore/keymaster_worker.cpp
+++ b/keystore/keymaster_worker.cpp
@@ -88,6 +88,8 @@
std::tie(rc, blob, charBlob) =
lockedEntry.readBlobs(userState->getEncryptionKey(), userState->getState());
+ userState = {};
+
if (rc != ResponseCode::NO_ERROR) {
return error = rc, result;
}