1
0
mirror of https://github.com/RIOT-OS/RIOT.git synced 2024-12-29 04:50:03 +01:00

pkg/lwip: fix race in async sock API with event

In TCP server mode, the sock_tcp_t sockets are managed by the network
stack and can be reused if a previous connection is no longer in used.
However, an event may still be posted in the event queue when the socket
is reused. Wiping it will result in the `next` pointer in that event to
be NULL, which will cause the event handler fetching that event to
crash.

This adds an `event_cancel()` at two places:
1. Just before reusing the socket
2. During sock_tcp_disconnect()

The former catches issues in server mode e.g. when a connect has been
closed (e.g. due to timeout) and is reused before a pending event (e.g.
a timeout event) has been processed.

The letter may be an issue on client side. E.g. when `sock_tcp_t` was
allocated on stack and goes out of scope after `sock_tcp_disconnect`
but before the event handler was run.
This commit is contained in:
Marian Buschsieweke 2024-12-16 19:39:22 +01:00
parent 4044c854f2
commit 8ac3a430e7
No known key found for this signature in database
GPG Key ID: 758BD52517F79C41

View File

@ -85,6 +85,9 @@ int sock_tcp_listen(sock_tcp_queue_t *queue, const sock_tcp_ep_t *local,
queue->array = queue_array;
queue->len = queue_len;
queue->used = 0;
/* This wipe is important: We cancel pending events in async event API when
* reusing the socket. If the memory would be uninitialized, bad things
* would happen */
memset(queue->array, 0, sizeof(sock_tcp_t) * queue_len);
mutex_unlock(&queue->mutex);
@ -125,6 +128,14 @@ void sock_tcp_disconnect(sock_tcp_t *sock)
}
}
#ifdef SOCK_HAS_ASYNC_CTX
/* Cancel any pending event in the event queue */
if (sock->base.async_ctx.queue) {
event_cancel(sock->base.async_ctx.queue,
&sock->base.async_ctx.event.super);
}
#endif
mutex_unlock(&sock->mutex);
memset(&sock->mutex, 0, sizeof(mutex_t));
}
@ -240,6 +251,16 @@ int sock_tcp_accept(sock_tcp_queue_t *queue, sock_tcp_t **sock,
for (unsigned short i = 0; i < queue->len; i++) {
sock_tcp_t *s = &queue->array[i];
if (s->base.conn == NULL) {
#ifdef SOCK_HAS_ASYNC_CTX
/* If there still is an event pending, we cannot just wipe
* its memory but have to remove the event from the list
* first. We rely here sock_tcp_listen to zero-initialize
* the sockets. */
if (s->base.async_ctx.queue) {
event_cancel(s->base.async_ctx.queue,
&s->base.async_ctx.event.super);
}
#endif
_tcp_sock_init(s, tmp, queue);
queue->used++;
assert(queue->used > 0);