commit e30373edfc3c893547bd5b59c9c72e4f1652e353
author nagendra modadugu <ngm@google.com> Wed Sep 19 14:57:35 2018 -0700
committer Nagendra Modadugu <ngm@google.com> Tue Oct 09 23:58:49 2018 +0000
tree 7bc4f07c500745d83fa204ff8158a264a0c1a294
parent 8c179d347b54ac301e5bdc9e10b3e95d12f39e89

keymaster: abort operations on HAL errors

Not all error code paths in the HAL are
aborting initiated operations.  This change
tackles the error returns in attest().

This particular error case has not been encountered
in the field, but any errors during attest would
result in a similar situtation as described in the
associated bug -- operation handle leakage on Citadel.

Bug: 116055338
Test: release-tests.sh pass on PVT1
Change-Id: Icae96cc502cf16ece29c87e84c7b54c3e3e43727

hals/keymaster/KeymasterDevice.cpp

diff --git a/hals/keymaster/KeymasterDevice.cpp b/hals/keymaster/KeymasterDevice.cpp
index 63d1447..3c569a0 100644
--- a/hals/keymaster/KeymasterDevice.cpp
+++ b/hals/keymaster/KeymasterDevice.cpp
@@ -117,6 +117,17 @@
     return return_value;
 }
 
+// Helper class to call a finalizer on stack unwind.
+class Finalize {
+ private:
+    std::function<void()> f_;
+
+ public:
+    Finalize(std::function<void()> f) : f_(f) {}
+    ~Finalize() { if (f_) f_(); }
+    void release() { f_ = {}; }
+};
+
 }  // namespace
 
 // std
@@ -696,12 +707,15 @@
     uint64_t operationHandle = startResponse.handle().handle();
     ContinueAttestKeyRequest continueRequest;
     ContinueAttestKeyResponse continueResponse;
+    // Prepare to abort the pending operation in event of an error.
+    Finalize finalize([&] () { abort(operationHandle); });
 
     continueRequest.mutable_handle()->set_handle(operationHandle);
     if (hidl_params_to_pb(
             attestParams, continueRequest.mutable_params()) != ErrorCode::OK) {
-      _hidl_cb(ErrorCode::INVALID_ARGUMENT, hidl_vec<hidl_vec<uint8_t> >{});
-      return Void();
+        LOG(ERROR) << "Failed to parse attest params";
+        _hidl_cb(ErrorCode::INVALID_ARGUMENT, hidl_vec<hidl_vec<uint8_t> >{});
+        return Void();
     }
 
     KM_CALLV(ContinueAttestKey, continueRequest, continueResponse,
@@ -851,6 +865,7 @@
     }
 
     _hidl_cb(ErrorCode::OK, chain);
+    finalize.release();
     return Void();
 }