From 9c41221206e0d57f702c5dd5eb1e133a21afb736 Mon Sep 17 00:00:00 2001
From: Janis Danisevskis <jdanis@google.com>
Date: Fri, 9 Nov 2018 07:51:56 -0800
Subject: [PATCH] Removed unsafe use of hidl_vec<>.setToExternal

hidl_vec objects that do not own their associated buffer are highly
unsafe in multithreaded environments where move semantic is used to
transfer ownership between threads. With keystore transitioning to a
multi threaded execution model we can no longer use this optimization
safely.

Bug: 111443219
Test: Ran full keystore cts test suite.
Change-Id: I9a366fc7df5dfee508dc092855545963ef6d9665
---
 .../3.0/vts/functional/authorization_set.h    |  7 +++----
 .../include/keymasterV4_0/authorization_set.h |  7 +++----
 .../include/keymasterV4_0/keymaster_utils.h   | 20 +++++++------------
 3 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/keymaster/3.0/vts/functional/authorization_set.h b/keymaster/3.0/vts/functional/authorization_set.h
index 5f92d816a1..60b00e4320 100644
--- a/keymaster/3.0/vts/functional/authorization_set.h
+++ b/keymaster/3.0/vts/functional/authorization_set.h
@@ -201,7 +201,7 @@ class AuthorizationSet {
     void push_back(TypedTag<TagType::BYTES, tag> ttag, const uint8_t* data, size_t data_length) {
         hidl_vec<uint8_t> new_blob;
         new_blob.setToExternal(const_cast<uint8_t*>(data), data_length);
-        push_back(ttag, std::move(new_blob));
+        push_back(ttag, new_blob);
     }
 
     /**
@@ -225,8 +225,7 @@ class AuthorizationSet {
     }
 
     hidl_vec<KeyParameter> hidl_data() const {
-        hidl_vec<KeyParameter> result;
-        result.setToExternal(const_cast<KeyParameter*>(data()), size());
+        hidl_vec<KeyParameter> result(begin(), end());
         return result;
     }
 
@@ -252,7 +251,7 @@ class AuthorizationSetBuilder : public AuthorizationSet {
                                            size_t data_length) {
         hidl_vec<uint8_t> new_blob;
         new_blob.setToExternal(const_cast<uint8_t*>(data), data_length);
-        push_back(ttag, std::move(new_blob));
+        push_back(ttag, new_blob);
         return *this;
     }
 
diff --git a/keymaster/4.0/support/include/keymasterV4_0/authorization_set.h b/keymaster/4.0/support/include/keymasterV4_0/authorization_set.h
index ac96c863ae..a131423f6a 100644
--- a/keymaster/4.0/support/include/keymasterV4_0/authorization_set.h
+++ b/keymaster/4.0/support/include/keymasterV4_0/authorization_set.h
@@ -214,9 +214,8 @@ class AuthorizationSet {
         }
     }
 
-    const hidl_vec<KeyParameter> hidl_data() const {
-        hidl_vec<KeyParameter> result;
-        result.setToExternal(const_cast<KeyParameter*>(data()), size());
+    hidl_vec<KeyParameter> hidl_data() const {
+        hidl_vec<KeyParameter> result(begin(), end());
         return result;
     }
 
@@ -242,7 +241,7 @@ class AuthorizationSetBuilder : public AuthorizationSet {
                                            size_t data_length) {
         hidl_vec<uint8_t> new_blob;
         new_blob.setToExternal(const_cast<uint8_t*>(data), data_length);
-        push_back(ttag, std::move(new_blob));
+        push_back(ttag, new_blob);
         return *this;
     }
 
diff --git a/keymaster/4.0/support/include/keymasterV4_0/keymaster_utils.h b/keymaster/4.0/support/include/keymasterV4_0/keymaster_utils.h
index 90a0f1b29f..5e5ae8d0ed 100644
--- a/keymaster/4.0/support/include/keymasterV4_0/keymaster_utils.h
+++ b/keymaster/4.0/support/include/keymasterV4_0/keymaster_utils.h
@@ -33,25 +33,19 @@ bool operator<(const HmacSharingParameters& a, const HmacSharingParameters& b);
 
 namespace support {
 
-inline static hidl_vec<uint8_t> blob2hidlVec(const uint8_t* data, const size_t length,
-                                             bool inPlace = true) {
-    hidl_vec<uint8_t> result;
-    result.setToExternal(const_cast<unsigned char*>(data), length, !inPlace);
+inline static hidl_vec<uint8_t> blob2hidlVec(const uint8_t* data, const size_t length) {
+    hidl_vec<uint8_t> result(data, data + length);
     return result;
 }
 
-inline static hidl_vec<uint8_t> blob2hidlVec(const std::string& value, bool inPlace = true) {
-    hidl_vec<uint8_t> result;
-    result.setToExternal(const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(value.data())),
-                         static_cast<size_t>(value.size()), !inPlace);
+inline static hidl_vec<uint8_t> blob2hidlVec(const std::string& value) {
+    hidl_vec<uint8_t> result(reinterpret_cast<const uint8_t*>(value.data()),
+                             reinterpret_cast<const uint8_t*>(value.data()) + value.size());
     return result;
 }
 
-inline static hidl_vec<uint8_t> blob2hidlVec(const std::vector<uint8_t>& blob,
-                                             bool inPlace = true) {
-    hidl_vec<uint8_t> result;
-    result.setToExternal(const_cast<uint8_t*>(blob.data()), static_cast<size_t>(blob.size()),
-                         !inPlace);
+inline static hidl_vec<uint8_t> blob2hidlVec(const std::vector<uint8_t>& blob) {
+    hidl_vec<uint8_t> result(blob.data(), blob.data() + static_cast<size_t>(blob.size()));
     return result;
 }