From 9d4d881cc5cd5e225f68c85050d99d60445edbcf Mon Sep 17 00:00:00 2001
From: Greg Kaiser <gkaiser@google.com>
Date: Wed, 17 Aug 2016 16:27:26 -0700
Subject: [PATCH] ContextHubService: Avoid uninitialized handle

We avoid ever using uninitialized stack for the value of a handle
which we pass up to the Java layer.

In cleaning up this code a little, we make more explicit a failure
case which already existed: The Context Hub properly loading a
nanoapp, but our Service code failing to set up the infrastructure
to track it.  We chose to tell the Java layer we failed in this
case, and more importantly, provide a consistent handle value.

Note that the INVALID_APP_ID is not known by the Java layer, but
it is consistent so future Java code could react to it.  At
the very least we will now always have consistent behavior when
this situation happens.

Bug:30795236
Change-Id: Id4eada529aa1b223867a47985ef1d5c1ba346ea3
---
 ...android_hardware_location_ContextHubService.cpp | 30 +++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/core/jni/android_hardware_location_ContextHubService.cpp b/core/jni/android_hardware_location_ContextHubService.cpp
index 1cc7c25c5bf7..0d28271fa262 100644
--- a/core/jni/android_hardware_location_ContextHubService.cpp
+++ b/core/jni/android_hardware_location_ContextHubService.cpp
@@ -35,6 +35,7 @@
 #include "core_jni_helpers.h"
 
 static constexpr jint OS_APP_ID = -1;
+static constexpr jint INVALID_APP_ID = -2;
 static constexpr uint64_t ALL_APPS = UINT64_C(0xFFFFFFFFFFFFFFFF);
 
 static constexpr jint MIN_APP_ID = 1;
@@ -663,7 +664,7 @@ void closeUnloadTxn(bool success) {
     closeTxn();
 }
 
-void closeLoadTxn(bool success, jint *appInstanceHandle) {
+static bool closeLoadTxn(bool success, jint *appInstanceHandle) {
     void *txnData;
     hub_messages_e txnId;
 
@@ -677,13 +678,17 @@ void closeLoadTxn(bool success, jint *appInstanceHandle) {
             add_app_instance(&info->appInfo, info->hubHandle, info->instanceId, env);
         } else {
             ALOGW("Could not attach to JVM !");
+            success = false;
         }
         sendQueryForApps(info->appInfo.app_name.id);
     } else {
         ALOGW("Could not load the app successfully ! Unexpected failure");
+        *appInstanceHandle = INVALID_APP_ID;
+        success = false;
     }
 
     closeTxn();
+    return success;
 }
 
 static bool isValidOsStatus(const uint8_t *msg, size_t msgLen,
@@ -740,8 +745,27 @@ static int handle_os_message(uint32_t msgType, uint32_t hubHandle,
       case CONTEXT_HUB_UNLOAD_APP:
           if (isValidOsStatus(msg, msgLen, &rsp)) {
               if (msgType == CONTEXT_HUB_LOAD_APP) {
-                  jint appInstanceHandle;
-                  closeLoadTxn(rsp.result == 0, &appInstanceHandle);
+                  jint appInstanceHandle = INVALID_APP_ID;
+                  bool appRunningOnHub = (rsp.result == 0);
+                  if (!(closeLoadTxn(appRunningOnHub, &appInstanceHandle))) {
+                      if (appRunningOnHub) {
+                          // Now we're in an odd situation.  Our nanoapp
+                          // is up and running on the Context Hub.  However,
+                          // something went wrong in our Service code so that
+                          // we're not able to properly track this nanoapp
+                          // in our Service code.  If we tell the Java layer
+                          // things are good, it's a lie because the handle
+                          // we give them will fail when used with the Service.
+                          // If we tell the Java layer this failed, it's kind
+                          // of a lie as well, since this nanoapp is running.
+                          //
+                          // We leave a more robust fix for later, and for
+                          // now just tell the user things have failed.
+                          //
+                          // TODO(b/30835981): Make this situation better.
+                          rsp.result = -1;
+                      }
+                  }
                   passOnOsResponse(hubHandle, msgType, &rsp, (int8_t *)(&appInstanceHandle),
                                    sizeof(appInstanceHandle));
               } else if (msgType == CONTEXT_HUB_UNLOAD_APP) {
-- 
2.11.0