[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
This email originated from an IP that might not be authorized by the domain it was sent from. Do not click links or open attachments unless it is an email you expected to receive. 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) {
On 7/22/26 10:36, Denis V. Lunev wrote: ping
This email originated from an IP that might not be authorized by the domain it was sent from. Do not click links or open attachments unless it is an email you expected to receive. 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) {
On 7/22/26 10:36, Denis V. Lunev wrote: ping v2
On 7/22/26 10:36, Denis V. Lunev via Devel wrote:
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(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
e791c090ea75 has landed with a wrong identity, fix it in the same way as commit 103fa13ddb23 did for others. Signed-off-by: Denis V. Lunev <den@openvz.org> --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index b12ee4308f..e82be1eb50 100644 --- a/.mailmap +++ b/.mailmap @@ -88,6 +88,7 @@ Marco Bozzolan <bozzolan@gmail.com> <redshift@gmx.com> # from their Signed-off-by trailer. Achill Gilgenast <achill@achill.org> Achill Gilgenast via Devel <devel@lists.libvirt.org> Akash Kulhalli <akash.kulhalli@oracle.com> Akash Kulhalli via Devel <devel@lists.libvirt.org> +Denis V. Lunev <den@openvz.org> Denis V. Lunev via Devel <devel@lists.libvirt.org> Enrique Llorente <ellorent@redhat.com> Enrique Llorente via Devel <devel@lists.libvirt.org> Han Han <hhan@redhat.com> Han Han via Devel <devel@lists.libvirt.org> Jim Fehlig <jfehlig@suse.com> Jim Fehlig via Devel <devel@lists.libvirt.org> -- 2.53.0
On Mon, Aug 10, 2026 at 04:24:07PM +0200, Denis V. Lunev via Devel wrote:
e791c090ea75 has landed with a wrong identity, fix it in the same way as commit 103fa13ddb23 did for others.
Opps :-(
Signed-off-by: Denis V. Lunev <den@openvz.org> --- .mailmap | 1 + 1 file changed, 1 insertion(+)
I've pushed this. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
On Thu, Aug 13, 2026 at 11:28:52AM +0100, Daniel P. Berrangé via Devel wrote:
On Mon, Aug 10, 2026 at 04:24:07PM +0200, Denis V. Lunev via Devel wrote:
e791c090ea75 has landed with a wrong identity, fix it in the same way as commit 103fa13ddb23 did for others.
Opps :-(
BTW, to avoid this in future for all projects use: $ git config --global format.from "Your Name <your@email.com>" $ git config --global format.forceInBodyFrom true That forces git to add an extra "From" line that in the body which git will use in preference to the mail header when applying the patch. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
participants (4)
-
Daniel P. Berrangé -
Denis V. Lunev -
Denis V. Lunev -
Michal Prívozník