diff --git a/src/debug_log/debug_log_writer.cc b/src/debug_log/debug_log_writer.cc index 5ddf8c449..5c4bcbcfe 100644 --- a/src/debug_log/debug_log_writer.cc +++ b/src/debug_log/debug_log_writer.cc @@ -27,6 +27,8 @@ #include +#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; } diff --git a/src/debug_log/debug_log_writer.h b/src/debug_log/debug_log_writer.h index ed8b0cda2..3d242ef2c 100644 --- a/src/debug_log/debug_log_writer.h +++ b/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) { } diff --git a/src/utils/shared_files.cc b/src/utils/shared_files.cc index ef66aaa53..f2667527a 100644 --- a/src/utils/shared_files.cc +++ b/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; } diff --git a/src/utils/shared_files.h b/src/utils/shared_files.h index 7d71089ca..dfd9c79d9 100644 --- a/src/utils/shared_files.h +++ b/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) { }