From daa63284184176c447f344c75272e89ef07cfebf Mon Sep 17 00:00:00 2001 From: "Dave Walker (Daviey)" Date: Sun, 11 Feb 2024 17:52:36 +0000 Subject: [PATCH] Fix segfault in HTTP API rebind Previously with HTTP API enabled on brenchmarking run, it is possible to cause a segfault due to an issue handling the m_httpd pointer and rebinding. - Initialize m_httpd to nullptr to indicate when it's not in use. - Safely delete m_httpd in Api's destructor to prevent use-after-free issues. - Add checks to ensure m_httpd is not nullptr before usage in start, stop, and tick methods. - Log errors for HTTP server start failures to aid in debugging. Fixes MoneroOcean/xmrig#120 Signed-off-by: Dave Walker (Daviey) --- src/base/api/Api.cpp | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/base/api/Api.cpp b/src/base/api/Api.cpp index ea78c35ed..e0907a84a 100644 --- a/src/base/api/Api.cpp +++ b/src/base/api/Api.cpp @@ -39,6 +39,7 @@ #include +#include namespace xmrig { @@ -80,7 +81,8 @@ static rapidjson::Value getResources(rapidjson::Document &doc) xmrig::Api::Api(Base *base) : m_base(base), - m_timestamp(Chrono::currentMSecsSinceEpoch()) + m_timestamp(Chrono::currentMSecsSinceEpoch()), + m_httpd(nullptr) { base->addListener(this); @@ -91,7 +93,11 @@ xmrig::Api::Api(Base *base) : xmrig::Api::~Api() { # ifdef XMRIG_FEATURE_HTTP - delete m_httpd; + if (m_httpd) { + m_httpd->stop(); + delete m_httpd; + m_httpd = nullptr; // Ensure the pointer is set to nullptr after deletion + } # endif } @@ -109,8 +115,14 @@ void xmrig::Api::start() genWorkerId(m_base->config()->apiWorkerId()); # ifdef XMRIG_FEATURE_HTTP - m_httpd = new Httpd(m_base); - m_httpd->start(); + if (!m_httpd) { + m_httpd = new Httpd(m_base); + if (!m_httpd->start()) { + std::cerr << "HTTP server failed to start." << std::endl; + delete m_httpd; // Properly handle failure to start + m_httpd = nullptr; + } + } # endif } @@ -118,7 +130,9 @@ void xmrig::Api::start() void xmrig::Api::stop() { # ifdef XMRIG_FEATURE_HTTP - m_httpd->stop(); + if (m_httpd) { + m_httpd->stop(); + } # endif } @@ -126,13 +140,15 @@ void xmrig::Api::stop() void xmrig::Api::tick() { # ifdef XMRIG_FEATURE_HTTP - if (m_httpd->isBound() || !m_base->config()->http().isEnabled()) { + if (!m_httpd || !m_base->config()->http().isEnabled() || m_httpd->isBound()) { return; } if (++m_ticks % 10 == 0) { m_ticks = 0; - m_httpd->start(); + if (m_httpd) { + m_httpd->start(); + } } # endif }