[v2,for-9.1,0/3] Avoid NBD crash on nbd-server-stop

Message

Eric Blake Aug. 2, 2024, 1:32 a.m. UTC

v1 was here:
https://lists.gnu.org/archive/html/qemu-devel/2024-06/msg01609.html

Since then, I've applied with Red Hat to get a CVE assigned (a client
not using TLS should never be able to cause the destination qemu to
crash during live storage migration), and will update to include that
number once it is assigned.  But since the issue has already been on
list, I see no need to further embargo this patch attempt.

I've also concluded from my audit that Alexander's initial hunch was
right: there's nothing simple we can do to prevent qio coroutines from
reaching our callback into nbd_blockdev_client_closed() after a QMP
nbd-server-stop (I could not find a graceful way to close the qio
channel and then forcefully wait for the coroutine to finish), so that
function has to be prepared to handle late clients, but the fix in
patch 1 is a bit more involved than Alexander's attempt since we must
also not corrupt a subsequent nbd-server-start.  Patch 2 is
insufficient on its own to prevent the problems, but reduces the
amount of time that a client can hang on to server resources after
nbd-server-stop (the client will see EPIPE rather than being able to
carry on a prolonged NBD_OPT_* conversation).

[I'm also aware of some Coverity analysis pointing to potential race
conditions in block/nbd.c; if those need fixes, I hope to also post
patches for those in time for inclusion in the same pull request that
picks up this series]

Eric Blake (3):
  nbd: CVE-XXX: Use cookie to track generation of nbd-server
  nbd: CVE-XXX: Close stray client sockets at server shutdown
  nbd: Minor style fixes

 include/block/nbd.h |  3 ++-
 blockdev-nbd.c      | 39 ++++++++++++++++++++++++++++++++++-----
 nbd/server.c        | 14 +++++++++-----
 qemu-nbd.c          |  8 +++++---
 4 files changed, 50 insertions(+), 14 deletions(-)