remote: fix stale status snapshot in daemonStreamHandleWrite

daemonStreamHandleWrite() snapshots status from msg->header.status
before dispatching to the per-type handler, then reuses that stale
snapshot afterwards to decide whether to send a fake release reply.
A stale snapshot lets it send that reply twice for the same 'msg',
linking the message to itself and deadlooping the event loop thread
in virNetMessageQueuePush()'s tail-walk loop.

Remove the 'status' variable and read msg->header.status directly
at the point of use instead. Commit 411cbe7199c ("remote: fix
stream use-after-free") introduced it alongside its real fix: it
also moved virNetMessageQueueServe() to dequeue 'msg' from
stream->rx before dispatch instead of after, which is what actually
prevents corrupting stream->rx's linkage and remains unchanged
here. The variable itself was never load-bearing: no dispatch
handler frees 'msg' on the path that reaches this check, so reading
msg->header.status straight from 'msg' cannot go stale.

Fixes: 411cbe7199ce ("remote: fix stream use-after-free")
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
index 3777c8e684924021d1d26d9e05101a63c58f6845..437cec374ff173a5c0635dd9561ad1a3a2feb4a5 100644 (file)
--- a/src/remote/remote_daemon_stream.c
+++ b/src/remote/remote_daemon_stream.c
@@ -736,7 +736,6 @@ static int
 daemonStreamHandleWrite(virNetServerClient *client,
                         daemonClientStream *stream)
 {
-    virNetMessageStatus status = VIR_NET_OK;
     VIR_DEBUG("client=%p, stream=%p", client, stream);
 
     while (stream->rx && !stream->closed) {
@@ -748,10 +747,8 @@ daemonStreamHandleWrite(virNetServerClient *client,
              * Otherwise just carry on with processing stream
              * data. */
             ret = daemonStreamHandleHole(client, stream, msg);
-            status = msg->header.status;
         } else if (msg->header.type == VIR_NET_STREAM) {
-            status = msg->header.status;
-            switch (status) {
+            switch (msg->header.status) {
             case VIR_NET_OK:
                 ret = daemonStreamHandleFinish(client, stream, msg);
                 break;
@@ -791,7 +788,7 @@ daemonStreamHandleWrite(virNetServerClient *client,
          * onto the wire, but this causes the client to reset
          * its active request count / throttling
          */
-        if (status == VIR_NET_CONTINUE) {
+        if (msg->header.status == VIR_NET_CONTINUE) {
             virNetMessageClear(msg);
             msg->header.type = VIR_NET_REPLY;
             if (virNetServerClientSendMessage(client, msg) < 0) {