shared memory crash fixes

src/debug_log/debug_log_writer.cc

@@ -27,6 +27,8 @@
 
 #include <fstream>
 
+#define MODSEC_SM_LOG_SIGNATURE        "m0d$log"
+
 namespace modsecurity {
 namespace debug_log {
 
@@ -45,29 +47,35 @@ debug_log_file_handler_t *DebugLogWriter::find_handler(
 
 
 debug_log_file_handler_t *DebugLogWriter::add_new_handler(
-    const std::string &fileName, std::string *error) {
+    const std::string &fileName, std::string *error, int pinc) {
     debug_log_file_handler_t *current = m_first;
     int shm_id;
     key_t mem_key_structure;
     key_t mem_key_file_name;
     debug_log_file_handler_t *new_debug_log;
     char *shm_ptr2;
+    static char memsign[] = MODSEC_SM_LOG_SIGNATURE;
     FILE *fp;
 
+    /* recursion protection */
+    if (pinc > 10) {
+        return NULL;
+    }
+
     fp = fopen(fileName.c_str(), "a");
-    if (fp == 0) {
+    if (fp == NULL) {
         error->assign("Failed to open file: " + fileName);
         goto err_fh;
     }
 
-    mem_key_structure = ftok(fileName.c_str(), 1);
+    mem_key_structure = ftok(fileName.c_str(), pinc);
     if (mem_key_structure < 0) {
         error->assign("Failed to select key for the shared memory (1): ");
         error->append(strerror(errno));
         goto err_mem_key;
     }
 
-    mem_key_file_name = ftok(fileName.c_str(), 2);
+    mem_key_file_name = ftok(fileName.c_str(), pinc+1);
     if (mem_key_file_name < 0) {
         error->assign("Failed to select key for the shared memory (2): ");
         error->append(strerror(errno));
@@ -89,6 +97,19 @@ debug_log_file_handler_t *DebugLogWriter::add_new_handler(
         error->append(strerror(errno));
         goto err_shmat1;
     }
+
+    /* there's a crash scenario when add_new_handler() initializes shared memory block,
+     * and later another copy of application reinitializes it (repointing new_debug_log->file_name
+     * to new process' local memory.  it causes first application instance to crash on any
+     * find_handler() invocation.  Use-case is 'nginx -t' / 'nginx -s reload'.
+     */
+    if (memcmp(new_debug_log->signature, memsign, strlen(memsign)) == 0) {
+        /* if shared memory block is in use, try allocating with another project_id in ftok() */
+        fclose(fp);
+        shmdt(new_debug_log);
+        return add_new_handler(fileName, error, pinc+2);
+    }
+
     memset(new_debug_log, '\0', sizeof(debug_log_file_handler_t));
 
     pthread_mutex_init(&new_debug_log->lock, NULL);
@@ -116,6 +137,9 @@ debug_log_file_handler_t *DebugLogWriter::add_new_handler(
 
     new_debug_log->file_name = shm_ptr2;
 
+    /* populate 'shared block in-use' signature */
+    memcpy(new_debug_log->signature, memsign, sizeof(new_debug_log->signature));
+
     if (m_first == NULL) {
         m_first = new_debug_log;
     } else {
@@ -134,14 +158,15 @@ debug_log_file_handler_t *DebugLogWriter::add_new_handler(
     }
 
     return new_debug_log;
+
 err_shmget2:
 err_shmat2:
     shmdt(shm_ptr2);
-    fclose(new_debug_log->fp);
 err_shmget1:
 err_shmat1:
     shmdt(new_debug_log);
 err_mem_key:
+    fclose(fp);
 err_fh:
     return NULL;
 }
@@ -150,7 +175,7 @@ debug_log_file_handler_t *DebugLogWriter::add_new_handler(
 int DebugLogWriter::open(const std::string& fileName, std::string *error) {
     debug_log_file_handler_t *a = find_handler(fileName);
     if (a == NULL) {
-        a = add_new_handler(fileName, error);
+        a = add_new_handler(fileName, error, 1);
         if (error->size() > 0) {
             return -1;
         }

src/debug_log/debug_log_writer.h

@@ -33,6 +33,7 @@ namespace debug_log {
 
 
 typedef struct debug_log_file_handler {
+    char signature[8];
     char *file_name;
     FILE *fp;
     int file_handler;
@@ -60,7 +61,7 @@ class DebugLogWriter {
  protected:
     debug_log_file_handler_t *find_handler(const std::string &fileName);
     debug_log_file_handler_t *add_new_handler(const std::string &fileName,
-        std::string *error);
+        std::string *error, int pinc);
 
  private:
     DebugLogWriter() : m_first(NULL) { }

src/utils/shared_files.cc

@@ -34,6 +34,7 @@
 namespace modsecurity {
 namespace utils {
 
+#define MODSEC_SM_SHF_SIGNATURE        "m0d$shf"
 
 msc_file_handler_t *SharedFiles::find_handler(
     const std::string &fileName) {
@@ -50,29 +51,35 @@ msc_file_handler_t *SharedFiles::find_handler(
 
 
 msc_file_handler_t *SharedFiles::add_new_handler(
-    const std::string &fileName, std::string *error) {
+    const std::string &fileName, std::string *error, int pinc) {
     msc_file_handler_t *current = m_first;
     int shm_id;
     key_t mem_key_structure;
     key_t mem_key_file_name;
     msc_file_handler_t *new_debug_log;
     char *shm_ptr2;
+    char memsign[] = MODSEC_SM_SHF_SIGNATURE;
     FILE *fp;
 
+    if (pinc > 10) {
+        /* recursion protection */
+        return NULL;
+    }
+
     fp = fopen(fileName.c_str(), "a");
-    if (fp == 0) {
+    if (fp == NULL) {
         error->assign("Failed to open file: " + fileName);
         goto err_fh;
     }
 
-    mem_key_structure = ftok(fileName.c_str(), 1);
+    mem_key_structure = ftok(fileName.c_str(), pinc);
     if (mem_key_structure < 0) {
         error->assign("Failed to select key for the shared memory (1): ");
         error->append(strerror(errno));
         goto err_mem_key;
     }
 
-    mem_key_file_name = ftok(fileName.c_str(), 2);
+    mem_key_file_name = ftok(fileName.c_str(), pinc+1);
     if (mem_key_file_name < 0) {
         error->assign("Failed to select key for the shared memory (2): ");
         error->append(strerror(errno));
@@ -94,6 +101,15 @@ msc_file_handler_t *SharedFiles::add_new_handler(
         error->append(strerror(errno));
         goto err_shmat1;
     }
+
+    /* there's a crash scenario when add_new_handler(), described in src/debug_log/debug_log_writer.cc */
+    if (memcmp(new_debug_log->signature, memsign, strlen(memsign)) == 0) {
+        /* if shared memory block is in use, try allocating with another project_id in ftok() */
+        fclose(fp);
+        shmdt(new_debug_log);
+        return add_new_handler(fileName, error, pinc+2);
+    }
+
     memset(new_debug_log, '\0', sizeof(msc_file_handler_t));
 
     pthread_mutex_init(&new_debug_log->lock, NULL);
@@ -121,6 +137,9 @@ msc_file_handler_t *SharedFiles::add_new_handler(
 
     new_debug_log->file_name = shm_ptr2;
 
+    /* populate 'shared block in-use' signature */
+    memcpy(new_debug_log->signature, memsign, sizeof(new_debug_log->signature));
+
     if (m_first == NULL) {
         m_first = new_debug_log;
     } else {
@@ -139,14 +158,15 @@ msc_file_handler_t *SharedFiles::add_new_handler(
     }
 
     return new_debug_log;
+
 err_shmget2:
 err_shmat2:
     shmdt(shm_ptr2);
-    fclose(new_debug_log->fp);
 err_shmget1:
 err_shmat1:
     shmdt(new_debug_log);
 err_mem_key:
+    fclose(fp);
 err_fh:
     return NULL;
 }
@@ -155,7 +175,7 @@ msc_file_handler_t *SharedFiles::add_new_handler(
 bool SharedFiles::open(const std::string& fileName, std::string *error) {
     msc_file_handler_t *a = find_handler(fileName);
     if (a == NULL) {
-        a = add_new_handler(fileName, error);
+        a = add_new_handler(fileName, error, 1);
         if (error->size() > 0) {
             return false;
         }

src/utils/shared_files.h

@@ -33,6 +33,7 @@ namespace utils {
 
 
 typedef struct msc_file_handler {
+    char signature[8];
     char *file_name;
     FILE *fp;
     int file_handler;
@@ -59,7 +60,7 @@ class SharedFiles {
  protected:
     msc_file_handler_t *find_handler(const std::string &fileName);
     msc_file_handler_t *add_new_handler(const std::string &fileName,
-        std::string *error);
+        std::string *error, int pinc);
 
  private:
     SharedFiles() : m_first(NULL) { }