Skip to content

Commit 3874f5f

Browse files
committed
nbd/server: CVE-2024-7409: Avoid use-after-free when closing server
Commit 3e7ef73 plugged the use-after-free of the global nbd_server object, but overlooked a use-after-free of nbd_server->listener. Although this race is harder to hit, notice that our shutdown path first drops the reference count of nbd_server->listener, then triggers actions that can result in a pending client reaching the nbd_blockdev_client_closed() callback, which in turn calls qio_net_listener_set_client_func on a potentially stale object. If we know we don't want any more clients to connect, and have already told the listener socket to shut down, then we should not be trying to update the listener socket's associated function. Reproducer: > #!/usr/bin/python3 > > import os > from threading import Thread > > def start_stop(): > while 1: > os.system('virsh qemu-monitor-command VM \'{"execute": "nbd-server-start", +"arguments":{"addr":{"type":"unix","data":{"path":"/tmp/nbd-sock"}}}}\'') > os.system('virsh qemu-monitor-command VM \'{"execute": "nbd-server-stop"}\'') > > def nbd_list(): > while 1: > os.system('/path/to/build/qemu-nbd -L -k /tmp/nbd-sock') > > def test(): > sst = Thread(target=start_stop) > sst.start() > nlt = Thread(target=nbd_list) > nlt.start() > > sst.join() > nlt.join() > > test() Fixes: CVE-2024-7409 Fixes: 3e7ef73 ("nbd/server: CVE-2024-7409: Close stray clients at server-stop") CC: [email protected] Reported-by: Andrey Drobyshev <[email protected]> Signed-off-by: Eric Blake <[email protected]> Message-ID: <[email protected]> Reviewed-by: Stefan Hajnoczi <[email protected]>
1 parent f259e4c commit 3874f5f

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

blockdev-nbd.c

+8-4
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,13 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
9292

9393
static void nbd_update_server_watch(NBDServerData *s)
9494
{
95-
if (!s->max_connections || s->connections < s->max_connections) {
96-
qio_net_listener_set_client_func(s->listener, nbd_accept, NULL, NULL);
97-
} else {
98-
qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
95+
if (s->listener) {
96+
if (!s->max_connections || s->connections < s->max_connections) {
97+
qio_net_listener_set_client_func(s->listener, nbd_accept, NULL,
98+
NULL);
99+
} else {
100+
qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
101+
}
99102
}
100103
}
101104

@@ -113,6 +116,7 @@ static void nbd_server_free(NBDServerData *server)
113116
*/
114117
qio_net_listener_disconnect(server->listener);
115118
object_unref(OBJECT(server->listener));
119+
server->listener = NULL;
116120
QLIST_FOREACH_SAFE(conn, &server->conns, next, tmp) {
117121
qio_channel_shutdown(QIO_CHANNEL(conn->cioc), QIO_CHANNEL_SHUTDOWN_BOTH,
118122
NULL);

0 commit comments

Comments
 (0)