aboutsummaryrefslogtreecommitdiff
path: root/blockdev-nbd.c
diff options
context:
space:
mode:
authorEric Blake <eblake@redhat.com>2017-06-08 17:26:17 -0500
committerPaolo Bonzini <pbonzini@redhat.com>2017-06-15 11:04:05 +0200
commit0c9390d978cbf61e8f16c9f580fa96b305c43568 (patch)
tree9f1f1489689110cfd167a50c9f058795f789cea9 /blockdev-nbd.c
parent457e03559dc57859eb12f993380ddeaeb73e9017 (diff)
nbd: Fix regression on resiliency to port scan
Back in qemu 2.5, qemu-nbd was immune to port probes (a transient server would not quit, regardless of how many probe connections came and went, until a connection actually negotiated). But we broke that in commit ee7d7aa when removing the return value to nbd_client_new(), although that patch also introduced a bug causing an assertion failure on a client that fails negotiation. We then made it worse during refactoring in commit 1a6245a (a segfault before we could even assert); the (masked) assertion was cleaned up in d3780c2 (still in 2.6), and just recently we finally fixed the segfault ("nbd: Fully intialize client in case of failed negotiation"). But that still means that ever since we added TLS support to qemu-nbd, we have been vulnerable to an ill-timed port-scan being able to cause a denial of service by taking down qemu-nbd before a real client has a chance to connect. Since negotiation is now handled asynchronously via coroutines, we no longer have a synchronous point of return by re-adding a return value to nbd_client_new(). So this patch instead wires things up to pass the negotiation status through the close_fn callback function. Simple test across two terminals: $ qemu-nbd -f raw -p 30001 file $ nmap 127.0.0.1 -p 30001 && \ qemu-io -c 'r 0 512' -f raw nbd://localhost:30001 Note that this patch does not change what constitutes successful negotiation (thus, a client must enter transmission phase before that client can be considered as a reason to terminate the server when the connection ends). Perhaps we may want to tweak things in a later patch to also treat a client that uses NBD_OPT_ABORT as being a 'successful' negotiation (the client correctly talked the NBD protocol, and informed us it was not going to use our export after all), but that's a discussion for another day. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1451614 Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170608222617.20376-1-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Diffstat (limited to 'blockdev-nbd.c')
-rw-r--r--blockdev-nbd.c6
1 files changed, 5 insertions, 1 deletions
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index dd0860f4a6..28f551a7b0 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -27,6 +27,10 @@ typedef struct NBDServerData {
static NBDServerData *nbd_server;
+static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
+{
+ nbd_client_put(client);
+}
static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition,
gpointer opaque)
@@ -46,7 +50,7 @@ static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition,
qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
nbd_client_new(NULL, cioc,
nbd_server->tlscreds, NULL,
- nbd_client_put);
+ nbd_blockdev_client_closed);
object_unref(OBJECT(cioc));
return TRUE;
}