Skip to content

Commit

Permalink
net_telnet: Fix node leak.
Browse files Browse the repository at this point in the history
For TELNETS, if telnet_handshake fails,
bbs_node_exit was not called so the node
was not automatically cleaned up. The node
would thus stay registered indefinitely
until manually kicked by the sysop.

Node setup for network protocols is also
simplified a bit, and we now keeptrack
of a "lifetime node ID", unique for the lifetime
of the BBS process, to make matching up registered
nodes to previous events in the logs a little
bit easier.
  • Loading branch information
InterLinked1 committed Nov 25, 2023
1 parent 576dc9a commit e9aab62
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 66 deletions.
25 changes: 14 additions & 11 deletions bbs/node.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ const char *bbs_name(void)
return bbs_name_buf;
}

static int lifetime_nodes = 0;
static unsigned int lifetime_nodes = 0;

struct bbs_node *__bbs_node_request(int fd, const char *protname, void *mod)
{
Expand Down Expand Up @@ -269,11 +269,10 @@ struct bbs_node *__bbs_node_request(int fd, const char *protname, void *mod)
} else {
RWLIST_INSERT_HEAD(&nodes, node, entry); /* This is the first node. */
}
lifetime_nodes++;
node->lifetimeid = lifetime_nodes++;
RWLIST_UNLOCK(&nodes);

bbs_debug(1, "Allocated new node with ID %u\n", node->id);

bbs_debug(1, "Allocated new node with ID %u (lifetime ID %d)\n", node->id, node->lifetimeid);
return node;
}

Expand Down Expand Up @@ -686,7 +685,7 @@ static int cli_nodes(struct bbs_cli_args *a)
}
RWLIST_UNLOCK(&nodes);

bbs_dprintf(a->fdout, "%d active node%s, %d lifetime node%s\n", c, ESS(c), lifetime_nodes, ESS(lifetime_nodes));
bbs_dprintf(a->fdout, "%d active node%s, %u lifetime node%s\n", c, ESS(c), lifetime_nodes, ESS(lifetime_nodes));
return 0;
}

Expand Down Expand Up @@ -791,7 +790,6 @@ static int node_info(int fd, unsigned int nodenum)
char connecttime[29];
struct bbs_node *n;
char menufull[16];
int lwp;
time_t now = time(NULL);

RWLIST_RDLOCK(&nodes);
Expand Down Expand Up @@ -826,10 +824,9 @@ static int node_info(int fd, unsigned int nodenum)
bbs_dprintf(fd, BBS_FMT_S, title,fallback); \
}

lwp = bbs_pthread_tid(n->thread);

pthread_mutex_lock(&n->lock);
bbs_dprintf(fd, BBS_FMT_D, "#", n->id);
bbs_dprintf(fd, BBS_FMT_D, "Lifetime #", n->lifetimeid);
bbs_dprintf(fd, BBS_FMT_S, "Protocol", n->protname);
bbs_dprintf(fd, BBS_FMT_S, "IP Address", n->ip);
bbs_dprintf(fd, BBS_FMT_S, "Connected", connecttime);
Expand All @@ -842,9 +839,8 @@ static int node_info(int fd, unsigned int nodenum)
bbs_dprintf(fd, BBS_FMT_D, "Node PTY Master FD", n->amaster);
bbs_dprintf(fd, BBS_FMT_D, "Node PTY Slave FD", n->slavefd);
bbs_dprintf(fd, BBS_FMT_S, "Node PTY Slave Name", n->slavename);
if (lwp != -1) {
bbs_dprintf(fd, BBS_FMT_D, "Node Thread ID", lwp);
}
bbs_dprintf(fd, BBS_FMT_D, "Node PTY Thread ID", bbs_pthread_tid(n->ptythread));
bbs_dprintf(fd, BBS_FMT_D, "Node Thread ID", bbs_pthread_tid(n->thread));
bbs_dprintf(fd, BBS_FMT_S, "User", bbs_username(n->user));
if (bbs_user_is_guest(n->user)) {
bbs_dprintf(fd, BBS_FMT_S, "Guest Name/Alias", S_IF(n->user->guestname));
Expand Down Expand Up @@ -1403,8 +1399,15 @@ void bbs_node_begin(struct bbs_node *node)
bbs_auth("New %s connection to node %d from %s:%u\n", node->protname, node->id, node->ip, node->rport);
}

