From c8c7dc7a445892a820223be2e7617a790e0400f5 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 7 Aug 2024 08:50:01 -0500 Subject: [PATCH 2/5] nbd/server: Plumb in new args to nbd_client_add() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Eric Blake RH-MergeRequest: 386: nbd/server: fix CVE-2024-7409 (qemu crash on nbd-server-stop) [rhel-9.4.z] RH-Jira: RHEL-52616 RH-Acked-by: Miroslav Rezanina RH-Commit: [1/4] 658d80c80a1163fcad456a34ec20d7828273a36c (ebblake/qemu-kvm) Upcoming patches to fix a CVE need to track an opaque pointer passed in by the owner of a client object, as well as request for a time limit on how fast negotiation must complete. Prepare for that by changing the signature of nbd_client_new() and adding an accessor to get at the opaque pointer, although for now the two servers (qemu-nbd.c and blockdev-nbd.c) do not change behavior even though they pass in a new default timeout value. Suggested-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Eric Blake Message-ID: <20240807174943.771624-11-eblake@redhat.com> Reviewed-by: Daniel P. Berrangé [eblake: s/LIMIT/MAX_SECS/ as suggested by Dan] Signed-off-by: Eric Blake (cherry picked from commit fb1c2aaa981e0a2fa6362c9985f1296b74f055ac) Jira: https://issues.redhat.com/browse/RHEL-52616 Signed-off-by: Eric Blake --- blockdev-nbd.c | 6 ++++-- include/block/nbd.h | 11 ++++++++++- nbd/server.c | 20 +++++++++++++++++--- qemu-nbd.c | 4 +++- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 213012435f..267a1de903 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -64,8 +64,10 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nbd_update_server_watch(nbd_server); qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); - nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz, - nbd_blockdev_client_closed); + /* TODO - expose handshake timeout as QMP option */ + nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS, + nbd_server->tlscreds, nbd_server->tlsauthz, + nbd_blockdev_client_closed, NULL); } static void nbd_update_server_watch(NBDServerData *s) diff --git a/include/block/nbd.h b/include/block/nbd.h index 4e7bd6342f..1d4d65922d 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -33,6 +33,12 @@ typedef struct NBDMetaContexts NBDMetaContexts; extern const BlockExportDriver blk_exp_nbd; +/* + * NBD_DEFAULT_HANDSHAKE_MAX_SECS: Number of seconds in which client must + * succeed at NBD_OPT_GO before being forcefully dropped as too slow. + */ +#define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10 + /* Handshake phase structs - this struct is passed on the wire */ typedef struct NBDOption { @@ -403,9 +409,12 @@ AioContext *nbd_export_aio_context(NBDExport *exp); NBDExport *nbd_export_find(const char *name); void nbd_client_new(QIOChannelSocket *sioc, + uint32_t handshake_max_secs, QCryptoTLSCreds *tlscreds, const char *tlsauthz, - void (*close_fn)(NBDClient *, bool)); + void (*close_fn)(NBDClient *, bool), + void *owner); +void *nbd_client_owner(NBDClient *client); void nbd_client_get(NBDClient *client); void nbd_client_put(NBDClient *client); diff --git a/nbd/server.c b/nbd/server.c index 892797bb11..e50012499f 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -124,12 +124,14 @@ struct NBDMetaContexts { struct NBDClient { int refcount; /* atomic */ void (*close_fn)(NBDClient *client, bool negotiated); + void *owner; QemuMutex lock; NBDExport *exp; QCryptoTLSCreds *tlscreds; char *tlsauthz; + uint32_t handshake_max_secs; QIOChannelSocket *sioc; /* The underlying data channel */ QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */ @@ -3191,6 +3193,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque) qemu_co_mutex_init(&client->send_lock); + /* TODO - utilize client->handshake_max_secs */ if (nbd_negotiate(client, &local_err)) { if (local_err) { error_report_err(local_err); @@ -3205,14 +3208,17 @@ static coroutine_fn void nbd_co_client_start(void *opaque) } /* - * Create a new client listener using the given channel @sioc. + * Create a new client listener using the given channel @sioc and @owner. * Begin servicing it in a coroutine. When the connection closes, call - * @close_fn with an indication of whether the client completed negotiation. + * @close_fn with an indication of whether the client completed negotiation + * within @handshake_max_secs seconds (0 for unbounded). */ void nbd_client_new(QIOChannelSocket *sioc, + uint32_t handshake_max_secs, QCryptoTLSCreds *tlscreds, const char *tlsauthz, - void (*close_fn)(NBDClient *, bool)) + void (*close_fn)(NBDClient *, bool), + void *owner) { NBDClient *client; Coroutine *co; @@ -3225,13 +3231,21 @@ void nbd_client_new(QIOChannelSocket *sioc, object_ref(OBJECT(client->tlscreds)); } client->tlsauthz = g_strdup(tlsauthz); + client->handshake_max_secs = handshake_max_secs; client->sioc = sioc; qio_channel_set_delay(QIO_CHANNEL(sioc), false); object_ref(OBJECT(client->sioc)); client->ioc = QIO_CHANNEL(sioc); object_ref(OBJECT(client->ioc)); client->close_fn = close_fn; + client->owner = owner; co = qemu_coroutine_create(nbd_co_client_start, client); qemu_coroutine_enter(co); } + +void * +nbd_client_owner(NBDClient *client) +{ + return client->owner; +} diff --git a/qemu-nbd.c b/qemu-nbd.c index bac0b5e3ec..01b4a63c31 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -389,7 +389,9 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nb_fds++; nbd_update_server_watch(); - nbd_client_new(cioc, tlscreds, tlsauthz, nbd_client_closed); + /* TODO - expose handshake timeout as command line option */ + nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS, + tlscreds, tlsauthz, nbd_client_closed, NULL); } static void nbd_update_server_watch(void) -- 2.39.3