Branch: refs/heads/staging
Home: https://github.com/qemu/qemu
Commit: 04496ce926d77b6de1af2f3c4d050b8e1d411895
https://github.com/qemu/qemu/commit/04496ce926d77b6de1af2f3c4d050b8e1d411895
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M tests/qemu-iotests/tests/vvfat.out
Log Message:
-----------
iotests: Drop execute permissions on vvfat.out
Output files are not executables. Noticed while preparing another
iotest addition.
Fixes: c8f60bfb43 ("iotests: Add `vvfat` tests", v9.1.0)
Signed-off-by: Eric Blake <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
Commit: 59506e59e0f0a773e892104b945d0f15623381a7
https://github.com/qemu/qemu/commit/59506e59e0f0a773e892104b945d0f15623381a7
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M io/net-listener.c
M io/trace-events
Log Message:
-----------
qio: Add trace points to net_listener
Upcoming patches will adjust how net_listener watches for new client
connections; adding trace points now makes it easier to debug that the
changes work as intended. For example, adding
--trace='qio_net_listener*' to the qemu-storage-daemon command line
before --nbd-server will track when the server first starts listening
for clients.
Signed-off-by: Eric Blake <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
Commit: 6e03d5cdc991f5db86969fc6aeaca96234426263
https://github.com/qemu/qemu/commit/6e03d5cdc991f5db86969fc6aeaca96234426263
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M io/net-listener.c
Log Message:
-----------
qio: Unwatch before notify in QIONetListener
When changing the callback registered with QIONetListener, the code
was calling notify on the old opaque data prior to actually removing
the old GSource objects still pointing to that data. Similarly,
during finalize, it called notify before tearing down the various
GSource objects tied to the data.
In practice, a grep of the QEMU code base found that every existing
client of QIONetListener passes in a NULL notifier (the opaque data,
if non-NULL, outlives the NetListener and so does not need cleanup
when the NetListener is torn down), so this patch has no impact. And
even if a caller had passed in a reference-counted object with a
notifier of object_unref but kept its own reference on the data, then
the early notify would merely reduce a refcount from (say) 2 to 1, but
not free the object. However, it is a latent bug waiting to bite any
future caller that passes in data where the notifier actually frees
the object, because the GSource could then trigger a use-after-free if
it loses the race on a last-minute client connection resulting in the
data being passed to one final use of the async callback.
Better is to delay the notify call until after all GSource that have
been given a copy of the opaque data are torn down.
CC: [email protected]
Fixes: 530473924d "io: introduce a network socket listener API", v2.12.0
Signed-off-by: Eric Blake <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
Commit: b5676493a08b4ff80680aae7a1b1bfef8797c6e7
https://github.com/qemu/qemu/commit/b5676493a08b4ff80680aae7a1b1bfef8797c6e7
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M include/io/net-listener.h
M io/net-listener.c
M io/trace-events
Log Message:
-----------
qio: Remember context of qio_net_listener_set_client_func_full
io/net-listener.c has two modes of use: asynchronous (the user calls
qio_net_listener_set_client_func to wake up the callback via the
global GMainContext, or qio_net_listener_set_client_func_full to wake
up the callback via the caller's own alternative GMainContext), and
synchronous (the user calls qio_net_listener_wait_client which creates
its own GMainContext and waits for the first client connection before
returning, with no need for a user's callback). But commit 938c8b79
has a latent logic flaw: when qio_net_listener_wait_client finishes on
its temporary context, it reverts all of the siocs back to the global
GMainContext rather than the potentially non-NULL context they might
have been originally registered with. Similarly, if the user creates
a net-listener, adds initial addresses, registers an async callback
with a non-default context (which ties to all siocs for the initial
addresses), then adds more addresses with qio_net_listener_add, the
siocs for later addresses are blindly placed in the global context,
rather than sharing the context of the earlier ones.
In practice, I don't think this has caused issues. As pointed out by
the original commit, all async callers prior to that commit were
already okay with the NULL default context; and the typical usage
pattern is to first add ALL the addresses the listener will pay
attention to before ever setting the async callback. Likewise, if a
file uses only qio_net_listener_set_client_func instead of
qio_net_listener_set_client_func_full, then it is never using a custom
context, so later assignments of async callbacks will still be to the
same global context as earlier ones. Meanwhile, any callers that want
to do the sync operation to grab the first client are unlikely to
register an async callback; altogether bypassing the question of
whether later assignments of a GSource are being tied to a different
context over time.
I do note that chardev/char-socket.c is the only file that calls both
qio_net_listener_wait_client (sync for a single client in
tcp_chr_accept_server_sync), and qio_net_listener_set_client_func_full
(several places, all with chr->gcontext, but sometimes with a NULL
callback function during teardown). But as far as I can tell, the two
uses are mutually exclusive, based on the is_waitconnect parameter to
qmp_chardev_open_socket_server.
That said, it is more robust to remember when an async callback
function is tied to a non-default context, and have both the sync wait
and any late address additions honor that same context. That way, the
code will be robust even if a later user performs a sync wait for a
specific client in the middle of servicing a longer-lived
QIONetListener that has an async callback for all other clients.
CC: [email protected]
Fixes: 938c8b79 ("qio: store gsources for net listeners", v2.12.0)
Signed-off-by: Eric Blake <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
Commit: 9d86181874ab7b0e95ae988f6f80715943c618c6
https://github.com/qemu/qemu/commit/9d86181874ab7b0e95ae988f6f80715943c618c6
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M include/io/net-listener.h
M io/net-listener.c
Log Message:
-----------
qio: Protect NetListener callback with mutex
Without a mutex, NetListener can run into this data race between a
thread changing the async callback callback function to use when a
client connects, and the thread servicing polling of the listening
sockets:
Thread 1:
qio_net_listener_set_client_func(lstnr, f1, ...);
=> foreach sock: socket
=> object_ref(lstnr)
=> sock_src = qio_channel_socket_add_watch_source(sock, ....,
lstnr, object_unref);
Thread 2:
poll()
=> event POLLIN on socket
=> ref(GSourceCallback)
=> if (lstnr->io_func) // while lstnr->io_func is f1
...interrupt..
Thread 1:
qio_net_listener_set_client_func(lstnr, f2, ...);
=> foreach sock: socket
=> g_source_unref(sock_src)
=> foreach sock: socket
=> object_ref(lstnr)
=> sock_src = qio_channel_socket_add_watch_source(sock, ....,
lstnr, object_unref);
Thread 2:
=> call lstnr->io_func(lstnr->io_data) // now sees f2
=> return dispatch(sock)
=> unref(GSourceCallback)
=> destroy-notify
=> object_unref
Found by inspection; I did not spend the time trying to add sleeps or
execute under gdb to try and actually trigger the race in practice.
This is a SEGFAULT waiting to happen if f2 can become NULL because
thread 1 deregisters the user's callback while thread 2 is trying to
service the callback. Other messes are also theoretically possible,
such as running callback f1 with an opaque pointer that should only be
passed to f2 (if the client code were to use more than just a binary
choice between a single async function or NULL).
Mitigating factor: if the code that modifies the QIONetListener can
only be reached by the same thread that is executing the polling and
async callbacks, then we are not in a two-thread race documented above
(even though poll can see two clients trying to connect in the same
window of time, any changes made to the listener by the first async
callback will be completed before the thread moves on to the second
client). However, QEMU is complex enough that this is hard to
generically analyze. If QMP commands (like nbd-server-stop) are run
in the main loop and the listener uses the main loop, things should be
okay. But when a client uses an alternative GMainContext, or if
servicing a QMP command hands off to a coroutine to avoid blocking, I
am unable to state with certainty whether a given net listener can be
modified by a thread different from the polling thread running
callbacks.
At any rate, it is worth having the API be robust. To ensure that
modifying a NetListener can be safely done from any thread, add a
mutex that guarantees atomicity to all members of a listener object
related to callbacks. This problem has been present since
QIONetListener was introduced.
Note that this does NOT prevent the case of a second round of the
user's old async callback being invoked with the old opaque data, even
when the user has already tried to change the async callback during
the first async callback; it is only about ensuring that there is no
sharding (the eventual io_func(io_data) call that does get made will
correspond to a particular combination that the user had requested at
some point in time, and not be sharded to a combination that never
existed in practice). In other words, this patch maintains the status
quo that a user's async callback function already needs to be robust
to parallel clients landing in the same window of poll servicing, even
when only one client is desired, if that particular listener can be
amended in a thread other than the one doing the polling.
CC: [email protected]
Fixes: 53047392 ("io: introduce a network socket listener API", v2.12.0)
Signed-off-by: Eric Blake <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
[eblake: minor commit message wording improvements]
Signed-off-by: Eric Blake <[email protected]>
Commit: 36769aeedf6dbd56eda16f0c6e3032d937896fdc
https://github.com/qemu/qemu/commit/36769aeedf6dbd56eda16f0c6e3032d937896fdc
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M io/net-listener.c
Log Message:
-----------
qio: Minor optimization when callback function is unchanged
In qemu-nbd and other NBD server setups where parallel clients are
supported, it is common that the caller will re-register the same
callback function as long as it has not reached its limit on
simultaneous clients. In that case, there is no need to tear down and
reinstall GSource watches in the GMainContext.
In practice, all existing callers currently pass NULL for notify, and
no caller ever changes context across calls (for async uses, either
the caller consistently uses qio_net_listener_set_client_func_full
with the same context, or the caller consistently uses only
qio_net_listener_set_client_func which always uses the global
context); but the time spent checking these two fields in addition to
the more important func and data is still less than the savings of not
churning through extra GSource manipulations when the result will be
unchanged.
Signed-off-by: Eric Blake <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
Commit: e685dd26c7afbf2461dfb4d51a699b9a10824114
https://github.com/qemu/qemu/commit/e685dd26c7afbf2461dfb4d51a699b9a10824114
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M io/net-listener.c
Log Message:
-----------
qio: Factor out helpers qio_net_listener_[un]watch
The code had three similar repetitions of an iteration over one or all
of nsiocs to set up a GSource, and likewise for teardown. Since an
upcoming patch wants to tweak whether GSource or AioContext is used,
it's better to consolidate that into one helper function for fewer
places to edit later.
Signed-off-by: Eric Blake <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Commit: dfeadf82c2b428d1c7f59d4b573588095630d99e
https://github.com/qemu/qemu/commit/dfeadf82c2b428d1c7f59d4b573588095630d99e
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M chardev/char-socket.c
Log Message:
-----------
chardev: Reuse channel's cached local address
Directly accessing the fd member of a QIOChannelSocket is an
undesirable leaky abstraction. What's more, grabbing that fd merely
to force an eventual call to getsockname() can be wasteful, since the
channel is often able to return its cached local name.
Reported-by: Daniel P. Berrangé <[email protected]>
Signed-off-by: Eric Blake <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Commit: ec59a65a4db79297ca155738a2a7358204696f12
https://github.com/qemu/qemu/commit/ec59a65a4db79297ca155738a2a7358204696f12
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M chardev/char-socket.c
M include/io/channel-socket.h
M include/io/net-listener.h
M io/net-listener.c
M migration/socket.c
M ui/vnc.c
Log Message:
-----------
qio: Provide accessor around QIONetListener->sioc
An upcoming patch needs to pass more than just sioc as the opaque
pointer to an AioContext; but since our AioContext code in general
(and its QIO Channel wrapper code) lacks a notify callback present
with GSource, we do not have the trivial option of just g_malloc'ing a
small struct to hold all that data coupled with a notify of g_free.
Instead, the data pointer must outlive the registered handler; in
fact, having the data pointer have the same lifetime as QIONetListener
is adequate.
But the cleanest way to stick such a helper struct in QIONetListener
will be to rearrange internal struct members. And that in turn means
that all existing code that currently directly accesses
listener->nsioc and listener->sioc[] should instead go through
accessor functions, to be immune to the upcoming struct layout
changes. So this patch adds accessor methods qio_net_listener_nsioc()
and qio_net_listener_sioc(), and puts them to use.
While at it, notice that the pattern of grabbing an sioc from the
listener only to turn around can call
qio_channel_socket_get_local_address is common enough to also warrant
the helper of qio_net_listener_get_local_address, and fix a copy-paste
error in the corresponding documentation.
Signed-off-by: Eric Blake <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Commit: cc0faf8273f090a44429be0365cd476efc2d63a2
https://github.com/qemu/qemu/commit/cc0faf8273f090a44429be0365cd476efc2d63a2
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M include/io/net-listener.h
M io/net-listener.c
M io/trace-events
Log Message:
-----------
qio: Prepare NetListener to use AioContext
For ease of review, this patch adds an AioContext pointer to the
QIONetListener struct, the code to trace it, and refactors
listener->io_source to instead be an array of utility structs; but the
aio_context pointer is always NULL until the next patch adds an API to
set it. There should be no semantic change in this patch.
Signed-off-by: Eric Blake <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
Commit: de252f7993bb0a258537c7d3359bb524c6122ae3
https://github.com/qemu/qemu/commit/de252f7993bb0a258537c7d3359bb524c6122ae3
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M include/io/net-listener.h
M io/net-listener.c
Log Message:
-----------
qio: Add QIONetListener API for using AioContext
The user calling himself "John Doe" reported a deadlock when
attempting to use qemu-storage-daemon to serve both a base file over
NBD, and a qcow2 file with that NBD export as its backing file, from
the same process, even though it worked just fine when there were two
q-s-d processes. The bulk of the NBD server code properly uses
coroutines to make progress in an event-driven manner, but the code
for spawning a new coroutine at the point when listen(2) detects a new
client was hard-coded to use the global GMainContext; in other words,
the callback that triggers nbd_client_new to let the server start the
negotiation sequence with the client requires the main loop to be
making progress. However, the code for bdrv_open of a qcow2 image
with an NBD backing file uses an AIO_WAIT_WHILE nested event loop to
ensure that the entire qcow2 backing chain is either fully loaded or
rejected, without any side effects from the main loop causing unwanted
changes to the disk being loaded (in short, an AioContext represents
the set of actions that are known to be safe while handling block
layer I/O, while excluding any other pending actions in the global
main loop with potentially larger risk of unwanted side effects).
This creates a classic case of deadlock: the server can't progress to
the point of accept(2)ing the client to write to the NBD socket
because the main loop is being starved until the AIO_WAIT_WHILE
completes the bdrv_open, but the AIO_WAIT_WHILE can't progress because
it is blocked on the client coroutine stuck in a read() of the
expected magic number from the server side of the socket.
This patch adds a new API to allow clients to opt in to listening via
an AioContext rather than a GMainContext. This will allow NBD to fix
the deadlock by performing all actions during bdrv_open in the main
loop AioContext.
Technical debt warning: I would have loved to utilize a notify
function with AioContext to guarantee that we don't finalize listener
due to an object_unref if there is any callback still running (the way
GSource does), but wiring up notify functions into AioContext is a
bigger task that will be deferred to a later QEMU release. But for
solving the NBD deadlock, it is sufficient to note that the QMP
commands for enabling and disabling the NBD server are really the only
points where we want to change the listener's callback. Furthermore,
those commands are serviced in the main loop, which is the same
AioContext that is also listening for connections. Since a thread
cannot interrupt itself, we are ensured that at the point where we are
changing the watch, there are no callbacks active. This is NOT as
powerful as the GSource cross-thread safety, but sufficient for the
needs of today.
An upcoming patch will then add a unit test (kept separate to make it
easier to rearrange the series to demonstrate the deadlock without
this patch).
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3169
Signed-off-by: Eric Blake <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Commit: 89179bb4d91cc8d816e762c2c92f7c2120dda663
https://github.com/qemu/qemu/commit/89179bb4d91cc8d816e762c2c92f7c2120dda663
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M blockdev-nbd.c
Log Message:
-----------
nbd: Avoid deadlock in client connecting to same-process server
See the previous patch for a longer description of the deadlock. Now
that QIONetListener supports waiting for clients in the main loop
AioContext, NBD can use that to ensure that the server can make
progress even when a client is intentionally starving the GMainContext
from any activity not tied to an AioContext.
Note that command-line arguments and QMP commands like
nbd-server-start or nbd-server-stop that manipulate whether the NBD
server exists are serviced in the main loop; and therefore, this patch
does not fall foul of the restrictions in the previous patch about the
inherent unsafe race possible if a QIONetListener can have its async
callback modified by a different thread than the one servicing polls.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3169
Signed-off-by: Eric Blake <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
Commit: 24fd6d75b3e73105a95b93e54c0a17bae8f79c0c
https://github.com/qemu/qemu/commit/24fd6d75b3e73105a95b93e54c0a17bae8f79c0c
Author: Eric Blake <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
A tests/qemu-iotests/tests/nbd-in-qcow2-chain
A tests/qemu-iotests/tests/nbd-in-qcow2-chain.out
Log Message:
-----------
iotests: Add coverage of recent NBD qio deadlock fix
Test that all images in a qcow2 chain using an NBD backing file can be
served by the same process. Prior to the recent QIONetListener fixes,
this test would demonstrate deadlock.
The test borrows heavily from the original formula by "John Doe" in
the gitlab bug, but uses a Unix socket rather than TCP to avoid port
contention, and uses a full-blown QEMU rather than qemu-storage-daemon
since both programs were impacted.
The test starts out with the even simpler task of directly adding an
NBD client without qcow2 chain ('client'), which also provokes the
deadlock; but commenting out the 'Adding explicit NBD client' section
will still show deadlock when reaching the 'Adding wrapper image...'.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3169
Signed-off-by: Eric Blake <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Message-ID: <[email protected]>
Commit: 1c34de6d1f929af447a131ee97ab70c06f98d4e3
https://github.com/qemu/qemu/commit/1c34de6d1f929af447a131ee97ab70c06f98d4e3
Author: Thomas Huth <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M tests/qemu-iotests/207
Log Message:
-----------
tests/qemu-iotests: Fix broken grep command in iotest 207
Running "./check -ssh 207" fails for me with lots of lines like this
in the output:
+base64: invalid input
While looking closer at it, I noticed that the grep -v "\\^#" command
in this test is not working as expected - it is likely meant to filter
out the comment lines that are starting with a "#", but at least my
version of grep (GNU grep 3.11) does not work with the backslashes here.
There does not seem to be a compelling reason for these backslashes,
so let's simply drop them to fix this issue.
Signed-off-by: Thomas Huth <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Signed-off-by: Eric Blake <[email protected]>
Commit: 4c91719a6a78a1c24d8bb854f7594e767962d0d9
https://github.com/qemu/qemu/commit/4c91719a6a78a1c24d8bb854f7594e767962d0d9
Author: Alberto Garcia <[email protected]>
Date: 2025-11-13 (Thu, 13 Nov 2025)
Changed paths:
M tests/qemu-iotests/024
M tests/qemu-iotests/024.out
Log Message:
-----------
tests/qemu-iotest: fix iotest 024 with qed images
Use 'qemu-io -c map' instead of 'qemu-img map' to get an output that
works with both image types.
Cc: qemu-stable <[email protected]>
Fixes: 909852ba6b4a ("qemu-img rebase: don't exceed IO_BUF_SIZE in one
operation")
Signed-off-by: Alberto Garcia <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Eric Blake <[email protected]>
Tested-by: Thomas Huth <[email protected]>
Signed-off-by: Eric Blake <[email protected]>
Commit: 781b5470ec38d0b44991a9f4624fc5ffe91ee5b0
https://github.com/qemu/qemu/commit/781b5470ec38d0b44991a9f4624fc5ffe91ee5b0
Author: Jonah Palmer <[email protected]>
Date: 2025-11-14 (Fri, 14 Nov 2025)
Changed paths:
M net/hub.c
Log Message:
-----------
net/hub: make net_hub_port_cleanup idempotent
Makes the net_hub_port_cleanup function idempotent to avoid double
removals by guarding its QLIST_REMOVE with a flag.
When using a Xen networking device with hubport backends, e.g.:
-accel kvm,xen-version=0x40011
-netdev hubport,...
-device xen-net-device,...
the shutdown order starts with net_cleanup, which walks the list and
deletes netdevs (including hubports). Then Xen's xen_device_unrealize is
called, which eventually leads to a second net_hub_port_cleanup call,
resulting in a segfault.
Fixes: e7891c57 ("net: move backend cleanup to NIC cleanup")
Reported-by: David Woodhouse <[email protected]>
Signed-off-by: Jonah Palmer <[email protected]>
Signed-off-by: Jason Wang <[email protected]>
Commit: 6da0c9828194eb21e54fe4264cd29a1b85a29f33
https://github.com/qemu/qemu/commit/6da0c9828194eb21e54fe4264cd29a1b85a29f33
Author: Peter Maydell <[email protected]>
Date: 2025-11-14 (Fri, 14 Nov 2025)
Changed paths:
M hw/net/e1000e_core.c
Log Message:
-----------
hw/net/e1000e_core: Don't advance desc_offset for NULL buffer RX descriptors
In e1000e_write_packet_to_guest() we don't write data for RX descriptors
where the buffer address is NULL (as required by the i82574 datasheet
section 7.1.7.2). However, when we do this we still update desc_offset
by the amount of data we would have written to the RX descriptor if
it had a valid buffer pointer, resulting in our dropping that data
entirely. The data sheet is not 100% clear on the subject, but this
seems unlikely to be the correct behaviour.
Rearrange the null-descriptor logic so that we don't treat these
do-nothing descriptors as if we'd really written the data.
This both fixes a bug and also is a prerequisite to cleaning up
the size calculation logic in the next patch.
(Cc to stable largely because it will be needed for the next patch,
which fixes a more serious bug.)
Cc: [email protected]
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Akihiko Odaki <[email protected]>
Signed-off-by: Jason Wang <[email protected]>
Commit: 9d946d56a2ac8a6c2df186e20d24810255c83a3f
https://github.com/qemu/qemu/commit/9d946d56a2ac8a6c2df186e20d24810255c83a3f
Author: Peter Maydell <[email protected]>
Date: 2025-11-14 (Fri, 14 Nov 2025)
Changed paths:
M hw/net/e1000e_core.c
Log Message:
-----------
hw/net/e1000e_core: Correct rx oversize packet checks
In e1000e_write_packet_to_guest() we attempt to ensure that we don't
write more of a packet to a descriptor than will fit in the guest
configured receive buffers. However, this code does not allow for
the "packet split" feature. When packet splitting is enabled, the
first of up to 4 buffers in the descriptor is used for the packet
header only, with the payload going into buffers 2, 3 and 4. Our
length check only checks against the total sizes of all 4 buffers,
which meant that if an incoming packet was large enough to fit in (1
+ 2 + 3 + 4) but not into (2 + 3 + 4) and packet splitting was
enabled, we would run into the assertion in
e1000e_write_hdr_frag_to_rx_buffers() that we had enough buffers for
the data:
qemu-system-i386: ../../hw/net/e1000e_core.c:1418: void
e1000e_write_payload_frag_to_rx_buffers(E1000ECore *, hwaddr *, E1000EBAState
*, const char *, dma_addr_t): Assertion `bastate->cur_idx < MAX_PS_BUFFERS'
failed.
A malicious guest could provoke this assertion by configuring the
device into loopback mode, and then sending itself a suitably sized
packet into a suitably arrange rx descriptor.
The code also fails to deal with the possibility that the descriptor
buffers are sized such that the trailing checksum word does not fit
into the last descriptor which has actual data, which might also
trigger this assertion.
Rework the length handling to use two variables:
* desc_size is the total amount of data DMA'd to the guest
for the descriptor being processed in this iteration of the loop
* rx_desc_buf_size is the total amount of space left in it
As we copy data to the guest (packet header, payload, checksum),
update these two variables. (Previously we attempted to calculate
desc_size once at the top of the loop, but this is too difficult to
do correctly.) Then we can use the variables to ensure that we clamp
the amount of copied payload data to the remaining space in the
descriptor's buffers, even if we've used one of the buffers up in the
packet-split code, and we can tell whether we have enough space for
the full checksum word in this descriptor or whether we're going to
need to split that to the following descriptor.
I have included comments that hopefully help to make the loop
logic a little clearer.
Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/537
Reviewed-by: Akihiko Odaki <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
Signed-off-by: Jason Wang <[email protected]>
Commit: bab496a18358643b686f69e2b97d73fb98d37e79
https://github.com/qemu/qemu/commit/bab496a18358643b686f69e2b97d73fb98d37e79
Author: Peter Maydell <[email protected]>
Date: 2025-11-14 (Fri, 14 Nov 2025)
Changed paths:
M hw/net/e1000e_core.c
Log Message:
-----------
hw/net/e1000e_core: Adjust e1000e_write_payload_frag_to_rx_buffers() assert
An assertion in e1000e_write_payload_frag_to_rx_buffers() attempts to
guard against the calling code accidentally trying to write too much
data to a single RX descriptor, such that the E1000EBAState::cur_idx
indexes off the end of the EB1000BAState::written[] array.
Unfortunately it is overzealous: it asserts that cur_idx is in
range after it has been incremented. This will fire incorrectly
for the case where the guest configures four buffers and exactly
enough bytes are written to fill all four of them.
The only places where we use cur_idx and index in to the written[]
array are the functions e1000e_write_hdr_frag_to_rx_buffers() and
e1000e_write_payload_frag_to_rx_buffers(), so we can rewrite this to
assert before doing the array dereference, rather than asserting
after updating cur_idx.
Cc: [email protected]
Reviewed-by: Akihiko Odaki <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
Signed-off-by: Jason Wang <[email protected]>
Commit: a01344d9d78089e9e585faaeb19afccff2050abf
https://github.com/qemu/qemu/commit/a01344d9d78089e9e585faaeb19afccff2050abf
Author: Peter Maydell <[email protected]>
Date: 2025-11-14 (Fri, 14 Nov 2025)
Changed paths:
M net/net.c
Log Message:
-----------
net: pad packets to minimum length in qemu_receive_packet()
In commits like 969e50b61a28 ("net: Pad short frames to minimum size
before sending from SLiRP/TAP") we switched away from requiring
network devices to handle short frames to instead having the net core
code do the padding of short frames out to the ETH_ZLEN minimum size.
We then dropped the code for handling short frames from the network
devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
Remove the logic of padding short frames in the receive path").
This missed one route where the device's receive code can still see a
short frame: if the device is in loopback mode and it transmits a
short frame via the qemu_receive_packet() function, this will be fed
back into its own receive code without being padded.
Add the padding logic to qemu_receive_packet().
This fixes a buffer overrun which can be triggered in the
e1000_receive_iov() logic via the loopback code path.
Other devices that use qemu_receive_packet() to implement loopback
are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
and sungem.
Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
Reviewed-by: Akihiko Odaki <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
Signed-off-by: Jason Wang <[email protected]>
Commit: 389aaa0792a4a6cb3bc075c9860beeac71c792b4
https://github.com/qemu/qemu/commit/389aaa0792a4a6cb3bc075c9860beeac71c792b4
Author: Richard Henderson <[email protected]>
Date: 2025-11-14 (Fri, 14 Nov 2025)
Changed paths:
M blockdev-nbd.c
M chardev/char-socket.c
M include/io/channel-socket.h
M include/io/net-listener.h
M io/net-listener.c
M io/trace-events
M migration/socket.c
M tests/qemu-iotests/024
M tests/qemu-iotests/024.out
M tests/qemu-iotests/207
A tests/qemu-iotests/tests/nbd-in-qcow2-chain
A tests/qemu-iotests/tests/nbd-in-qcow2-chain.out
M tests/qemu-iotests/tests/vvfat.out
M ui/vnc.c
Log Message:
-----------
Merge tag 'pull-nbd-2025-11-13' of https://repo.or.cz/qemu/ericb into staging
NBD patches for 2025-11-13
- Fix NBD client deadlock when connecting to same-process server
- Several iotests improvements
# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmkWYUwACgkQp6FrSiUn
# Q2rYDgf/TQZ1UVkLhUvnH7RhF4y94tXpfVcl3/PObtis5mldZKkGlTEnFSZGJG4Y
# +ra/tdMS8ZBbTgXIAdR7tEp+n9YpWMLvYxcWcLpQQ2H3MXghtBGGjYHwkzppIvG+
# U3F8YdImbuOgR0V9NP0JWlk9DztsoRkiO3zaqLqvtwvzDXKPdjsMsGM13pHJVVru
# LdkM828Mrr8eu+DcAVFd7ZofftEgyd/E7IV1/0YCj3MaWR3BJ45gsfMUHvWwtaBP
# Mn8tQvB6yJEbAZwmepZbxrkFAJQhE916qbQyZscbnEJvDiKwK6PagQ5NAVtBaiz5
# xN3ywPOw4kghRaRLMiOsq1q/9M/p9A==
# =hhAb
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 13 Nov 2025 11:53:00 PM CET
# gpg: using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <[email protected]>" [unknown]
# gpg: aka "Eric Blake (Free Software Programmer)
<[email protected]>" [unknown]
# gpg: aka "[jpeg image of size 6874]" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2 F3AA A7A1 6B4A 2527 436A
* tag 'pull-nbd-2025-11-13' of https://repo.or.cz/qemu/ericb:
tests/qemu-iotest: fix iotest 024 with qed images
tests/qemu-iotests: Fix broken grep command in iotest 207
iotests: Add coverage of recent NBD qio deadlock fix
nbd: Avoid deadlock in client connecting to same-process server
qio: Add QIONetListener API for using AioContext
qio: Prepare NetListener to use AioContext
qio: Provide accessor around QIONetListener->sioc
chardev: Reuse channel's cached local address
qio: Factor out helpers qio_net_listener_[un]watch
qio: Minor optimization when callback function is unchanged
qio: Protect NetListener callback with mutex
qio: Remember context of qio_net_listener_set_client_func_full
qio: Unwatch before notify in QIONetListener
qio: Add trace points to net_listener
iotests: Drop execute permissions on vvfat.out
Signed-off-by: Richard Henderson <[email protected]>
Commit: 409be85c2f54223673dde89940948508189941c8
https://github.com/qemu/qemu/commit/409be85c2f54223673dde89940948508189941c8
Author: Richard Henderson <[email protected]>
Date: 2025-11-14 (Fri, 14 Nov 2025)
Changed paths:
M hw/net/e1000e_core.c
M net/hub.c
M net/net.c
Log Message:
-----------
Merge tag 'net-pull-request' of https://github.com/jasowang/qemu into staging
# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEIV1G9IJGaJ7HfzVi7wSWWzmNYhEFAmkWo9EACgkQ7wSWWzmN
# YhHargf/Uf801PmKskryVENF9sVe6u5NxJZlT3BUJVsSTGitucBIHWZ5J7MMR1lw
# If4tfMho3BX5Wrtl5GuCEzolk9pCz3wmSN6nyOU25C5tKaoJ/uR135K25D0CwVmD
# eTOyg+gKktVfogXxJ/zwZpRHMq4XXrk/C2ZP41r/CdcLyaeuDS9GIbd/q4N7f3vv
# bEsVqECzjEwWr2JBY9SD0xlIRp3nWwEvRsgRZPzBiQzfjSTlImqGLUsxIpF5V2LV
# 1BU0V/FShWyrwckBXSqCWBUh6uBUGgEl6qKnK4vH7+ed4Kd9giyp1vWAFEjHgIg+
# gZtPaT/MJQOtLyCuzfuSdUpAzz5Sfw==
# =Is8a
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 14 Nov 2025 04:36:49 AM CET
# gpg: using RSA key 215D46F48246689EC77F3562EF04965B398D6211
# gpg: Good signature from "Jason Wang (Jason Wang on RedHat)
<[email protected]>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 215D 46F4 8246 689E C77F 3562 EF04 965B 398D 6211
* tag 'net-pull-request' of https://github.com/jasowang/qemu:
net: pad packets to minimum length in qemu_receive_packet()
hw/net/e1000e_core: Adjust e1000e_write_payload_frag_to_rx_buffers() assert
hw/net/e1000e_core: Correct rx oversize packet checks
hw/net/e1000e_core: Don't advance desc_offset for NULL buffer RX descriptors
net/hub: make net_hub_port_cleanup idempotent
Signed-off-by: Richard Henderson <[email protected]>
Compare: https://github.com/qemu/qemu/compare/9febfa94b69b...409be85c2f54
To unsubscribe from these emails, change your notification settings at
https://github.com/qemu/qemu/settings/notifications
