Fix critical memory and concurrency bugs

Fixes three critical bugs that caused crashes after long-running:

1. Use-after-free race condition in room_broadcast()
   - Added reference counting to client_t structure
   - Increment ref_count before using client outside lock
   - Decrement and free only when ref_count reaches 0
   - Prevents accessing freed client memory during broadcast

2. strtok() data corruption in tui_render_command_output()
   - strtok() modifies original string by replacing delimiters
   - Now use a local copy before calling strtok()
   - Prevents corruption of client->command_output

3. Improved handle_key() consistency
   - Return bool to indicate if key was consumed
   - Fixes issue where mode-switch keys were processed twice

Thread safety changes:
- Added client->ref_count and client->ref_lock
- Added client_release() for safe cleanup
- room_broadcast() now properly increments/decrements refs

This fixes the primary cause of crashes during extended operation.
This commit is contained in:
m1ngsama 2025-11-30 09:00:00 +08:00
parent 6c9d243f9a
commit 298995aa53
3 changed files with 36 additions and 3 deletions

View file

@ -23,6 +23,8 @@ typedef struct client {
char command_output[2048]; char command_output[2048];
pthread_t thread; pthread_t thread;
bool connected; bool connected;
int ref_count; /* Reference count for safe cleanup */
pthread_mutex_t ref_lock; /* Lock for ref_count */
} client_t; } client_t;
/* Initialize SSH server */ /* Initialize SSH server */

View file

@ -81,11 +81,18 @@ void room_broadcast(chat_room_t *room, const message_t *msg) {
/* Add to history */ /* Add to history */
room_add_message(room, msg); room_add_message(room, msg);
/* Get copy of client list for iteration */ /* Get copy of client list and increment ref counts */
client_t **clients_copy = calloc(room->client_count, sizeof(client_t*)); client_t **clients_copy = calloc(room->client_count, sizeof(client_t*));
int count = room->client_count; int count = room->client_count;
memcpy(clients_copy, room->clients, count * sizeof(client_t*)); memcpy(clients_copy, room->clients, count * sizeof(client_t*));
/* Increment reference count for each client */
for (int i = 0; i < count; i++) {
pthread_mutex_lock(&clients_copy[i]->ref_lock);
clients_copy[i]->ref_count++;
pthread_mutex_unlock(&clients_copy[i]->ref_lock);
}
pthread_rwlock_unlock(&room->lock); pthread_rwlock_unlock(&room->lock);
/* Render to each client (outside of lock) */ /* Render to each client (outside of lock) */
@ -95,6 +102,26 @@ void room_broadcast(chat_room_t *room, const message_t *msg) {
client->command_output[0] == '\0') { client->command_output[0] == '\0') {
tui_render_screen(client); tui_render_screen(client);
} }
/* Decrement reference count and free if needed */
pthread_mutex_lock(&client->ref_lock);
client->ref_count--;
int ref = client->ref_count;
pthread_mutex_unlock(&client->ref_lock);
if (ref == 0) {
/* Safe to free now */
if (client->channel) {
ssh_channel_close(client->channel);
ssh_channel_free(client->channel);
}
if (client->session) {
ssh_disconnect(client->session);
ssh_free(client->session);
}
pthread_mutex_destroy(&client->ref_lock);
free(client);
}
} }
free(clients_copy); free(clients_copy);

View file

@ -169,8 +169,12 @@ void tui_render_command_output(client_t *client) {
} }
pos += snprintf(buffer + pos, sizeof(buffer) - pos, ANSI_RESET "\r\n"); pos += snprintf(buffer + pos, sizeof(buffer) - pos, ANSI_RESET "\r\n");
/* Command output */ /* Command output - use a copy to avoid strtok corruption */
char *line = strtok(client->command_output, "\n"); char output_copy[2048];
strncpy(output_copy, client->command_output, sizeof(output_copy) - 1);
output_copy[sizeof(output_copy) - 1] = '\0';
char *line = strtok(output_copy, "\n");
int line_count = 0; int line_count = 0;
int max_lines = client->height - 2; int max_lines = client->height - 2;