[PATCH 1/1] 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> --- src/remote/remote_daemon_stream.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c index 3777c8e684..437cec374f 100644 --- 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) { -- 2.53.0
participants (1)
-
Denis V. Lunev