From nobody Tue Sep 22 04:49:40 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 38.145.34.151 as permitted sender) client-ip=38.145.34.151; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 38.145.34.151 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [38.145.34.151]) by mx.zohomail.com with SMTPS id 1785746231167929.1491777437922; Mon, 3 Aug 2026 01:37:11 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 993) id 92908417CA; Mon, 3 Aug 2026 04:37:09 -0400 (EDT) Received: from [172.19.199.10] (unknown [10.16.107.18]) by lists.libvirt.org (Postfix) with ESMTP id E8D0F41B63; Mon, 3 Aug 2026 04:33:19 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 993) id DB3A53FB96; Sun, 2 Aug 2026 09:47:18 -0400 (EDT) Received: from v5244.v57ae4e16.euw1.send.eu.mailgun.net (v5244.v57ae4e16.euw1.send.eu.mailgun.net [161.38.204.244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with UTF8SMTPS id B475B3FAAD for ; Sun, 2 Aug 2026 09:47:16 -0400 (EDT) Received: from x870e.golder.lan (node-u79.pool-125-27.dynamic.nt-isp.net [125.27.152.229]) by 144294aa76f8373def79bdd025f3f4c8c9008d3202958d6ffc3f44d277f01fa0 with SMTP id 6a6f4935cd45e8032b6498c2 (version=TLS1.3, cipher=TLS_AES_128_GCM_SHA256); Sun, 02 Aug 2026 13:42:13 GMT X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=4.0.1 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=golder.org; q=dns/txt; s=s1; t=1785678435; x=1785685635; h=Content-Transfer-Encoding: Content-Type: MIME-Version: References: In-Reply-To: Message-ID: Date: Subject: Subject: Cc: To: To: From: From: Sender: Sender; bh=+OIwZ7TJz32FSyUXubSLcUo+7rcI5fLzxalOCuUYZzA=; b=nZBCwuKWat74ezJhVHqu0sIijVM7iJkXYlvfOcJbJDOlqKm63mA3RupuE9fVucLCOl4Ob0yzwhXd/Dn8v/aMmDrCFrw3+ogj++gN6vgB8N1ElBFaZ1AWCFZDMiyw6W3jVsa6eh4sC6qgryxLUjuuyIfoZYF22j7kxVX175wT5xPrrK/q6O7b+pVhP41mfCrKmYO5ogeBL2v26h2S6UwfFCIb7mGD6etO3qHg4R97GL6b8eNQXY4mgMGMsErzghGWVz7CxJ9xb69zLk13C5PHf5JUR1S6PzctYEfYwZAqs5paOh8bl4AvccjyCOcLaz/F4HrzQx3armB+P91i/t9HSw== X-Mailgun-Sid: WyI2MTcxOCIsImRldmVsQGxpc3RzLmxpYnZpcnQub3JnIiwiNjI1ZDciXQ== X-Mailgun-Sending-Ip: 161.38.204.244 Sender: ross@golder.org From: Ross Golder To: devel@lists.libvirt.org Subject: [PATCH 1/2] remote: don't queue the stream error reply twice Date: Sun, 2 Aug 2026 20:42:05 +0700 Message-ID: <218f78fd8d61db4005bd7c4d429ead72f44acae8.1785660713.git.ross@golder.org> X-Mailer: git-send-email 2.53.0 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-MailFrom: bounce+bcaaa1.625d7-devel=lists.libvirt.org@golder.org X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; header-match-devel.lists.libvirt.org-0; emergency; member-moderation Message-ID-Hash: SAVPW4LL7LOSKI3KOLSQ7C6VXQUWBL52 X-Message-ID-Hash: SAVPW4LL7LOSKI3KOLSQ7C6VXQUWBL52 X-Mailman-Approved-At: Mon, 03 Aug 2026 08:33:00 +0000 CC: Ross Golder X-Mailman-Version: 3.3.10 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-ZohoMail-DKIM: fail (Invalid signature record: Signature is expired since 16h.) X-ZM-MESSAGEID: 1785746232195158500 Content-Type: text/plain; charset="utf-8" When a stream write fails, daemonStreamHandleWriteData() reports the error to the client via virNetServerProgramSendReplyError(), which takes ownership of 'msg' and queues it on client->tx. It then returns that function's return value, which is 0 on success. Its caller daemonStreamHandleWrite() treats 0 as "the handler did not send anything", so for VIR_NET_CONTINUE it clears the message and queues it a second time to release the client's request slot. As the message is by then the sole element of client->tx, virNetMessageQueuePush() walks to the tail - which is the message itself - and links it to itself. The resulting cycle makes virNetMessageQueueServe() hand out the same pointer twice, and virNetServerClientDispatchWrite() frees it twice: libvirtd[109078]: free(): invalid pointer systemd[1]: libvirtd.service: Main process exited, code=3Ddumped, status=3D6/ABRT The daemon then crash-loops until systemd's start limit is reached. Note that virNetMessageClear() memsets the whole message, including ->next, so the doubly-queued message looks unlinked and the condition is not detectable by inspecting msg->next alone. Give the handlers a distinct return value 2, meaning "fully processed and already queued, the caller must not touch msg again", and honour it in daemonStreamHandleWrite(). The requeue test changes from "ret > 0" to "ret =3D=3D 1" - otherwise the new value would requeue a message which is already on client->tx. daemonStreamHandleHole() had the identical defect and is fixed the same way. daemonStreamHandleFinish() and daemonStreamHandleAbort() also consume the message, but are only reached for VIR_NET_OK and VIR_NET_ERROR respectively, so the VIR_NET_CONTINUE re-send never applies to them. Closes: https://gitlab.com/libvirt/libvirt/-/issues/902 Signed-off-by: Ross Golder Reviewed-by: Daniel P. Berrang=C3=A9 --- src/remote/remote_daemon_stream.c | 51 +++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_s= tream.c index 3777c8e684..f2514f9d5f 100644 --- a/src/remote/remote_daemon_stream.c +++ b/src/remote/remote_daemon_stream.c @@ -537,8 +537,10 @@ daemonRemoveAllClientStreams(daemonClientStream *strea= m) /* * Returns: * -1 if fatal error occurred - * 0 if message was fully processed + * 0 if message was fully processed and the caller still owns 'msg' * 1 if message is still being processed + * 2 if message was fully processed and has already been queued for + * sending, so the caller must not touch 'msg' again */ static int daemonStreamHandleWriteData(virNetServerClient *client, @@ -577,11 +579,16 @@ daemonStreamHandleWriteData(virNetServerClient *clien= t, =20 virErrorRestore(&err); =20 - return virNetServerProgramSendReplyError(stream->prog, - client, - msg, - &rerr, - &msg->header); + /* SendReplyError() takes ownership of 'msg' and queues it on the + * client, so tell the caller not to send it a second time */ + if (virNetServerProgramSendReplyError(stream->prog, + client, + msg, + &rerr, + &msg->header) < 0) + return -1; + + return 2; } =20 return 0; @@ -680,6 +687,13 @@ daemonStreamHandleAbort(virNetServerClient *client, } =20 =20 +/* + * Returns: + * -1 if fatal error occurred + * 0 if message was fully processed and the caller still owns 'msg' + * 2 if message was fully processed and has already been queued for + * sending, so the caller must not touch 'msg' again + */ static int daemonStreamHandleHole(virNetServerClient *client, daemonClientStream *stream, @@ -714,11 +728,16 @@ daemonStreamHandleHole(virNetServerClient *client, virStreamEventRemoveCallback(stream->st); virStreamAbort(stream->st); =20 - return virNetServerProgramSendReplyError(stream->prog, - client, - msg, - &rerr, - &msg->header); + /* SendReplyError() takes ownership of 'msg' and queues it on the + * client, so tell the caller not to send it a second time */ + if (virNetServerProgramSendReplyError(stream->prog, + client, + msg, + &rerr, + &msg->header) < 0) + return -1; + + return 2; } =20 return 0; @@ -772,7 +791,7 @@ daemonStreamHandleWrite(virNetServerClient *client, ret =3D -1; } =20 - if (ret > 0) { + if (ret =3D=3D 1) { /* still processing data from msg, put it back into queue */ msg->next =3D stream->rx; stream->rx =3D msg; @@ -785,6 +804,14 @@ daemonStreamHandleWrite(virNetServerClient *client, return -1; } =20 + if (ret =3D=3D 2) { + /* The handler hit an error and has already queued 'msg' on the + * client as the error reply. Sending it again below would push + * a message which is still on client->tx back onto that same + * queue, linking it to itself and freeing it twice. */ + continue; + } + /* 'CONTINUE' messages don't send a reply (unless error * occurred), so to release the 'msg' object we need to * send a fake zero-length reply. Nothing actually gets --=20 2.53.0