| Message ID | 20240822143617.800419-2-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-9.1] nbd/server: CVE-2024-7409: Avoid use-after-free when closing server | expand |
On Thu, Aug 22, 2024 at 09:35:29AM -0500, Eric Blake wrote: > Commit 3e7ef738 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: 3e7ef738c8 ("nbd/server: CVE-2024-7409: Close stray clients at server-stop") > CC: qemu-stable@nongnu.org > Reported-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > blockdev-nbd.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/blockdev-nbd.c b/blockdev-nbd.c index f73409ae494..b36f41b7c5a 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -92,10 +92,13 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, static void nbd_update_server_watch(NBDServerData *s) { - if (!s->max_connections || s->connections < s->max_connections) { - qio_net_listener_set_client_func(s->listener, nbd_accept, NULL, NULL); - } else { - qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL); + if (s->listener) { + if (!s->max_connections || s->connections < s->max_connections) { + qio_net_listener_set_client_func(s->listener, nbd_accept, NULL, + NULL); + } else { + qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL); + } } } @@ -113,6 +116,7 @@ static void nbd_server_free(NBDServerData *server) */ qio_net_listener_disconnect(server->listener); object_unref(OBJECT(server->listener)); + server->listener = NULL; QLIST_FOREACH_SAFE(conn, &server->conns, next, tmp) { qio_channel_shutdown(QIO_CHANNEL(conn->cioc), QIO_CHANNEL_SHUTDOWN_BOTH, NULL);