From 07db01d9af3ac18a2be12bbf58904a6d6fb749f6 Mon Sep 17 00:00:00 2001
From: Kevin Tang <zhikait@codeaurora.org>
Date: Fri, 5 Apr 2019 19:46:29 -0700
Subject: LocIpc derefs nullptr if stopping blocking listener

LocIpcRunnable is created only for nonBlocking listeners.
But stopListening would use LocIpcRunnable not knowing if
this is a blocking or nonblocking listener. LocIpcRunnable
is no longer a shared resource between the two threads. We
use a copy of string to keep the needed socket name.

Change-Id: Ib5c57a89f630807d6d03f3fbc698a56b9ffddf4d
CRs-Fixed: 2431397
---
 utils/LocIpc.cpp | 38 ++++++++++++++++++++------------------
 utils/LocIpc.h   |  6 ++++--
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/utils/LocIpc.cpp b/utils/LocIpc.cpp
index 7347b78..4178d52 100644
--- a/utils/LocIpc.cpp
+++ b/utils/LocIpc.cpp
@@ -33,6 +33,8 @@
 #include <log_util.h>
 #include "LocIpc.h"
 
+using std::string;
+
 namespace loc_util {
 
 #ifdef LOG_TAG
@@ -47,7 +49,7 @@ namespace loc_util {
 class LocIpcRunnable : public LocRunnable {
 friend LocIpc;
 public:
-    LocIpcRunnable(LocIpc& locIpc, const std::string& ipcName)
+    LocIpcRunnable(LocIpc& locIpc, const string& ipcName)
             : mLocIpc(locIpc), mIpcName(ipcName) {}
     bool run() override {
         if (!mLocIpc.startListeningBlocking(mIpcName)) {
@@ -58,17 +60,17 @@ public:
     }
 private:
      LocIpc& mLocIpc;
-     const std::string mIpcName;
+     const string mIpcName;
 };
 
-bool LocIpc::startListeningNonBlocking(const std::string& name) {
-    mRunnable = new LocIpcRunnable(*this, name);
-    std::string threadName("LocIpc-");
+bool LocIpc::startListeningNonBlocking(const string& name) {
+    auto runnable = new LocIpcRunnable(*this, name);
+    string threadName("LocIpc-");
     threadName.append(name);
-    return mThread.start(threadName.c_str(), mRunnable);
+    return mThread.start(threadName.c_str(), runnable);
 }
 
-bool LocIpc::startListeningBlocking(const std::string& name) {
+bool LocIpc::startListeningBlocking(const string& name) {
     bool stopRequested = false;
     int fd = socket(AF_UNIX, SOCK_DGRAM, 0);
 
@@ -90,13 +92,14 @@ bool LocIpc::startListeningBlocking(const std::string& name) {
         LOC_LOGe("bind socket error. reason:%s", strerror(errno));
     } else {
         mIpcFd = fd;
+        mIpcName = name;
 
         // inform that the socket is ready to receive message
         onListenerReady();
 
         ssize_t nBytes = 0;
-        std::string msg = "";
-        std::string abort = LOC_MSG_ABORT;
+        string msg = "";
+        string abort = LOC_MSG_ABORT;
         while (1) {
             msg.resize(LOC_MSG_BUF_LEN);
             nBytes = ::recvfrom(fd, (void*)(msg.data()), msg.size(), 0, NULL, NULL);
@@ -147,20 +150,19 @@ bool LocIpc::startListeningBlocking(const std::string& name) {
 }
 
 void LocIpc::stopListening() {
-    const char *socketName = nullptr;
-
     if (mIpcFd >= 0) {
-        std::string abort = LOC_MSG_ABORT;
-        socketName = (reinterpret_cast<LocIpcRunnable *>(mRunnable))->mIpcName.c_str();
-        send(socketName, abort);
+        string abort = LOC_MSG_ABORT;
+        if (!mIpcName.empty()) {
+            send(mIpcName.c_str(), abort);
+        }
         mIpcFd = -1;
     }
-    if (mRunnable) {
-        mRunnable = nullptr;
+    if (!mIpcName.empty()) {
+        mIpcName.clear();
     }
 }
 
-bool LocIpc::send(const char name[], const std::string& data) {
+bool LocIpc::send(const char name[], const string& data) {
     return send(name, (const uint8_t*)data.c_str(), data.length());
 }
 
@@ -195,7 +197,7 @@ bool LocIpc::sendData(int fd, const sockaddr_un &addr, const uint8_t data[], uin
             result = false;
         }
     } else {
-        std::string head = LOC_MSG_HEAD;
+        string head = LOC_MSG_HEAD;
         head.append(std::to_string(length));
         if (::sendto(fd, head.c_str(), head.length(), 0,
                 (struct sockaddr*)&addr, sizeof(addr)) < 0) {
diff --git a/utils/LocIpc.h b/utils/LocIpc.h
index 87f2ff8..2dee5ff 100644
--- a/utils/LocIpc.h
+++ b/utils/LocIpc.h
@@ -37,6 +37,8 @@
 #include <sys/un.h>
 #include <LocThread.h>
 
+using std::string;
+
 namespace loc_util {
 
 class LocIpcSender;
@@ -44,7 +46,7 @@ class LocIpcSender;
 class LocIpc {
 friend LocIpcSender;
 public:
-    inline LocIpc() : mIpcFd(-1), mRunnable(nullptr) {}
+    inline LocIpc() : mIpcFd(-1) {}
     inline virtual ~LocIpc() { stopListening(); }
 
     // Listen for new messages in current thread. Calling this funciton will
@@ -94,7 +96,7 @@ private:
 
     int mIpcFd;
     LocThread mThread;
-    LocRunnable *mRunnable;
+    string mIpcName;
 };
 
 class LocIpcSender {
-- 
cgit v1.2.3