Fix: Replace hardcoded 256-byte shared memory buffer with 4096-byte#516
Merged
pradeeban merged 2 commits intoControlCore-Project:devfrom Mar 28, 2026
Merged
Conversation
…HM_SIZE constant (ControlCore-Project#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 ControlCore-Project#514
There was a problem hiding this comment.
Pull request overview
This PR fixes silent data truncation in the Linux shared-memory transport used by the C++ Concore implementations by replacing the hardcoded 256-byte buffer limit with a named 4096-byte constant and adding explicit overflow signaling.
Changes:
- Introduces
SHM_SIZE = 4096and replaces previous hardcoded256usages inshmget(),strnlen(), andstrncpy(). - Adds overflow detection in
write_SM()that emits anstderrerror when the payload exceeds the shared-memory capacity. - Adds explicit null termination after
strncpy()to ensure safe reads.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| concore.hpp | Uses SHM_SIZE for SHM allocation/read/write; adds overflow warning + explicit null termination on SHM writes. |
| concoredocker.hpp | Same SHM sizing + overflow warning + explicit null termination for Docker SHM path. |
Comments suppressed due to low confidence (2)
concoredocker.hpp:243
- shmget(key, SHM_SIZE, IPC_CREAT|0666) will not resize an existing shared-memory segment for the same key; it will return the existing segment even if it was created with a smaller size. That can defeat the intention of increasing the buffer after upgrades or after a crash leaves the segment behind. Consider checking shm_segsz via shmctl(IPC_STAT) and recreating or failing with a clear error if the segment is smaller than SHM_SIZE.
shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666);
if (shmId_create == -1) {
std::cerr << "Failed to create shared memory segment.\n";
return;
}
sharedData_create = static_cast<char*>(shmat(shmId_create, NULL, 0));
concore.hpp:272
- In createSharedMemory(), if shmget() fails (shmId_create == -1) the code still calls shmat(shmId_create, ...). This can end up calling shmat(-1) and masking the real failure, and later writes may still proceed with shmId_create set to -1/invalid. Return early (or throw) when shmget() fails before attempting shmat().
shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666);
if (shmId_create == -1) {
std::cerr << "Failed to create shared memory segment." << std::endl;
}
// Attach the shared memory segment to the process's address space
sharedData_create = static_cast<char*>(shmat(shmId_create, NULL, 0));
if (sharedData_create == reinterpret_cast<char*>(-1)) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The C++ shared memory implementation in
concore.hppandconcoredocker.hppallocates a fixed 256-byte buffer for inter-node data exchange and silently truncates any payload exceeding 255 characters with no warning, no error, and no log output. Downstream nodes receive incomplete data and produce wrong results.Closes #514
Root Cause
Three hardcoded
256values acrossshmget(),strncpy(), andstrnlen()calls limited the shared memory buffer to 256 bytes. The wire format[simtime, val1, val2, ..., valN]uses ~8–18 characters per double, so payloads with 25+ values silently overflow. This is a critical data corruption bug for realistic neuromodulation studies with 32+ sensor channels (EEG, multi-electrode arrays).Changes
In both
concore.hppandconcoredocker.hpp:static constexpr size_t SHM_SIZE = 4096replaces all hardcoded256with a single named constant. Buffer now supports ~200+ double values (up from ~25).256in:shmget(key, 256, ...)→shmget(key, SHM_SIZE, ...)strnlen(sharedData_get, 256)→strnlen(sharedData_get, SHM_SIZE)strncpy(sharedData_create, ..., 256 - 1)→strncpy(sharedData_create, ..., SHM_SIZE - 1)write_SM()prints a clearstderrerror message when payload exceedsSHM_SIZE, instead of silently truncating.sharedData_create[SHM_SIZE - 1] = '\0'after everystrncpy()to prevent read overruns (defense-in-depth).Affected Files
concore.hppSHM_SIZEconstant + 7 replacements + 2 overflow checks + 2 null terminatorsconcoredocker.hppSHM_SIZEconstant + 4 replacements + 1 overflow check + 1 null terminatorTesting
pytest tests/ -v→ 164 passed)#ifdef __linux__guarded, compiles cleanlyBefore / After