Manager: Prevent modem reboot req. when already restarting

Fixed a race condition where reboot would be requested when a restart was
already ongoing. Also fixed potential buffer overflow in DBUS communication.

Signed-off-by: Sverre Vegge <sverre.vegge@stericsson.com>

1 files changed, 47 insertions, 42 deletions

diff --git a/u300-ril-manager.c b/u300-ril-manager.c
index c4ef018..867ab54 100644
--- a/u300-ril-manager.c
+++ b/u300-ril-manager.c
@@ -104,9 +104,8 @@ static DBusHandlerResult midSignalHandler(DBusConnection *dbcon, DBusMessage
     DBusMessageIter args;
     DBusPendingCall* pending;
     DBusMessage *dbmsg;
-    char return_buf[BUF_MID_RESPONSE_SIZE];
     UNUSED(data);
-    const char* signame = return_buf;
+    char *signame = NULL;
 
     /*
      * managerRelease is here used a state indication of QueueRunners.
@@ -123,35 +122,35 @@ static DBusHandlerResult midSignalHandler(DBusConnection *dbcon, DBusMessage
             LOGD("%s(): Argument is not string!", __func__);
         } else {
             dbus_message_iter_get_basic(&args, &signame);
-            LOGD("%s(): Got Signal with value %s", __func__, signame);
+            LOGD("%s(): Got Signal with value \"%s\"", __func__, signame);
 
             if (strncmp(signame, "on", 2) == 0) {
                 if (managerRelease) {
-                    LOGD("%s() Received unexpected \"on\" in already running "
-                         "state. Ignored...", __func__);
+                    LOGD("%s() Already running state. Signal ignored...",
+                         __func__);
                 } else {
-                    LOGD("%s() Received \"on\". Releasing queue threads..."
-                         , __func__);
+                    LOGD("%s() Releasing queueRunner threads..." , __func__);
                     releaseCommandThreads();
                 }
             } else if (strncmp(signame, "prepare_off", 11) == 0) {
                 if (managerRelease) {
-                    LOGD("%s(): Received \"prepare_off\". Unhandled..."
-                         , __func__);
-                    /* TODO: add functionality of early modemcleanup here */
+                    LOGD("%s(): Signal queueRunner threads and prepare to go "
+                         "back to initial state...", __func__);
+                    haltCommandThreads();
+                    signalCloseQueues();
                 } else {
-                    LOGD("%s(): Received \"prepare_off\". Queue threads (already) "
-                         "stopped waiting for \"on\"...", __func__);
+                    LOGD("%s(): queueRunner threads (already) stopped waiting "
+                         "for \"on\"...", __func__);
                 }
             } else if (strncmp(signame, "off", 3) == 0) {
                 if (managerRelease) {
-                    LOGD("%s(): Received \"off\". Signal queue threads and "
-                         "prepare to go back to initial state...", __func__);
+                    LOGD("%s(): Signal queueRunner threads and prepare to go "
+                         "back to initial state...", __func__);
                     haltCommandThreads();
                     signalCloseQueues();
                 } else {
-                    LOGD("%s(): Received \"off\". Queue threads (already) "
-                         "stopped waiting for \"on\"...", __func__);
+                    LOGD("%s(): queueRunner threads (already) stopped waiting "
+                         "for \"on\"...", __func__);
                 }
             } else
                 LOGD("%s(): message \"%s\" ignored.", __func__, signame);
@@ -380,8 +379,7 @@ static int requestMIDWithResponse(DBusConnection *dbcon,
                                     char *requestMethod,
                                     char *response)
 {
-    char temp[BUF_MID_RESPONSE_SIZE];
-    char *pTemp = temp;
+    char *return_val = NULL;
     int ret = 0;
 
     DBusPendingCall *pending = NULL;
@@ -426,10 +424,10 @@ static int requestMIDWithResponse(DBusConnection *dbcon,
         LOGD("%s(): Argument is not a string!", __func__);
         goto error;
     } else {
-        dbus_message_iter_get_basic(&args, &pTemp);
-        strncpy(response, pTemp, BUF_MID_RESPONSE_SIZE);
-        LOGD("%s(): Got message, pTemp:\"%s\", response: \"%s\"",
-             __func__, pTemp, response);
+        dbus_message_iter_get_basic(&args, &return_val);
+        strncpy(response, return_val, BUF_MID_RESPONSE_SIZE-1);
+        LOGD("%s(): Got message: \"%s\", response: \"%s\"",
+             __func__, return_val, response);
     }
 
     goto exit;
@@ -441,19 +439,23 @@ exit:
 }
 
 static bool queryModemOn(DBusConnection *dbcon) {
+    bool res = false;
     char responseArray[BUF_MID_RESPONSE_SIZE];
     char *pResponse = responseArray;
 
+    memset(pResponse, 0x0, BUF_MID_RESPONSE_SIZE);
+
     if (requestMIDWithResponse(dbcon, "GetState", pResponse) != 0) {
         LOGE("%s(): Failed to query state of MID.", __func__);
     } else {
         if (strncmp(pResponse, "on", 2) == 0) {
-            return true;
+            res = true;
         }
         else
             LOGD("%s(): %s returned and ignored.", __func__, pResponse);
     }
-    return false;
+
+    return res;
 }
 
 static void *dbusAndThreadRunner(void *param)
@@ -605,26 +607,29 @@ static void *managerRunner(void *param)
         char responseArray[BUF_MID_RESPONSE_SIZE];
         char *pResponse = responseArray;
 
-        /* Signal MID to restart Modem */
-        if (requestMIDWithResponse(dbcon, "Reboot", pResponse) != 0) {
-            LOGE("%s(): Failed to reboot modem."
-                    "Restarting threads anyway.", __func__);
-        } else {
-            if (strncmp(pResponse, "OK", 2) == 0) {
-                LOGI("%s(): %s returned. Modem restarting!",
+        if (managerRelease) {
+            /* Signal MID to restart Modem */
+            if (requestMIDWithResponse(dbcon, "Reboot", pResponse) == 0) {
+                if (strncmp(pResponse, "OK", 2) == 0) {
+                    LOGI("%s(): MID \"reboot\" request returned %s. "
+                         "Modem restarting!", __func__, pResponse);
+                    /* Instruct QueueRunners to wait for 'on' from MID */
+                    haltCommandThreads();
+                } else {
+                    /*
+                    * In the event we are not allowed to do a modem reboot we
+                    * have little to do but try a direct restart of the
+                    * queuerunners. AT channels will anyway be re-opened.
+                    */
+                    LOGE("%s(): MID \"reboot\" request returned %s. "
+                        "Continuing letting queue runners execute immediatly.",
                         __func__, pResponse);
-                /* Instruct QueueRunners to wait for 'on' from MID */
-                haltCommandThreads();
+                }
+            } else {
+                LOGE("%s(): Failed to reboot modem."
+                     "Continuing letting queue runners execute immediatly.",
+                     __func__);
             }
-            else
-                /*
-                 * In the event we are not allowed to do a modem reboot we have
-                 * little to do but try a direct restart of the queuerunners.
-                 * AT channels will anyway be re-opened.
-                 */
-                LOGD("%s(): %s returned from MID on \"reboot\" request. "
-                     "Continuing... (letting queue runners execute immediatly)",
-                     __func__, pResponse);
         }
 #endif /* EXTERNAL_MODEM_CONTROL_MODULE_DISABLED */
     }