libvirtd aborts with "free(): invalid pointer" whenever a stream write
fails, and then crash-loops until systemd's start limit is reached,
leaving the host with no management plane. Running guests are unaffected
but become unmanageable.

The cause is an ambiguous return contract. daemonStreamHandleWriteData()
reports a failed stream write through virNetServerProgramSendReplyError(),
which takes ownership of the message and queues it on client->tx, and then
returns that function's result - 0 on success. Its caller reads 0 as "no
reply was sent" and, for VIR_NET_CONTINUE, clears the message and queues
it a second time. Since the message is by then the sole element of
client->tx, virNetMessageQueuePush() walks to the tail - itself - and
links it to itself. virNetMessageQueueServe() then hands out the same
pointer twice and virNetServerClientDispatchWrite() frees it twice.

Patch 1 is the fix: give the handlers a distinct return value meaning
"already queued, do not touch msg again". daemonStreamHandleHole() had the
same defect and is fixed alongside. Note the requeue test has to change
from "ret > 0" to "ret == 1", otherwise the new value would requeue a
message which is already on client->tx.

Patch 2 is independent hardening: make virNetMessageQueuePush() refuse a
push that would corrupt the list, so a caller bug of this shape surfaces
as a log message rather than as heap corruption. It includes a regression
test that reproduces the self-cycle deterministically.

Note that virNetMessageClear() memsets the whole message including ->next,
so a queued message can appear unlinked. That is why patch 2 walks the
queue rather than testing msg->next, and why this class of bug is easy to
miss by inspection.

There is no ABI change; src/libvirt_remote.syms is untouched.

Evidence
--------
- valgrind memcheck: "Invalid free()" with the previous free at the *same*
  call site, plus 13 invalid reads and 6 invalid writes as the
  "while (client->tx)" loop re-reads the freed block. Exactly one invalid
  free per occurrence, as a self-cycle predicts.
- A production core dump whose crash IP is the return address of the same
  call to virNetMessageFree(), resolving to virnetserverclient.c:1374.
- Instrumented builds logging both push sites:
  virNetServerProgramSendError:168 followed by daemonStreamHandleWrite:797.

Two cautions for anyone reproducing this:

Under valgrind the daemon does *not* abort - memcheck replaces the
allocator, so glibc's malloc_printerr never runs. It logs and continues,
and the fault is easy to mistake for "works fine".

Freed virNetMessage blocks are promptly reused by malloc, so the same
address legitimately reappears as a new message moments after being freed.
In RPC debug logs that closely resembles a use-after-free and is not one.

Testing
-------
Reproduced and fixed on three production hypervisors running 12.5.0 with
these patches backported. One host had logged 5197 aborts beforehand;
across all three there have been zero aborts and zero hardening warnings
since. The hardening warning firing before the patch 1 fix and never
firing after it is the direct evidence that the root cause, and not just
the symptom, is addressed.

Full test suite passes on master (306 ok, 1 expected fail, 0 failures).
CI on a personal fork is green: 22 jobs passed, 0 failed, covering
Fedora 43/44/rawhide, CentOS Stream 9/10, Ubuntu 24.04/26.04 (including
clang), openSUSE Tumbleweed/Leap 16, Debian 13, armv7l and mingw32/64,
plus check-dco and codestyle.

Reported as https://gitlab.com/libvirt/libvirt/-/issues/902

Ross Golder (2):
  remote: don't queue the stream error reply twice
  rpc: refuse to queue a message that is already queued

 src/remote/remote_daemon_stream.c | 51 +++++++++++++-----
 src/rpc/virnetmessage.c           | 43 +++++++++++++++
 tests/virnetmessagetest.c         | 87 +++++++++++++++++++++++++++++++
 3 files changed, 169 insertions(+), 12 deletions(-)

-- 
2.53.0