void bbs_node_net_begin(struct bbs_node *node)
{
node->thread = pthread_self();
bbs_node_begin(node);
}

void bbs_node_exit(struct bbs_node *node)
{
bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
if (node->active) {
/* User quit: unlink and free */
bbs_node_unlink(node);
Expand Down
9 changes: 8 additions & 1 deletion include/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct pollfd;

struct bbs_node {
unsigned int id; /*!< Node number, 1-indexed for user-friendliness */
unsigned int lifetimeid; /*!< Lifetime node number, 1-indexed */
int fd; /*!< Socket file descriptor */
int rfd; /*!< File descriptor for reading */
int wfd; /*!< File descriptor for writing */
Expand Down Expand Up @@ -685,13 +686,19 @@ int bbs_node_wait_key(struct bbs_node *node, int ms);
*/
void bbs_node_begin(struct bbs_node *node);

/*! \brief Begin handling a node from a network protocol
* \note Only intended for network protocols that don't use psuedoterminals
*/
void bbs_node_net_begin(struct bbs_node *node);

/*! \brief Stop handling a node
* \note Not needed if you use bbs_node_handler
* \note After calling this function, node will no longer be a valid reference
*/
void bbs_node_exit(struct bbs_node *node) __attribute__ ((nonnull (1)));

/*!
* \brief Top-level node handler
* \brief Top-level node handler for terminal protocols
* \param varg BBS node
*/
void *bbs_node_handler(void *varg);
8 changes: 2 additions & 6 deletions modules/mod_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -2651,13 +2651,9 @@ static void *__http_handler(void *varg)
{
struct bbs_node *node = varg;

node->thread = pthread_self();
bbs_node_begin(node);

bbs_node_net_begin(node);
http_handler(node, !strcmp(node->protname, "HTTPS") ? 1 : 0); /* Actually handle the HTTP/HTTPS client */

bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
bbs_node_exit(node); /* node is no longer a valid reference */
bbs_node_exit(node);
return NULL;
}

Expand Down
4 changes: 1 addition & 3 deletions nets/net_finger.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ static void *finger_handler(void *varg)
int verbose = 0;
ssize_t res;

/* This thread is running instead of the normal node handler thread */
/* Remember, no pseudoterminal is allocated for this node! Can NOT use normal bbs_ I/O functions. */
bbs_node_begin(node);
bbs_node_net_begin(node);

/* A RUIP (Remote User Information Program) client has connected to us.
* Note that the GNU finger client doesn't seem to work for me but the Windows one does. Go figure.
Expand Down
4 changes: 1 addition & 3 deletions nets/net_ftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,7 @@ static void *ftp_handler(void *varg)
struct ftp_session ftpstack, *ftp;
SSL *ssl = NULL, *ssl2 = NULL;

/* This thread is running instead of the normal node handler thread */
/* Remember, no pseudoterminal is allocated for this node! Can NOT use normal bbs_ I/O functions. */
bbs_node_begin(node);
bbs_node_net_begin(node);

if (bbs_get_local_ip(our_ip, sizeof(our_ip))) { /* Determine it just once, now */
goto cleanup;
Expand Down
4 changes: 1 addition & 3 deletions nets/net_gopher.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ static void *gopher_handler(void *varg)
struct stat st;
ssize_t res;

/* This thread is running instead of the normal node handler thread */
/* Remember, no pseudoterminal is allocated for this node! Can NOT use normal bbs_ I/O functions. */
bbs_node_begin(node);
bbs_node_net_begin(node);

/* This is not buffered since there's no pseudoterminal. */
res = bbs_poll_read(node->fd, 1000, buf, sizeof(buf) - 1); /* Read the retrieval/selector string from the client */
Expand Down
8 changes: 2 additions & 6 deletions nets/net_imap.c
Original file line number Diff line number Diff line change
Expand Up @@ -4691,13 +4691,9 @@ static void *__imap_handler(void *varg)
{
struct bbs_node *node = varg;

node->thread = pthread_self();
bbs_node_begin(node);

bbs_node_net_begin(node);
imap_handler(node, !strcmp(node->protname, "IMAPS"));

bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
bbs_node_exit(node); /* node is no longer a valid reference */
bbs_node_exit(node);
return NULL;
}

Expand Down
8 changes: 2 additions & 6 deletions nets/net_irc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3452,13 +3452,9 @@ static void *__irc_handler(void *varg)
{
struct bbs_node *node = varg;

node->thread = pthread_self();
bbs_node_begin(node);

bbs_node_net_begin(node);
irc_handler(node, !strcmp(node->protname, "IRCS") ? 1 : 0); /* Actually handle the IRC/IRCS client */

bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
bbs_node_exit(node); /* node is no longer a valid reference */
bbs_node_exit(node);
return NULL;
}

Expand Down
8 changes: 2 additions & 6 deletions nets/net_nntp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1303,13 +1303,9 @@ static void *__nntp_handler(void *varg)
{
struct bbs_node *node = varg;

node->thread = pthread_self();
bbs_node_begin(node);

bbs_node_net_begin(node);
nntp_handler(node, !strcmp(node->protname, "NNTPS"), !strcmp(node->protname, "NNSP") ? NNTP_MODE_TRANSIT: NNTP_MODE_READER);

bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
bbs_node_exit(node); /* node is no longer a valid reference */
bbs_node_exit(node);
return NULL;
}

Expand Down
8 changes: 2 additions & 6 deletions nets/net_pop3.c
Original file line number Diff line number Diff line change
Expand Up @@ -784,13 +784,9 @@ static void *__pop3_handler(void *varg)
{
struct bbs_node *node = varg;

node->thread = pthread_self();
bbs_node_begin(node);

bbs_node_net_begin(node);
pop3_handler(node, !strcmp(node->protname, "POP3S"));

bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
bbs_node_exit(node); /* node is no longer a valid reference */
bbs_node_exit(node);
return NULL;
}

Expand Down
6 changes: 2 additions & 4 deletions nets/net_sieve.c
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,7 @@ static void *__sieve_handler(void *varg)
#endif
struct sieve_session sieve;

node->thread = pthread_self();
bbs_node_begin(node);
bbs_node_net_begin(node);

memset(&sieve, 0, sizeof(sieve));
sieve.rfd = sieve.wfd = node->fd;
Expand All @@ -516,8 +515,7 @@ static void *__sieve_handler(void *varg)
#endif
sieve_cleanup(&sieve);

bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
bbs_node_exit(node); /* node is no longer a valid reference */
bbs_node_exit(node);
return NULL;
}

Expand Down
7 changes: 2 additions & 5 deletions nets/net_smtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2646,14 +2646,11 @@ static void *__smtp_handler(void *varg)
struct bbs_node *node = varg;
int secure = !strcmp(node->protname, "SMTPS") ? 1 : 0;

node->thread = pthread_self();
bbs_node_begin(node);
bbs_node_net_begin(node);

/* If it's secure, it's for message submission agent, MTAs are never secure by default. */
smtp_handler(node, secure || !strcmp(node->protname, "SMTP (MSA)"), secure); /* Actually handle the SMTP/SMTPS/message submission agent client */

bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
bbs_node_exit(node); /* node is no longer a valid reference */
bbs_node_exit(node);
return NULL;
}

Expand Down
2 changes: 2 additions & 0 deletions nets/net_telnet.c
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ static void *telnets_handler(void *varg)

if (!telnet_handshake(node)) {
bbs_node_handler(node); /* Run the normal node handler */
} else {
bbs_node_exit(node); /* Manual cleanup */
}

ssl_close(ssl);
Expand Down
8 changes: 2 additions & 6 deletions nets/net_ws.c
Original file line number Diff line number Diff line change
Expand Up @@ -1174,13 +1174,9 @@ static void *__ws_handler(void *varg)
{
struct bbs_node *node = varg;

node->thread = pthread_self();
bbs_node_begin(node);

bbs_node_net_begin(node);
ws_direct_handler(node, !strcmp(node->protname, "WSS"));

bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
bbs_node_exit(node); /* node is no longer a valid reference */
bbs_node_exit(node);
return NULL;
}

Expand Down

0 comments on commit e9aab62

Please sign in to comment.