commit 8e73b9064fdd414b9a71806a44f0e0f176b5a88d
author Xinnan Xie <xiexinnan@bytedance.com> Wed Aug 23 11:14:48 2023 +0800
committer Ed Tanous <ed@tanous.net> Thu Aug 24 16:21:32 2023 +0000
tree a85f7d8f7eaf21dd8b7db99ebd18e57466e5735a
parent e7c2991e044d2e31f911a237b7ee41197b7d018c

kvm_websocket: Fix crash on dangling reference

Kvm_websocket captures the this pointer in the handler lambda of the
socket. When the callback is called, if the object has been destructed,
there will be a crash problem. This is fixed by using weak_from_this in
the callback, if the object was destructed, the callback just returns
without doing anything.

Tested:
1. Open two kvm sessions in WebUI, and keep refreshing in one of the
pages, there is a small chance of coredump happening.
Debug infomation shows:
```
bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470
bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470
bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470
bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470
bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470
bmcweb[5331]: DEBUG: onclose. conn:0x28d19a0
bmcweb[5331]: DEBUG: doRead. conn:0x2876648. this: 0x284d470
systemd[1]: bmeweb.service: Main process exited, code=dumped, status=11/SEGV
systemd[1]: bmcweb.service: Failed with result 'core-dump
systemd[1]: Started Start bmweb server.
```
2. After this fix no coredump occurred.

Change-Id: I7bba9b67c470def90ddb1e471a0ac95edd6165e5
Signed-off-by: Xinnan Xie <xiexinnan@bytedance.com>

include/kvm_websocket.hpp

diff --git a/include/kvm_websocket.hpp b/include/kvm_websocket.hpp
index e3131bb..d04c19f 100644
--- a/include/kvm_websocket.hpp
+++ b/include/kvm_websocket.hpp
@@ -14,7 +14,7 @@
 
 static constexpr const uint maxSessions = 4;
 
-class KvmSession
+class KvmSession : public std::enable_shared_from_this<KvmSession>
 {
   public:
     explicit KvmSession(crow::websocket::Connection& connIn) :
@@ -71,7 +71,13 @@
                          bytes);
         hostSocket.async_read_some(
             outputBuffer.prepare(outputBuffer.capacity() - outputBuffer.size()),
-            [this](const boost::system::error_code& ec, std::size_t bytesRead) {
+            [this, weak(weak_from_this())](const boost::system::error_code& ec,
+                                           std::size_t bytesRead) {
+            auto self = weak.lock();
+            if (self == nullptr)
+            {
+                return;
+            }
             BMCWEB_LOG_DEBUG("conn:{}, read done.  Read {} bytes",
                              logPtr(&conn), bytesRead);
             if (ec)
@@ -115,9 +121,15 @@
         }
 
         doingWrite = true;
-        hostSocket.async_write_some(inputBuffer.data(),
-                                    [this](const boost::system::error_code& ec,
+        hostSocket.async_write_some(
+            inputBuffer.data(),
+            [this, weak(weak_from_this())](const boost::system::error_code& ec,
                                            std::size_t bytesWritten) {
+            auto self = weak.lock();
+            if (self == nullptr)
+            {
+                return;
+            }
             BMCWEB_LOG_DEBUG("conn:{}, Wrote {}bytes", logPtr(&conn),
                              bytesWritten);
             doingWrite = false;
@@ -140,7 +152,7 @@
             }
 
             doWrite();
-        });
+            });
     }
 
     crow::websocket::Connection& conn;
@@ -151,7 +163,7 @@
 };
 
 using SessionMap = boost::container::flat_map<crow::websocket::Connection*,
-                                              std::unique_ptr<KvmSession>>;
+                                              std::shared_ptr<KvmSession>>;
 // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 static SessionMap sessions;
 
@@ -171,7 +183,7 @@
                 return;
             }
 
-            sessions[&conn] = std::make_unique<KvmSession>(conn);
+            sessions[&conn] = std::make_shared<KvmSession>(conn);
         })
         .onclose([](crow::websocket::Connection& conn, const std::string&) {
             sessions.erase(&conn);