From 8d6d98992a164ee82dbe423564aee792b27940a3 Mon Sep 17 00:00:00 2001 From: Ganesh Patil <7030871503ganeshpatil@gmail.com> Date: Fri, 27 Mar 2026 11:01:39 +0530 Subject: [PATCH 1/2] Fix: Replace hardcoded 256-byte shared memory buffer with 4096-byte SHM_SIZE constant (#514) The C++ shared memory implementation in concore.hpp and concoredocker.hpp allocated a fixed 256-byte buffer for inter-node data exchange and silently truncated any payload exceeding 255 characters. Changes: - Add static constexpr SHM_SIZE = 4096 in both concore.hpp and concoredocker.hpp - Replace all hardcoded 256 in shmget(), strnlen(), and strncpy() with SHM_SIZE - Add overflow detection: stderr error message when payload exceeds SHM_SIZE - Add explicit null termination after strncpy() to prevent read overruns - Buffer now supports ~200+ double values (up from ~25) Fixes #514 --- concore.hpp | 26 ++++++++++++++++++++------ concoredocker.hpp | 18 +++++++++++++----- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/concore.hpp b/concore.hpp index eb88108..9e98115 100644 --- a/concore.hpp +++ b/concore.hpp @@ -40,6 +40,8 @@ class Concore{ string inpath = "./in"; string outpath = "./out"; + static constexpr size_t SHM_SIZE = 4096; + int shmId_create = -1; int shmId_get = -1; @@ -259,7 +261,7 @@ class Concore{ */ void createSharedMemory(key_t key) { - shmId_create = shmget(key, 256, IPC_CREAT | 0666); + shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666); if (shmId_create == -1) { std::cerr << "Failed to create shared memory segment." << std::endl; @@ -284,7 +286,7 @@ class Concore{ const int MAX_RETRY = 100; while (retry < MAX_RETRY) { // Get the shared memory segment created by Writer - shmId_get = shmget(key, 256, 0666); + shmId_get = shmget(key, SHM_SIZE, 0666); // Check if shared memory exists if (shmId_get != -1) { break; // Break the loop if shared memory exists @@ -490,7 +492,7 @@ class Concore{ try { if (shmId_get != -1) { if (sharedData_get && sharedData_get[0] != '\0') { - std::string message(sharedData_get, strnlen(sharedData_get, 256)); + std::string message(sharedData_get, strnlen(sharedData_get, SHM_SIZE)); ins = message; } else @@ -515,7 +517,7 @@ class Concore{ this_thread::sleep_for(timespan); try{ if(shmId_get != -1) { - std::string message(sharedData_get, strnlen(sharedData_get, 256)); + std::string message(sharedData_get, strnlen(sharedData_get, SHM_SIZE)); ins = message; retrycount++; } @@ -664,7 +666,13 @@ class Concore{ outfile<= SHM_SIZE) { + std::cerr << "ERROR: write_SM payload (" << result.size() + << " bytes) exceeds " << SHM_SIZE - 1 + << "-byte shared memory limit. Data truncated!" << std::endl; + } + std::strncpy(sharedData_create, result.c_str(), SHM_SIZE - 1); + sharedData_create[SHM_SIZE - 1] = '\0'; // simtime must not be mutated here (issue #385). } else{ @@ -689,7 +697,13 @@ class Concore{ this_thread::sleep_for(timespan); try { if(shmId_create != -1){ - std::strncpy(sharedData_create, val.c_str(), 256 - 1); + if (val.size() >= SHM_SIZE) { + std::cerr << "ERROR: write_SM payload (" << val.size() + << " bytes) exceeds " << SHM_SIZE - 1 + << "-byte shared memory limit. Data truncated!" << std::endl; + } + std::strncpy(sharedData_create, val.c_str(), SHM_SIZE - 1); + sharedData_create[SHM_SIZE - 1] = '\0'; } else throw 505; } diff --git a/concoredocker.hpp b/concoredocker.hpp index 51d6ca5..1218aae 100644 --- a/concoredocker.hpp +++ b/concoredocker.hpp @@ -26,6 +26,8 @@ class Concore { private: + static constexpr size_t SHM_SIZE = 4096; + int shmId_create = -1; int shmId_get = -1; char* sharedData_create = nullptr; @@ -233,7 +235,7 @@ class Concore { #ifdef __linux__ void createSharedMemory(key_t key) { - shmId_create = shmget(key, 256, IPC_CREAT | 0666); + shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666); if (shmId_create == -1) { std::cerr << "Failed to create shared memory segment.\n"; return; @@ -249,7 +251,7 @@ class Concore { int retry = 0; const int MAX_RETRY = 100; while (retry < MAX_RETRY) { - shmId_get = shmget(key, 256, 0666); + shmId_get = shmget(key, SHM_SIZE, 0666); if (shmId_get != -1) break; std::cout << "Shared memory does not exist. Make sure the writer process is running.\n"; @@ -345,7 +347,7 @@ class Concore { std::string ins; try { if (shmId_get != -1 && sharedData_get && sharedData_get[0] != '\0') - ins = std::string(sharedData_get, strnlen(sharedData_get, 256)); + ins = std::string(sharedData_get, strnlen(sharedData_get, SHM_SIZE)); else throw 505; } catch (...) { @@ -359,7 +361,7 @@ class Concore { std::this_thread::sleep_for(std::chrono::seconds(delay)); try { if (shmId_get != -1 && sharedData_get) { - ins = std::string(sharedData_get, strnlen(sharedData_get, 256)); + ins = std::string(sharedData_get, strnlen(sharedData_get, SHM_SIZE)); retrycount++; } else { retrycount++; @@ -426,7 +428,13 @@ class Concore { outfile << val[i] << ','; outfile << val[val.size() - 1] << ']'; std::string result = outfile.str(); - std::strncpy(sharedData_create, result.c_str(), 256 - 1); + if (result.size() >= SHM_SIZE) { + std::cerr << "ERROR: write_SM payload (" << result.size() + << " bytes) exceeds " << SHM_SIZE - 1 + << "-byte shared memory limit. Data truncated!" << std::endl; + } + std::strncpy(sharedData_create, result.c_str(), SHM_SIZE - 1); + sharedData_create[SHM_SIZE - 1] = '\0'; } catch (...) { std::cerr << "skipping +" << outpath << port << "/" << name << "\n"; } From 0af338c0cc2c3b7fab1cd2d09db69fef1bf3ab9d Mon Sep 17 00:00:00 2001 From: Ganesh Patil <7030871503ganeshpatil@gmail.com> Date: Fri, 27 Mar 2026 11:57:09 +0530 Subject: [PATCH 2/2] Fix: Add nullptr guard for sharedData_create and shmctl segment size verification Address Copilot review feedback: - Add sharedData_create != nullptr check before strncpy/null-termination in all write_SM() overloads to prevent null pointer dereference when shmat() fails in createSharedMemory() - Add shmctl(IPC_STAT) verification in createSharedMemory() to detect stale segments smaller than SHM_SIZE (shmget won't resize existing segments); removes and recreates the segment when too small - Add early return in concore.hpp createSharedMemory() on shmget failure (was missing, unlike concoredocker.hpp which already had it) --- concore.hpp | 18 ++++++++++++++++++ concoredocker.hpp | 16 ++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/concore.hpp b/concore.hpp index 9e98115..e3fb5e1 100644 --- a/concore.hpp +++ b/concore.hpp @@ -265,6 +265,20 @@ class Concore{ if (shmId_create == -1) { std::cerr << "Failed to create shared memory segment." << std::endl; + return; + } + + // Verify the segment is large enough (shmget won't resize an existing segment) + struct shmid_ds shm_info; + if (shmctl(shmId_create, IPC_STAT, &shm_info) == 0 && shm_info.shm_segsz < SHM_SIZE) { + std::cerr << "Shared memory segment too small (" << shm_info.shm_segsz + << " bytes, need " << SHM_SIZE << "). Removing and recreating." << std::endl; + shmctl(shmId_create, IPC_RMID, nullptr); + shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666); + if (shmId_create == -1) { + std::cerr << "Failed to recreate shared memory segment." << std::endl; + return; + } } // Attach the shared memory segment to the process's address space @@ -660,6 +674,8 @@ class Concore{ try { std::ostringstream outfile; if(shmId_create != -1){ + if (sharedData_create == nullptr) + throw 506; val.insert(val.begin(),simtime+delta); outfile<<'['; for(int i=0;i= SHM_SIZE) { std::cerr << "ERROR: write_SM payload (" << val.size() << " bytes) exceeds " << SHM_SIZE - 1 diff --git a/concoredocker.hpp b/concoredocker.hpp index 1218aae..d9c9a1e 100644 --- a/concoredocker.hpp +++ b/concoredocker.hpp @@ -240,6 +240,20 @@ class Concore { std::cerr << "Failed to create shared memory segment.\n"; return; } + + // Verify the segment is large enough (shmget won't resize an existing segment) + struct shmid_ds shm_info; + if (shmctl(shmId_create, IPC_STAT, &shm_info) == 0 && shm_info.shm_segsz < SHM_SIZE) { + std::cerr << "Shared memory segment too small (" << shm_info.shm_segsz + << " bytes, need " << SHM_SIZE << "). Removing and recreating.\n"; + shmctl(shmId_create, IPC_RMID, nullptr); + shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666); + if (shmId_create == -1) { + std::cerr << "Failed to recreate shared memory segment.\n"; + return; + } + } + sharedData_create = static_cast(shmat(shmId_create, NULL, 0)); if (sharedData_create == reinterpret_cast(-1)) { std::cerr << "Failed to attach shared memory segment.\n"; @@ -421,6 +435,8 @@ class Concore { try { if (shmId_create == -1) throw 505; + if (sharedData_create == nullptr) + throw 506; val.insert(val.begin(), simtime + delta); std::ostringstream outfile; outfile << '[';