Skip to content

Commit 3f8fe0f

Browse files
authored
Merge pull request #516 from GaneshPatil7517/fix/shm-dynamic-allocation-514
Fix: Replace hardcoded 256-byte shared memory buffer with 4096-byte
2 parents 2b37f9c + 0af338c commit 3f8fe0f

2 files changed

Lines changed: 67 additions & 11 deletions

File tree

concore.hpp

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ class Concore{
4040
string inpath = "./in";
4141
string outpath = "./out";
4242

43+
static constexpr size_t SHM_SIZE = 4096;
44+
4345
int shmId_create = -1;
4446
int shmId_get = -1;
4547

@@ -259,10 +261,24 @@ class Concore{
259261
*/
260262
void createSharedMemory(key_t key)
261263
{
262-
shmId_create = shmget(key, 256, IPC_CREAT | 0666);
264+
shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666);
263265

264266
if (shmId_create == -1) {
265267
std::cerr << "Failed to create shared memory segment." << std::endl;
268+
return;
269+
}
270+
271+
// Verify the segment is large enough (shmget won't resize an existing segment)
272+
struct shmid_ds shm_info;
273+
if (shmctl(shmId_create, IPC_STAT, &shm_info) == 0 && shm_info.shm_segsz < SHM_SIZE) {
274+
std::cerr << "Shared memory segment too small (" << shm_info.shm_segsz
275+
<< " bytes, need " << SHM_SIZE << "). Removing and recreating." << std::endl;
276+
shmctl(shmId_create, IPC_RMID, nullptr);
277+
shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666);
278+
if (shmId_create == -1) {
279+
std::cerr << "Failed to recreate shared memory segment." << std::endl;
280+
return;
281+
}
266282
}
267283

268284
// Attach the shared memory segment to the process's address space
@@ -284,7 +300,7 @@ class Concore{
284300
const int MAX_RETRY = 100;
285301
while (retry < MAX_RETRY) {
286302
// Get the shared memory segment created by Writer
287-
shmId_get = shmget(key, 256, 0666);
303+
shmId_get = shmget(key, SHM_SIZE, 0666);
288304
// Check if shared memory exists
289305
if (shmId_get != -1) {
290306
break; // Break the loop if shared memory exists
@@ -490,7 +506,7 @@ class Concore{
490506
try {
491507
if (shmId_get != -1) {
492508
if (sharedData_get && sharedData_get[0] != '\0') {
493-
std::string message(sharedData_get, strnlen(sharedData_get, 256));
509+
std::string message(sharedData_get, strnlen(sharedData_get, SHM_SIZE));
494510
ins = message;
495511
}
496512
else
@@ -515,7 +531,7 @@ class Concore{
515531
this_thread::sleep_for(timespan);
516532
try{
517533
if(shmId_get != -1) {
518-
std::string message(sharedData_get, strnlen(sharedData_get, 256));
534+
std::string message(sharedData_get, strnlen(sharedData_get, SHM_SIZE));
519535
ins = message;
520536
retrycount++;
521537
}
@@ -658,13 +674,21 @@ class Concore{
658674
try {
659675
std::ostringstream outfile;
660676
if(shmId_create != -1){
677+
if (sharedData_create == nullptr)
678+
throw 506;
661679
val.insert(val.begin(),simtime+delta);
662680
outfile<<'[';
663681
for(int i=0;i<val.size()-1;i++)
664682
outfile<<val[i]<<',';
665683
outfile<<val[val.size()-1]<<']';
666684
std::string result = outfile.str();
667-
std::strncpy(sharedData_create, result.c_str(), 256 - 1);
685+
if (result.size() >= SHM_SIZE) {
686+
std::cerr << "ERROR: write_SM payload (" << result.size()
687+
<< " bytes) exceeds " << SHM_SIZE - 1
688+
<< "-byte shared memory limit. Data truncated!" << std::endl;
689+
}
690+
std::strncpy(sharedData_create, result.c_str(), SHM_SIZE - 1);
691+
sharedData_create[SHM_SIZE - 1] = '\0';
668692
// simtime must not be mutated here (issue #385).
669693
}
670694
else{
@@ -689,7 +713,15 @@ class Concore{
689713
this_thread::sleep_for(timespan);
690714
try {
691715
if(shmId_create != -1){
692-
std::strncpy(sharedData_create, val.c_str(), 256 - 1);
716+
if (sharedData_create == nullptr)
717+
throw 506;
718+
if (val.size() >= SHM_SIZE) {
719+
std::cerr << "ERROR: write_SM payload (" << val.size()
720+
<< " bytes) exceeds " << SHM_SIZE - 1
721+
<< "-byte shared memory limit. Data truncated!" << std::endl;
722+
}
723+
std::strncpy(sharedData_create, val.c_str(), SHM_SIZE - 1);
724+
sharedData_create[SHM_SIZE - 1] = '\0';
693725
}
694726
else throw 505;
695727
}

concoredocker.hpp

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626

2727
class Concore {
2828
private:
29+
static constexpr size_t SHM_SIZE = 4096;
30+
2931
int shmId_create = -1;
3032
int shmId_get = -1;
3133
char* sharedData_create = nullptr;
@@ -233,11 +235,25 @@ class Concore {
233235

234236
#ifdef __linux__
235237
void createSharedMemory(key_t key) {
236-
shmId_create = shmget(key, 256, IPC_CREAT | 0666);
238+
shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666);
237239
if (shmId_create == -1) {
238240
std::cerr << "Failed to create shared memory segment.\n";
239241
return;
240242
}
243+
244+
// Verify the segment is large enough (shmget won't resize an existing segment)
245+
struct shmid_ds shm_info;
246+
if (shmctl(shmId_create, IPC_STAT, &shm_info) == 0 && shm_info.shm_segsz < SHM_SIZE) {
247+
std::cerr << "Shared memory segment too small (" << shm_info.shm_segsz
248+
<< " bytes, need " << SHM_SIZE << "). Removing and recreating.\n";
249+
shmctl(shmId_create, IPC_RMID, nullptr);
250+
shmId_create = shmget(key, SHM_SIZE, IPC_CREAT | 0666);
251+
if (shmId_create == -1) {
252+
std::cerr << "Failed to recreate shared memory segment.\n";
253+
return;
254+
}
255+
}
256+
241257
sharedData_create = static_cast<char*>(shmat(shmId_create, NULL, 0));
242258
if (sharedData_create == reinterpret_cast<char*>(-1)) {
243259
std::cerr << "Failed to attach shared memory segment.\n";
@@ -249,7 +265,7 @@ class Concore {
249265
int retry = 0;
250266
const int MAX_RETRY = 100;
251267
while (retry < MAX_RETRY) {
252-
shmId_get = shmget(key, 256, 0666);
268+
shmId_get = shmget(key, SHM_SIZE, 0666);
253269
if (shmId_get != -1)
254270
break;
255271
std::cout << "Shared memory does not exist. Make sure the writer process is running.\n";
@@ -345,7 +361,7 @@ class Concore {
345361
std::string ins;
346362
try {
347363
if (shmId_get != -1 && sharedData_get && sharedData_get[0] != '\0')
348-
ins = std::string(sharedData_get, strnlen(sharedData_get, 256));
364+
ins = std::string(sharedData_get, strnlen(sharedData_get, SHM_SIZE));
349365
else
350366
throw 505;
351367
} catch (...) {
@@ -359,7 +375,7 @@ class Concore {
359375
std::this_thread::sleep_for(std::chrono::seconds(delay));
360376
try {
361377
if (shmId_get != -1 && sharedData_get) {
362-
ins = std::string(sharedData_get, strnlen(sharedData_get, 256));
378+
ins = std::string(sharedData_get, strnlen(sharedData_get, SHM_SIZE));
363379
retrycount++;
364380
} else {
365381
retrycount++;
@@ -419,14 +435,22 @@ class Concore {
419435
try {
420436
if (shmId_create == -1)
421437
throw 505;
438+
if (sharedData_create == nullptr)
439+
throw 506;
422440
val.insert(val.begin(), simtime + delta);
423441
std::ostringstream outfile;
424442
outfile << '[';
425443
for (size_t i = 0; i < val.size() - 1; i++)
426444
outfile << val[i] << ',';
427445
outfile << val[val.size() - 1] << ']';
428446
std::string result = outfile.str();
429-
std::strncpy(sharedData_create, result.c_str(), 256 - 1);
447+
if (result.size() >= SHM_SIZE) {
448+
std::cerr << "ERROR: write_SM payload (" << result.size()
449+
<< " bytes) exceeds " << SHM_SIZE - 1
450+
<< "-byte shared memory limit. Data truncated!" << std::endl;
451+
}
452+
std::strncpy(sharedData_create, result.c_str(), SHM_SIZE - 1);
453+
sharedData_create[SHM_SIZE - 1] = '\0';
430454
} catch (...) {
431455
std::cerr << "skipping +" << outpath << port << "/" << name << "\n";
432456
}

0 commit comments

Comments
 (0)