Skip to content

Commit

Permalink
io, modules: Fix memory errors and leaks.
Browse files Browse the repository at this point in the history
* io_tls: Ensure that the TLS session list is rebuilt immediately
  whenever SSL sessions are removed from the list (and subsequently
  freed). Although the list is locked for removal, in the main
  TLS I/O loop, the list isn't rebuilt during an iteration, only
  at the beginning, so if a session was removed at the beginning
  of the function, prior to the poll call, it was possible for now
  freed sessions to be accessed during the session traversal. To
  prevent this, force a list rebuild immediately in these cases.
  This addresses a wide variety of memory corruption / invalid
  accesses previously caused by this module.
* mod_oauth: Run cleanup code on success, not just failure.
* mod_webmail: Run clenaup code on off-nominal failure case.
  • Loading branch information
InterLinked1 committed Oct 8, 2024
1 parent c94e012 commit 4a854f9
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
21 changes: 19 additions & 2 deletions io/io_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,20 @@ static void ssl_fd_free(struct ssl_fd *sfd)
free(sfd);
}

static int needcreate = 1; /* Whether the list of TLS sessions is stale and needs to be rebuilt */

static int ssl_unregister_fd(SSL *ssl)
{
struct ssl_fd *sfd;

sfd = RWLIST_WRLOCK_REMOVE_BY_FIELD(&sslfds, ssl, ssl, entry);
RWLIST_WRLOCK(&sslfds);
sfd = RWLIST_REMOVE_BY_FIELD(&sslfds, ssl, ssl, entry);
if (sfd) {
/* Any list of SSL sessions is now stale, since it
* could potentially include the session we just removed. */
needcreate = 1;
}
RWLIST_UNLOCK(&sslfds);
if (sfd) {
ssl_fd_free(sfd);
bbs_alertpipe_write(ssl_alert_pipe); /* Notify I/O thread that we removed a fd, although it'll probably detect this anyways. */
Expand All @@ -261,6 +270,7 @@ static void ssl_cleanup_fds(void)
ssl_fd_free(sfd);
c++;
}
needcreate = 1;
RWLIST_UNLOCK(&sslfds);
if (c) {
bbs_warning("Forcibly removed %d SSL file descriptor%s\n", c, ESS(c));
Expand Down Expand Up @@ -322,7 +332,6 @@ static void *ssl_io_thread(void *unused)
* since we print these out, and nfds_t is signed on some platforms (e.g. Linux)
* and unsigned on others (e.g. FreeBSD), so we can't portably print them. */
int i, prevfds = 0, oldnumfds = 0, numfds = 0, numssl = 0;
int needcreate = 1;
char buf[8192];
int pending;
int inovertime = 0, overtime = 0;
Expand Down Expand Up @@ -425,6 +434,14 @@ static void *ssl_io_thread(void *unused)
inovertime = 1;
}
RWLIST_RDLOCK(&sslfds);
/* Now that we've acquired the lock, check if any sessions are stale.
* If so, we need to rebuild the list before iterating again,
* to avoid using sessions that were removed and may now be freed. */
if (needcreate) {
RWLIST_UNLOCK(&sslfds);
bbs_debug(4, "TLS session list has become stale since loop iteration began, rebuilding again...\n");
continue;
}
for (i = 0; res > 0 && i < numfds; i++) {
int ores;
ssize_t wres;
Expand Down
2 changes: 2 additions & 0 deletions modules/mod_oauth.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ static int refresh_token(struct oauth_client *client)
}

bbs_verb(4, "Refreshed OAuth token '%s' (good for %ds)\n", client->name, expires);
json_decref(json);
bbs_curl_free(&c);
return 0;

cleanup:
Expand Down
2 changes: 1 addition & 1 deletion modules/mod_webmail.c
Original file line number Diff line number Diff line change
Expand Up @@ -3499,7 +3499,7 @@ static int on_text_message(struct ws_session *ws, void *data, const char *buf, s
} else {
bbs_warning("Command unknown: %s\n", command);
}
return res;
goto cleanup;
}

if (!strcmp(command, "LIST")) {
Expand Down

0 comments on commit 4a854f9

Please sign in to comment.