Fix keystore wifi concurrency issue.
Keystore was conceptually single threaded. Even with the introduction of
Keymaster workers it was always assumed that the service dispatcher
thread was single threaded. The wifi keystore service, however, calls
into the keystore service concurrently.
This patch adds a lock around all keystore service entry points to make
sure all dispatcher executions are serialised despite being called from
both the binder and hwbinder service thread.
Bug: 128810613
Bug: 129145334
Bug: 128774635
Test: Regressions tested with Keystore CTS test suite.
Change-Id: I8c5602d2c2cb1dd9423df713037e99b247cee71f
diff --git a/keystore/key_store_service.h b/keystore/key_store_service.h
index 2171213..2fdc3dd 100644
--- a/keystore/key_store_service.h
+++ b/keystore/key_store_service.h
@@ -35,6 +35,8 @@
#include <keystore/OperationResult.h>
#include <keystore/keystore_return_types.h>
+#include <mutex>
+
namespace keystore {
// Class provides implementation for generated BnKeystoreService.h based on
@@ -230,6 +232,17 @@
sp<KeyStore> mKeyStore;
+ /**
+ * This mutex locks keystore operations from concurrent execution.
+ * The keystore service has always been conceptually single threaded. Even with the introduction
+ * of keymaster workers, it was assumed that the dispatcher thread executes exclusively on
+ * certain code paths. With the introduction of wifi Keystore service in the keystore process
+ * this assumption no longer holds as the hwbinder thread servicing this interface makes
+ * functions (rather than IPC) calls into keystore. This mutex protects the keystore logic
+ * from concurrent execution.
+ */
+ std::mutex keystoreServiceMutex_;
+
std::mutex operationDeviceMapMutex_;
std::map<sp<IBinder>, std::shared_ptr<KeymasterWorker>> operationDeviceMap_;