From 25b3a6f0a42e9f3c7f793dfd06ad431106e078b7 Mon Sep 17 00:00:00 2001
From: Veerendranath Jakkam <quic_vjakkam@quicinc.com>
Date: Tue, 14 Apr 2020 22:04:39 +0530
Subject: [PATCH] Synchronize on_ring_buffer_data_callback and
 writeRingbufferFiles

on_ring_buffer_data_callback callback is invoked on getting the
corresponding driver/firmware ringbuffer logs. This callback further
stores the ringbuffer data locally in cur_buffer. While the data is
getting updated through this callback, writeRingbufferFilesInternal
can get triggered from a different context which accesses the same buffer
-cur_buffer.
Synchronization is missing between these two contexts while accessing the
buffer and this commit attempts the same.

Bug: 153970986
Test: Manual - collect the bugreport and check the ring buffer logs.
Change-Id: Id99a517f4d72872b84b48c3df75dd29743f3e9b2
---
 wifi/1.4/default/wifi_chip.cpp | 57 ++++++++++++++++++++--------------
 wifi/1.4/default/wifi_chip.h   |  2 ++
 2 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/wifi/1.4/default/wifi_chip.cpp b/wifi/1.4/default/wifi_chip.cpp
index d64dfbfd60..23dd13be66 100644
--- a/wifi/1.4/default/wifi_chip.cpp
+++ b/wifi/1.4/default/wifi_chip.cpp
@@ -1314,13 +1314,18 @@ WifiStatus WifiChip::registerDebugRingBufferCallback() {
                 LOG(ERROR) << "Error converting ring buffer status";
                 return;
             }
-            const auto& target = shared_ptr_this->ringbuffer_map_.find(name);
-            if (target != shared_ptr_this->ringbuffer_map_.end()) {
-                Ringbuffer& cur_buffer = target->second;
-                cur_buffer.append(data);
-            } else {
-                LOG(ERROR) << "Ringname " << name << " not found";
-                return;
+            {
+                std::unique_lock<std::mutex> lk(shared_ptr_this->lock_t);
+                const auto& target =
+                    shared_ptr_this->ringbuffer_map_.find(name);
+                if (target != shared_ptr_this->ringbuffer_map_.end()) {
+                    Ringbuffer& cur_buffer = target->second;
+                    cur_buffer.append(data);
+                } else {
+                    LOG(ERROR) << "Ringname " << name << " not found";
+                    return;
+                }
+                // unlock
             }
         };
     legacy_hal::wifi_error legacy_status =
@@ -1595,25 +1600,29 @@ bool WifiChip::writeRingbufferFilesInternal() {
         return false;
     }
     // write ringbuffers to file
-    for (const auto& item : ringbuffer_map_) {
-        const Ringbuffer& cur_buffer = item.second;
-        if (cur_buffer.getData().empty()) {
-            continue;
-        }
-        const std::string file_path_raw =
-            kTombstoneFolderPath + item.first + "XXXXXXXXXX";
-        const int dump_fd = mkstemp(makeCharVec(file_path_raw).data());
-        if (dump_fd == -1) {
-            PLOG(ERROR) << "create file failed";
-            return false;
-        }
-        unique_fd file_auto_closer(dump_fd);
-        for (const auto& cur_block : cur_buffer.getData()) {
-            if (write(dump_fd, cur_block.data(),
-                      sizeof(cur_block[0]) * cur_block.size()) == -1) {
-                PLOG(ERROR) << "Error writing to file";
+    {
+        std::unique_lock<std::mutex> lk(lock_t);
+        for (const auto& item : ringbuffer_map_) {
+            const Ringbuffer& cur_buffer = item.second;
+            if (cur_buffer.getData().empty()) {
+                continue;
+            }
+            const std::string file_path_raw =
+                kTombstoneFolderPath + item.first + "XXXXXXXXXX";
+            const int dump_fd = mkstemp(makeCharVec(file_path_raw).data());
+            if (dump_fd == -1) {
+                PLOG(ERROR) << "create file failed";
+                return false;
+            }
+            unique_fd file_auto_closer(dump_fd);
+            for (const auto& cur_block : cur_buffer.getData()) {
+                if (write(dump_fd, cur_block.data(),
+                          sizeof(cur_block[0]) * cur_block.size()) == -1) {
+                    PLOG(ERROR) << "Error writing to file";
+                }
             }
         }
+        // unlock
     }
     return true;
 }
diff --git a/wifi/1.4/default/wifi_chip.h b/wifi/1.4/default/wifi_chip.h
index 3323ade681..98e18bba07 100644
--- a/wifi/1.4/default/wifi_chip.h
+++ b/wifi/1.4/default/wifi_chip.h
@@ -19,6 +19,7 @@
 
 #include <list>
 #include <map>
+#include <mutex>
 
 #include <android-base/macros.h>
 #include <android/hardware/wifi/1.4/IWifiChip.h>
@@ -272,6 +273,7 @@ class WifiChip : public V1_4::IWifiChip {
     bool is_valid_;
     // Members pertaining to chip configuration.
     uint32_t current_mode_id_;
+    std::mutex lock_t;
     std::vector<IWifiChip::ChipMode> modes_;
     // The legacy ring buffer callback API has only a global callback
     // registration mechanism. Use this to check if we have already