yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-nbd-Fully-initialize-client-in-case-of-failed-negoti.patch

76daa3
From 7c2ce59d5622187490728e29a91710a470d1c5b4 Mon Sep 17 00:00:00 2001
76daa3
From: Eric Blake <eblake@redhat.com>
76daa3
Date: Fri, 9 Jun 2017 21:56:07 +0200
76daa3
Subject: [PATCH 02/13] nbd: Fully initialize client in case of failed
76daa3
 negotiation
76daa3
76daa3
RH-Author: Eric Blake <eblake@redhat.com>
76daa3
Message-id: <20170609215608.27053-2-eblake@redhat.com>
76daa3
Patchwork-id: 75571
76daa3
O-Subject: [RHEV-7.4 qemu-kvm-rhev PATCH 1/2] nbd: Fully initialize client in case of failed negotiation
76daa3
Bugzilla: 1447948
76daa3
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
76daa3
RH-Acked-by: Thomas Huth <thuth@redhat.com>
76daa3
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
76daa3
76daa3
If a non-NBD client connects to qemu-nbd, we would end up with
76daa3
a SIGSEGV in nbd_client_put() because we were trying to
76daa3
unregister the client's association to the export, even though
76daa3
we skipped inserting the client into that list.  Easy trigger
76daa3
in two terminals:
76daa3
76daa3
$ qemu-nbd -p 30001 --format=raw file
76daa3
$ nmap 127.0.0.1 -p 30001
76daa3
76daa3
nmap claims that it thinks it connected to a pago-services1
76daa3
server (which probably means nmap could be updated to learn the
76daa3
NBD protocol and give a more accurate diagnosis of the open
76daa3
port - but that's not our problem), then terminates immediately,
76daa3
so our call to nbd_negotiate() fails.  The fix is to reorder
76daa3
nbd_co_client_start() to ensure that all initialization occurs
76daa3
before we ever try talking to a client in nbd_negotiate(), so
76daa3
that the teardown sequence on negotiation failure doesn't fault
76daa3
while dereferencing a half-initialized object.
76daa3
76daa3
While debugging this, I also noticed that nbd_update_server_watch()
76daa3
called by nbd_client_closed() was still adding a channel to accept
76daa3
the next client, even when the state was no longer RUNNING.  That
76daa3
is fixed by making nbd_can_accept() pay attention to the current
76daa3
state.
76daa3
76daa3
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1451614
76daa3
76daa3
Signed-off-by: Eric Blake <eblake@redhat.com>
76daa3
Message-Id: <20170527030421.28366-1-eblake@redhat.com>
76daa3
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
76daa3
(cherry picked from commit ???)
76daa3
https://bugzilla.redhat.com/show_bug.cgi?id=1447948
76daa3
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
76daa3
---
76daa3
 nbd/server.c | 8 +++-----
76daa3
 qemu-nbd.c   | 2 +-
76daa3
 2 files changed, 4 insertions(+), 6 deletions(-)
76daa3
76daa3
diff --git a/nbd/server.c b/nbd/server.c
76daa3
index 924a1fe..edfda84 100644
76daa3
--- a/nbd/server.c
76daa3
+++ b/nbd/server.c
76daa3
@@ -1376,16 +1376,14 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
76daa3
 
76daa3
     if (exp) {
76daa3
         nbd_export_get(exp);
76daa3
+        QTAILQ_INSERT_TAIL(&exp->clients, client, next);
76daa3
     }
76daa3
+    qemu_co_mutex_init(&client->send_lock);
76daa3
+
76daa3
     if (nbd_negotiate(data)) {
76daa3
         client_close(client);
76daa3
         goto out;
76daa3
     }
76daa3
-    qemu_co_mutex_init(&client->send_lock);
76daa3
-
76daa3
-    if (exp) {
76daa3
-        QTAILQ_INSERT_TAIL(&exp->clients, client, next);
76daa3
-    }
76daa3
 
76daa3
     nbd_client_receive_next_request(client);
76daa3
 
76daa3
diff --git a/qemu-nbd.c b/qemu-nbd.c
76daa3
index e080fb7..b44764e 100644
76daa3
--- a/qemu-nbd.c
76daa3
+++ b/qemu-nbd.c
76daa3
@@ -324,7 +324,7 @@ out:
76daa3
 
76daa3
 static int nbd_can_accept(void)
76daa3
 {
76daa3
-    return nb_fds < shared;
76daa3
+    return state == RUNNING && nb_fds < shared;
76daa3
 }
76daa3
 
76daa3
 static void nbd_export_closed(NBDExport *exp)
76daa3
-- 
76daa3
1.8.3.1
76daa3