From 53030dd8c67fb41e15644a1640b524ab797a0afb Mon Sep 17 00:00:00 2001 From: Rob Swindell <rob@synchro.net> Date: Sun, 13 Sep 2020 02:48:52 -0700 Subject: [PATCH] Insure active_clients is initialized before cleanup() can be called. Fix reported and observed crash in cleanup() (in ftp, mail, websrvr) when failing to create the temp directory. This was due to cleanup() being called before the protected integer "active_clients" was initialized. Also, md() needs to return the errno value (not a BOOL) since the caller may be in another DLL with a different errno (which likely has a value of 0/no error). --- src/sbbs3/ftpsrvr.c | 6 +++--- src/sbbs3/load_cfg.c | 8 ++++---- src/sbbs3/mailsrvr.c | 8 +++----- src/sbbs3/main.cpp | 13 +++++++------ src/sbbs3/sbbs.h | 3 +-- src/sbbs3/services.c | 4 ++-- src/sbbs3/websrvr.c | 9 +++++---- 7 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/sbbs3/ftpsrvr.c b/src/sbbs3/ftpsrvr.c index 5077391baa..f67899d860 100644 --- a/src/sbbs3/ftpsrvr.c +++ b/src/sbbs3/ftpsrvr.c @@ -6049,6 +6049,7 @@ void DLLCALL ftp_server(void* arg) startup->shutdown_now=FALSE; terminate_server=FALSE; protected_uint32_init(&thread_count, 0); + protected_uint32_init(&active_clients, 0); do { /* Setup intelligent defaults */ @@ -6124,8 +6125,8 @@ void DLLCALL ftp_server(void* arg) else SAFECOPY(scfg.temp_dir,"../temp"); prep_dir(scfg.ctrl_dir, scfg.temp_dir, sizeof(scfg.temp_dir)); - if(!md(scfg.temp_dir)) { - lprintf(LOG_CRIT,"!ERROR %d (%s) creating directory: %s", errno, strerror(errno), scfg.temp_dir); + if((i = md(scfg.temp_dir)) != 0) { + lprintf(LOG_CRIT,"!ERROR %d (%s) creating directory: %s", i, strerror(i), scfg.temp_dir); cleanup(1,__LINE__); break; } @@ -6155,7 +6156,6 @@ void DLLCALL ftp_server(void* arg) lprintf(LOG_DEBUG,"Maximum inactivity: %d seconds",startup->max_inactivity); - protected_uint32_init(&active_clients, 0); update_clients(); strlwr(scfg.sys_id); /* Use lower-case unix-looking System ID for group name */ diff --git a/src/sbbs3/load_cfg.c b/src/sbbs3/load_cfg.c index 1691863a88..61c69d880b 100644 --- a/src/sbbs3/load_cfg.c +++ b/src/sbbs3/load_cfg.c @@ -326,13 +326,13 @@ void DLLCALL free_text(char* text[]) /****************************************************************************/ /* If the directory 'path' doesn't exist, create it. */ /****************************************************************************/ -BOOL md(const char* inpath) +int md(const char* inpath) { char path[MAX_PATH+1]; char* p; if(inpath[0]==0) - return(FALSE); + return EINVAL; SAFECOPY(path,inpath); @@ -348,10 +348,10 @@ BOOL md(const char* inpath) if(!isdir(path)) { if(mkpath(path) != 0) - return FALSE; + return errno; } - return(TRUE); + return 0; } /****************************************************************************/ diff --git a/src/sbbs3/mailsrvr.c b/src/sbbs3/mailsrvr.c index 3a1ff9a983..44607a9e5c 100644 --- a/src/sbbs3/mailsrvr.c +++ b/src/sbbs3/mailsrvr.c @@ -5867,7 +5867,6 @@ static void cleanup(int code) } lprintf(LOG_INFO, "0000 Done waiting for child threads to terminate"); } - free_cfg(&scfg); semfile_list_free(&recycle_semfiles); @@ -5899,7 +5898,6 @@ static void cleanup(int code) if(WSAInitialized && WSACleanup()!=0) lprintf(LOG_ERR,"0000 !WSACleanup ERROR %d",ERROR_VALUE); #endif - thread_down(); status("Down"); if(terminate_server || code) { @@ -6014,6 +6012,7 @@ void DLLCALL mail_server(void* arg) SetThreadName("sbbs/mailServer"); protected_uint32_init(&thread_count, 0); + protected_uint32_init(&active_clients, 0); do { /* Setup intelligent defaults */ @@ -6084,8 +6083,8 @@ void DLLCALL mail_server(void* arg) else SAFECOPY(scfg.temp_dir,"../temp"); prep_dir(scfg.ctrl_dir, scfg.temp_dir, sizeof(scfg.temp_dir)); - if(!md(scfg.temp_dir)) { - lprintf(LOG_CRIT, "!ERROR %d (%s) creating directory: %s", errno, strerror(errno), scfg.temp_dir); + if((i = md(scfg.temp_dir)) != 0) { + lprintf(LOG_CRIT, "!ERROR %d (%s) creating directory: %s", i, strerror(i), scfg.temp_dir); cleanup(1); return; } @@ -6149,7 +6148,6 @@ void DLLCALL mail_server(void* arg) lprintf(LOG_DEBUG,"Maximum inactivity: %u seconds",startup->max_inactivity); - protected_uint32_init(&active_clients, 0); update_clients(); /* open a socket and wait for a client */ diff --git a/src/sbbs3/main.cpp b/src/sbbs3/main.cpp index d07f8e3f39..dc20a334cd 100644 --- a/src/sbbs3/main.cpp +++ b/src/sbbs3/main.cpp @@ -3312,8 +3312,8 @@ sbbs_t::sbbs_t(ushort node_num, union xp_sockaddr *addr, size_t addr_len, const else SAFECOPY(cfg.temp_dir,"../temp"); prep_dir(cfg.ctrl_dir, cfg.temp_dir, sizeof(cfg.temp_dir)); - if(!md(cfg.temp_dir)) - lprintf(LOG_CRIT,"!ERROR %d (%s) creating directory: %s", errno, strerror(errno), cfg.temp_dir); + if((i = md(cfg.temp_dir)) != 0) + lprintf(LOG_CRIT,"!ERROR %d (%s) creating directory: %s", i, strerror(i), cfg.temp_dir); if(sd==INVALID_SOCKET) { /* events thread */ if(startup->first_node==1) SAFEPRINTF(path,"%sevent",cfg.temp_dir); @@ -3541,8 +3541,8 @@ bool sbbs_t::init() return(false); } - if(!md(cfg.temp_dir)) { - lprintf(LOG_CRIT,"!ERROR %d (%s) creating directory: %s", errno, strerror(errno), cfg.temp_dir); + if((i = md(cfg.temp_dir)) != 0) { + lprintf(LOG_CRIT,"!ERROR %d (%s) creating directory: %s", i, strerror(i), cfg.temp_dir); return false; } @@ -5124,8 +5124,9 @@ void DLLCALL bbs_thread(void* arg) lprintf(LOG_INFO,"Verifying/creating node directories"); for(i=0;i<=scfg.sys_nodes;i++) { if(i) { - if(!md(scfg.node_path[i-1])) { - lprintf(LOG_CRIT,"!ERROR %d (%s) creating directory: %s", errno, strerror(errno), scfg.node_path[i-1]); + int err; + if((err = md(scfg.node_path[i-1])) != 0) { + lprintf(LOG_CRIT,"!ERROR %d (%s) creating directory: %s", err, strerror(err), scfg.node_path[i-1]); cleanup(1); return; } diff --git a/src/sbbs3/sbbs.h b/src/sbbs3/sbbs.h index ad52dda442..062e974718 100644 --- a/src/sbbs3/sbbs.h +++ b/src/sbbs3/sbbs.h @@ -1259,8 +1259,7 @@ extern "C" { DLLEXPORT void DLLCALL free_text(char* text[]); DLLEXPORT ushort DLLCALL sys_timezone(scfg_t* cfg); DLLEXPORT char * DLLCALL prep_dir(const char* base, char* dir, size_t buflen); - DLLEXPORT BOOL DLLCALL md(const char *path); - + DLLEXPORT int DLLCALL md(const char *path); /* scfgsave.c */ DLLEXPORT BOOL DLLCALL save_cfg(scfg_t* cfg, int backup_level); diff --git a/src/sbbs3/services.c b/src/sbbs3/services.c index d2346e18c2..f09ba8ee17 100644 --- a/src/sbbs3/services.c +++ b/src/sbbs3/services.c @@ -1837,8 +1837,8 @@ void DLLCALL services_thread(void* arg) else SAFECOPY(scfg.temp_dir,"../temp"); prep_dir(scfg.ctrl_dir, scfg.temp_dir, sizeof(scfg.temp_dir)); - if(!md(scfg.temp_dir)) { - lprintf(LOG_CRIT, "!ERROR %d (%s) creating directory: %s", errno, strerror(errno), scfg.temp_dir); + if((i = md(scfg.temp_dir)) != 0) { + lprintf(LOG_CRIT, "!ERROR %d (%s) creating directory: %s", i, strerror(i), scfg.temp_dir); cleanup(1); return; } diff --git a/src/sbbs3/websrvr.c b/src/sbbs3/websrvr.c index d7a10d54b4..bfabe167b4 100644 --- a/src/sbbs3/websrvr.c +++ b/src/sbbs3/websrvr.c @@ -6914,7 +6914,8 @@ void DLLCALL web_server(void* arg) http_session_t * session=NULL; void *acc_type; char *ssl_estr; - int lvl; + int lvl; + int i; startup=(web_startup_t*)arg; @@ -6953,6 +6954,7 @@ void DLLCALL web_server(void* arg) startup->shutdown_now=FALSE; terminate_server=FALSE; protected_uint32_init(&thread_count, 0); + protected_uint32_init(&active_clients,0); do { /* Setup intelligent defaults */ @@ -7033,8 +7035,8 @@ void DLLCALL web_server(void* arg) else SAFECOPY(scfg.temp_dir,"../temp"); prep_dir(startup->ctrl_dir, scfg.temp_dir, sizeof(scfg.temp_dir)); - if(!md(scfg.temp_dir)) { - lprintf(LOG_CRIT, "!ERROR %d (%s) creating directory: %s", errno, strerror(errno), scfg.temp_dir); + if((i = md(scfg.temp_dir)) != 0) { + lprintf(LOG_CRIT, "!ERROR %d (%s) creating directory: %s", i, strerror(i), scfg.temp_dir); cleanup(1); return; } @@ -7074,7 +7076,6 @@ void DLLCALL web_server(void* arg) if(uptime==0) uptime=time(NULL); /* this must be done *after* setting the timezone */ - protected_uint32_init(&active_clients,0); update_clients(); /* open a socket and wait for a client */ -- GitLab