From 03707e73523860548f6dba876fb6c3c3d96f2b84 Mon Sep 17 00:00:00 2001 From: Art Cancro Date: Tue, 26 Oct 1999 03:21:17 +0000 Subject: [PATCH 1/1] * Changed a lot of strncpy() calls to safestrncpy() and replaced most of their hardcoded size arguments with 'sizeof' based arguments. * Moved the CitContext destruction into the housekeeper thread and out of the thread being cancelled. Didn't fix it, though (to see what happens, link the server against ElectricFence and watch what happens when a session ends). --- citadel/ChangeLog | 8 +++++++- citadel/citserver.c | 36 +++++++++++++++++++++++------------- citadel/control.c | 32 ++++++++++++++++++++------------ citadel/dynloader.c | 4 +++- citadel/housekeeping.c | 15 +++++++++++++-- citadel/room_ops.c | 20 +++++++++++--------- citadel/serv_chat.c | 4 ++-- citadel/sysdep.c | 27 ++++++++++++++++----------- citadel/sysdep_decls.h | 2 +- 9 files changed, 96 insertions(+), 52 deletions(-) diff --git a/citadel/ChangeLog b/citadel/ChangeLog index fc5ac45cc..18a4babd3 100644 --- a/citadel/ChangeLog +++ b/citadel/ChangeLog @@ -1,4 +1,11 @@ $Log$ +Revision 1.394 1999/10/26 03:21:16 ajc +* Changed a lot of strncpy() calls to safestrncpy() and replaced most of their + hardcoded size arguments with 'sizeof' based arguments. +* Moved the CitContext destruction into the housekeeper thread and out of the + thread being cancelled. Didn't fix it, though (to see what happens, link + the server against ElectricFence and watch what happens when a session ends). + Revision 1.393 1999/10/24 19:22:51 nbryant * Makefile.in, configure.in: added --enable-icq flag; made checks for authentication libraries more intelligent. @@ -1347,4 +1354,3 @@ Sat Jul 11 00:20:48 EDT 1998 Nathan Bryant Fri Jul 10 1998 Art Cancro * Initial CVS import - diff --git a/citadel/citserver.c b/citadel/citserver.c index 31d8c0782..7343e230d 100644 --- a/citadel/citserver.c +++ b/citadel/citserver.c @@ -124,6 +124,7 @@ void deallocate_user_data(struct CitContext *con) */ void cleanup_stuff(void *arg) { + char buf[256]; lprintf(9, "cleanup_stuff() called\n"); @@ -142,11 +143,18 @@ void cleanup_stuff(void *arg) /* Deallocate any user-data attached to this session */ deallocate_user_data(CC); - /* Now get rid of the session and context */ - lprintf(7, "cleanup_stuff() calling RemoveContext(%d)\n", CC->cs_pid); - RemoveContext(CC); + /* Tell the housekeeping thread to remove the session and context. + * This can't be done inline because the context data gets destroyed + * halfway through, and the context being destroyed can't be the one + * doing the work. + */ + lprintf(7, "Scheduling housekeeper REMOVE_CONTEXT(%d)\n", CC->cs_pid); + sprintf(buf, "REMOVE_CONTEXT|%d", CC->cs_pid); + enter_housekeeping_cmd(buf); - /* Wake up the housekeeping thread */ + /* Tell the housekeeping thread to check to see if this is the time + * to initiate a scheduled shutdown event. + */ enter_housekeeping_cmd("SCHED_SHUTDOWN"); } @@ -240,12 +248,13 @@ void cmd_info(void) { void cmd_rchg(char *argbuf) { - char newroomname[256]; /* set to 256 to prevent buffer overruns */ + char newroomname[ROOMNAMELEN]; extract(newroomname, argbuf, 0); newroomname[ROOMNAMELEN] = 0; if (strlen(newroomname) > 0) { - strncpy(CC->fake_roomname, newroomname, ROOMNAMELEN); + safestrncpy(CC->fake_roomname, newroomname, + sizeof(CC->fake_roomname) ); CC->fake_roomname[ROOMNAMELEN - 1] = 0; } else { @@ -259,7 +268,7 @@ void cmd_hchg(char *newhostname) if ((newhostname) && (newhostname[0])) { memset(CC->fake_hostname, 0, 25); - strncpy(CC->fake_hostname, newhostname, 24); + safestrncpy(CC->fake_hostname, newhostname, sizeof(CC->fake_hostname)); } else strcpy(CC->fake_hostname, ""); @@ -279,7 +288,7 @@ void cmd_uchg(char *newusername) CC->cs_flags &= ~CS_STEALTH; memset(CC->fake_username, 0, 32); if (strncasecmp(newusername, CC->curr_user, strlen(CC->curr_user))) - strncpy(CC->fake_username, newusername, 31); + safestrncpy(CC->fake_username, newusername, sizeof(CC->fake_username)); } else { @@ -390,14 +399,14 @@ void cmd_iden(char *argbuf) rev_level = extract_int(argbuf,2); extract(desc,argbuf,3); - strncpy(from_host,config.c_fqdn,sizeof from_host); + safestrncpy(from_host, config.c_fqdn, sizeof from_host); from_host[sizeof from_host - 1] = 0; if (num_parms(argbuf)>=5) extract(from_host,argbuf,4); CC->cs_clientdev = dev_code; CC->cs_clienttyp = cli_code; CC->cs_clientver = rev_level; - strncpy(CC->cs_clientname,desc,31); + safestrncpy(CC->cs_clientname, desc, sizeof CC->cs_clientname); CC->cs_clientname[31] = 0; lprintf(9, "Looking up hostname\n"); @@ -406,7 +415,7 @@ void cmd_iden(char *argbuf) if (inet_aton(from_host, &addr)) locate_host(CC->cs_host, &addr); else { - strncpy(CC->cs_host,from_host,24); + safestrncpy(CC->cs_host, from_host, sizeof CC->cs_host); CC->cs_host[24] = 0; } } @@ -829,7 +838,7 @@ void *context_loop(struct CitContext *con) strcpy(CC->curr_user,"(not logged in)"); strcpy(CC->net_node,""); snprintf(CC->temp, sizeof CC->temp, tmpnam(NULL)); - strncpy(CC->cs_host, config.c_fqdn, sizeof CC->cs_host); + safestrncpy(CC->cs_host, config.c_fqdn, sizeof CC->cs_host); CC->cs_host[sizeof CC->cs_host - 1] = 0; len = sizeof sin; if (!getpeername(CC->client_socket, (struct sockaddr *) &sin, &len)) @@ -874,7 +883,8 @@ void *context_loop(struct CitContext *con) && (strncasecmp(cmdbuf, "PEXP", 4)) && (strncasecmp(cmdbuf, "GEXP", 4)) ) { strcpy(CC->lastcmdname, " "); - strncpy(CC->lastcmdname, cmdbuf, 4); + safestrncpy(CC->lastcmdname, cmdbuf, + sizeof(CC->lastcmdname) ); time(&CC->lastidle); } diff --git a/citadel/control.c b/citadel/control.c index f21e3d88a..1455c72c7 100644 --- a/citadel/control.c +++ b/citadel/control.c @@ -172,13 +172,17 @@ void cmd_conf(char *argbuf) { a = 0; while (client_gets(buf), strcmp(buf, "000")) { switch(a) { - case 0: strncpy(config.c_nodename, buf, 16); + case 0: safestrncpy(config.c_nodename, buf, + sizeof config.c_nodename); break; - case 1: strncpy(config.c_fqdn, buf, 64); + case 1: safestrncpy(config.c_fqdn, buf, + sizeof config.c_fqdn); break; - case 2: strncpy(config.c_humannode, buf, 21); + case 2: safestrncpy(config.c_humannode, buf, + sizeof config.c_humannode); break; - case 3: strncpy(config.c_phonenum, buf, 16); + case 3: safestrncpy(config.c_phonenum, buf, + sizeof config.c_phonenum); break; case 4: config.c_creataide = atoi(buf); break; @@ -196,31 +200,35 @@ void cmd_conf(char *argbuf) { if (config.c_twitdetect != 0) config.c_twitdetect = 1; break; - case 9: strncpy(config.c_twitroom, - buf, ROOMNAMELEN); + case 9: safestrncpy(config.c_twitroom, buf, + sizeof config.c_twitroom); break; - case 10: strncpy(config.c_moreprompt, buf, 80); + case 10: safestrncpy(config.c_moreprompt, buf, + sizeof config.c_moreprompt); break; case 11: config.c_restrict = atoi(buf); if (config.c_restrict != 0) config.c_restrict = 1; break; - case 12: strncpy(config.c_bbs_city, buf, 32); + case 12: safestrncpy(config.c_bbs_city, buf, + sizeof config.c_bbs_city); break; - case 13: strncpy(config.c_sysadm, buf, 26); + case 13: safestrncpy(config.c_sysadm, buf, + sizeof config.c_sysadm); break; case 14: config.c_maxsessions = atoi(buf); if (config.c_maxsessions < 1) config.c_maxsessions = 1; break; - case 15: strncpy(config.c_net_password, buf, 20); + case 15: safestrncpy(config.c_net_password, buf, + sizeof config.c_net_password); break; case 16: config.c_userpurge = atoi(buf); break; case 17: config.c_roompurge = atoi(buf); break; - case 18: strncpy(config.c_logpages, - buf, ROOMNAMELEN); + case 18: safestrncpy(config.c_logpages, buf, + sizeof config.c_logpages); break; case 19: config.c_createax = atoi(buf); if (config.c_createax < 1) diff --git a/citadel/dynloader.c b/citadel/dynloader.c index 1993328c8..d11ca6790 100644 --- a/citadel/dynloader.c +++ b/citadel/dynloader.c @@ -94,7 +94,9 @@ void DLoader_Init(char *pathname) exit(1); } while ((dptr = readdir(dir)) != NULL) { - if (dptr->d_name[0] == '.') + if (strlen(dptr->d_name) < 4) + continue; + if (strcasecmp(&dptr->d_name[strlen(dptr->d_name)-3], ".so")) continue; snprintf(pathbuf, PATH_MAX, "%s/%s", pathname, dptr->d_name); diff --git a/citadel/housekeeping.c b/citadel/housekeeping.c index ac6d8882e..fef2f694c 100644 --- a/citadel/housekeeping.c +++ b/citadel/housekeeping.c @@ -88,6 +88,7 @@ void housekeeping_loop(void) { fd_set readfds; int did_something; char house_cmd[256]; /* Housekeep cmds are always 256 bytes long */ + char cmd[256]; if (pipe(housepipe) != 0) { lprintf(1, "FATAL ERROR: can't create housekeeping pipe: %s\n", @@ -119,16 +120,26 @@ void housekeeping_loop(void) { strcpy(house_cmd, "MINUTE"); } + extract(cmd, house_cmd, 0); /* Do whatever this cmd requires */ - if (!strcmp(house_cmd, "MINUTE")) { + + /* Once-every-minute housekeeper */ + if (!strcmp(cmd, "MINUTE")) { terminate_idle_sessions(); } - else if (!strcmp(house_cmd, "SCHED_SHUTDOWN")) { + /* Scheduled shutdown housekeeper */ + else if (!strcmp(cmd, "SCHED_SHUTDOWN")) { check_sched_shutdown(); } + /* Remove a context (session ending) */ + else if (!strcmp(cmd, "REMOVE_CONTEXT")) { + RemoveContext( extract_int(house_cmd, 1) ); + } + + /* Unknown */ else { lprintf(7, "Unknown housekeeping command\n"); } diff --git a/citadel/room_ops.c b/citadel/room_ops.c index 753e863cd..9dd7ed0ed 100644 --- a/citadel/room_ops.c +++ b/citadel/room_ops.c @@ -852,8 +852,9 @@ void cmd_rdir(void) buf[strlen(buf) - 1] = 0; if ((!strncasecmp(buf, flnm, strlen(flnm))) && (buf[strlen(flnm)] == ' ')) - strncpy(comment, - &buf[strlen(flnm) + 1], 255); + safestrncpy(comment, + &buf[strlen(flnm) + 1], + sizeof comment); } cprintf("%s|%ld|%s\n", flnm, statbuf.st_size, comment); } @@ -938,13 +939,14 @@ void cmd_setr(char *args) strcpy(old_name, CC->quickroom.QRname); extract(buf, args, 0); buf[ROOMNAMELEN] = 0; - strncpy(CC->quickroom.QRname, buf, ROOMNAMELEN - 1); + safestrncpy(CC->quickroom.QRname, buf, sizeof CC->quickroom.QRname); extract(buf, args, 1); buf[10] = 0; - strncpy(CC->quickroom.QRpasswd, buf, 9); + safestrncpy(CC->quickroom.QRpasswd, buf, sizeof CC->quickroom.QRpasswd); extract(buf, args, 2); buf[15] = 0; - strncpy(CC->quickroom.QRdirname, buf, 19); + safestrncpy(CC->quickroom.QRdirname, buf, + sizeof CC->quickroom.QRdirname); CC->quickroom.QRflags = (extract_int(args, 3) | QR_INUSE); if (num_parms(args) >= 7) CC->quickroom.QRorder = (char) new_order; @@ -1197,8 +1199,8 @@ unsigned create_room(char *new_room_name, return (0); /* already exists */ memset(&qrbuf, 0, sizeof(struct quickroom)); - strncpy(qrbuf.QRname, new_room_name, ROOMNAMELEN); - strncpy(qrbuf.QRpasswd, new_room_pass, 9); + safestrncpy(qrbuf.QRname, new_room_name, sizeof qrbuf.QRname); + safestrncpy(qrbuf.QRpasswd, new_room_pass, sizeof qrbuf.QRpasswd); qrbuf.QRflags = QR_INUSE; qrbuf.QRnumber = get_new_room_number(); if (new_room_type > 0) @@ -1336,7 +1338,7 @@ void cmd_cre8(char *args) new_room_type, new_room_pass, new_room_floor); /* post a message in Aide> describing the new room */ - strncpy(aaa, new_room_name, 255); + safestrncpy(aaa, new_room_name, sizeof aaa); strcat(aaa, "> created by "); strcat(aaa, CC->usersupp.fullname); if (newflags & QR_MAILBOX) @@ -1489,7 +1491,7 @@ void cmd_cflr(char *argbuf) lgetfloor(&flbuf, free_slot); flbuf.f_flags = F_INUSE; flbuf.f_ref_count = 0; - strncpy(flbuf.f_name, new_floor_name, 255); + safestrncpy(flbuf.f_name, new_floor_name, sizeof flbuf.f_name); lputfloor(&flbuf, free_slot); cprintf("%d %d\n", OK, free_slot); } diff --git a/citadel/serv_chat.c b/citadel/serv_chat.c index 8328259a3..5b23aedd3 100644 --- a/citadel/serv_chat.c +++ b/citadel/serv_chat.c @@ -86,10 +86,10 @@ void allwrite(char *cmdbuf, int flag, char *roomname, char *username) time(&now); clnew->next = NULL; clnew->chat_time = now; - strncpy(clnew->chat_room, roomname, sizeof clnew->chat_room); + safestrncpy(clnew->chat_room, roomname, sizeof clnew->chat_room); clnew->chat_room[sizeof clnew->chat_room - 1] = 0; if (username) { - strncpy(clnew->chat_username, username, + safestrncpy(clnew->chat_username, username, sizeof clnew->chat_username); clnew->chat_username[sizeof clnew->chat_username - 1] = 0; } else diff --git a/citadel/sysdep.c b/citadel/sysdep.c index 8c9a12b21..26d132b5f 100644 --- a/citadel/sysdep.c +++ b/citadel/sysdep.c @@ -67,7 +67,7 @@ pthread_mutex_t Critters[MAX_SEMAPHORES]; /* Things needing locking */ pthread_key_t MyConKey; /* TSD key for MyContext() */ int msock; /* master listening socket */ -int verbosity = 3; /* Logging level */ +int verbosity = 9; /* Logging level */ struct CitContext masterCC; @@ -433,13 +433,14 @@ void InitMyContext(struct CitContext *con) /* * Remove a context from the context list. */ -void RemoveContext(struct CitContext *con) +void RemoveContext(int con) { - struct CitContext *ptr; + struct CitContext *ptr = NULL; + struct CitContext *ToFree = NULL; lprintf(7, "Starting RemoveContext()\n"); - if (con==NULL) { - lprintf(7, "WARNING: RemoveContext() called with null!\n"); + if (con==0) { + lprintf(7, "WARNING: RemoveContext() called with 0!\n"); return; } @@ -448,21 +449,25 @@ void RemoveContext(struct CitContext *con) * so do not call it from within this loop. */ begin_critical_section(S_SESSION_TABLE); - lprintf(7, "Closing socket %d\n", con->client_socket); - close(con->client_socket); - if (ContextList==con) { + if (ContextList->client_socket == con) { + ToFree = ContextList; ContextList = ContextList->next; } else { for (ptr = ContextList; ptr != NULL; ptr = ptr->next) { - if (ptr->next == con) { - ptr->next = con->next; + if (ptr->next->client_socket == con) { + ToFree = ptr->next; + ptr->next = ptr->next->next; } } } - phree(con); + + lprintf(7, "Closing socket %d\n", ToFree->client_socket); + close(ToFree->client_socket); + phree(ToFree); + end_critical_section(S_SESSION_TABLE); lprintf(7, "Done with RemoveContext\n"); } diff --git a/citadel/sysdep_decls.h b/citadel/sysdep_decls.h index 4f1587f8c..d1777bdc1 100644 --- a/citadel/sysdep_decls.h +++ b/citadel/sysdep_decls.h @@ -7,7 +7,7 @@ int ig_tcp_server (int port_number, int queue_len); struct CitContext *MyContext (void); struct CitContext *CreateNewContext (void); void InitMyContext (struct CitContext *con); -void RemoveContext (struct CitContext *con); +void RemoveContext (int con); int session_count (void); void client_write (char *buf, int nbytes); void cprintf (const char *format, ...); -- 2.39.2