Skip to content

Commit

Permalink
Fix locking when writing to connections
Browse files Browse the repository at this point in the history
There are 2 race conditions when writing to connections:
- Since we don't hold the semaphore during iteration, a connection can
  be removed by another thread while we try to use it, which will lead
  to use after free.
- Multiple threads may try to write to the same connection socket,
  corrupting the packets (lima-vm#39).

Both issues are fixed by holding the semaphore during iteration and
writing to the socket. This is not the most efficient way but
socket_vmnet crashes daily and we must stop the bleeding first. We can
to add more fine grain locking later.
  • Loading branch information
nirs committed Sep 16, 2024
1 parent 503cdc5 commit 40ade7f
Showing 1 changed file with 4 additions and 6 deletions.
10 changes: 4 additions & 6 deletions main.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,7 @@ static void _on_vmnet_packets_available(interface_ref iface, int64_t buf_count,
dest_mac[5], src_mac[0], src_mac[1], src_mac[2], src_mac[3],
src_mac[4], src_mac[5]);
dispatch_semaphore_wait(state->sem, DISPATCH_TIME_FOREVER);
struct conn *conns = state->conns;
dispatch_semaphore_signal(state->sem);
for (struct conn *conn = conns; conn != NULL; conn = conn->next) {
for (struct conn *conn = state->conns; conn != NULL; conn = conn->next) {
// FIXME: avoid flooding
DEBUGF("[Handler i=%d] Sending to the socket %d: 4 + %ld bytes [Dest "
"%02X:%02X:%02X:%02X:%02X:%02X]",
Expand All @@ -197,6 +195,7 @@ static void _on_vmnet_packets_available(interface_ref iface, int64_t buf_count,
goto done;
}
}
dispatch_semaphore_signal(state->sem);
}
done:
if (pdv != NULL) {
Expand Down Expand Up @@ -518,9 +517,7 @@ static void on_accept(struct state *state, int accept_fd, interface_ref iface) {
// (Not handled by vmnet)
// FIXME: avoid flooding
dispatch_semaphore_wait(state->sem, DISPATCH_TIME_FOREVER);
struct conn *conns = state->conns;
dispatch_semaphore_signal(state->sem);
for (struct conn *conn = conns; conn != NULL; conn = conn->next) {
for (struct conn *conn = state->conns; conn != NULL; conn = conn->next) {
if (conn->socket_fd == accept_fd)
continue;
DEBUGF("[Socket-to-Socket i=%lld] Sending from socket %d to socket %d: "
Expand All @@ -545,6 +542,7 @@ static void on_accept(struct state *state, int accept_fd, interface_ref iface) {
continue;
}
}
dispatch_semaphore_signal(state->sem);
}
done:
INFOF("Closing a connection (fd %d)", accept_fd);
Expand Down

0 comments on commit 40ade7f

Please sign in to comment.