commit bec89993a95194ab774f6b7aaec0b5e3c0602bc6
author Janis Danisevskis <jdanis@google.com> Wed Aug 14 13:42:19 2019 -0700
committer Wenhao Wang <wenhaowang@google.com> Tue Sep 03 20:34:08 2019 +0000
tree 40dae388d875b4ad28bdbc490211c706d0fdbdb2
parent 81f35573c71f0448bb0a1d2989726a55805ff131

Fix memory leak in keystore

When operations are aborted by an app or an app dies, tokens are not
removed from the device token map. This patch moves the this map from
key_store_service to KeyStore so that it can be accessed by the
keymaster workers. It also adds calls to removeOperationDevice to the
binderDied hook of the keymaster workers as well as to keystore service
abort.

Add a call to removeOperationDevice() inside pruneOperation() function on
keystore/keymaster_worker.cpp

Bug: 139383076
Test: atest keystore_unit_tests (passed)

Change-Id: I90d4dc9d4510f4ac250022c89240a742b9e8d4b4

keystore/key_store_service.cpp

diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index 279df2d..20c787c 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -889,7 +889,7 @@
                [this, cb, dev](OperationResult result_) {
                    if (result_.resultCode.isOk() ||
                        result_.resultCode == ResponseCode::OP_AUTH_NEEDED) {
-                       addOperationDevice(result_.token, dev);
+                       mKeyStore->addOperationDevice(result_.token, dev);
                    }
                    cb->onFinished(result_);
                });
@@ -906,14 +906,14 @@
         return AIDL_RETURN(ErrorCode::INVALID_ARGUMENT);
     }
 
-    auto dev = getOperationDevice(token);
+    auto dev = mKeyStore->getOperationDevice(token);
     if (!dev) {
         return AIDL_RETURN(ErrorCode::INVALID_OPERATION_HANDLE);
     }
 
     dev->update(token, params.getParameters(), input, [this, cb, token](OperationResult result_) {
         if (!result_.resultCode.isOk()) {
-            removeOperationDevice(token);
+            mKeyStore->removeOperationDevice(token);
         }
         cb->onFinished(result_);
     });
@@ -931,7 +931,7 @@
         return AIDL_RETURN(ErrorCode::INVALID_ARGUMENT);
     }
 
-    auto dev = getOperationDevice(token);
+    auto dev = mKeyStore->getOperationDevice(token);
     if (!dev) {
         return AIDL_RETURN(ErrorCode::INVALID_OPERATION_HANDLE);
     }
@@ -939,7 +939,7 @@
     dev->finish(token, params.getParameters(), {}, signature, entropy,
                 [this, cb, token](OperationResult result_) {
                     if (!result_.resultCode.isOk()) {
-                        removeOperationDevice(token);
+                        mKeyStore->removeOperationDevice(token);
                     }
                     cb->onFinished(result_);
                 });
@@ -951,12 +951,15 @@
                               const ::android::sp<::android::IBinder>& token,
                               int32_t* _aidl_return) {
     KEYSTORE_SERVICE_LOCK;
-    auto dev = getOperationDevice(token);
+    auto dev = mKeyStore->getOperationDevice(token);
     if (!dev) {
         return AIDL_RETURN(ErrorCode::INVALID_OPERATION_HANDLE);
     }
 
-    dev->abort(token, [cb](KeyStoreServiceReturnCode rc) { cb->onFinished(rc); });
+    dev->abort(token, [this, cb, token](KeyStoreServiceReturnCode rc) {
+        mKeyStore->removeOperationDevice(token);
+        cb->onFinished(rc);
+    });
 
     return AIDL_RETURN(ResponseCode::NO_ERROR);
 }