riscv/qemu: QEMU main repository: Please see https://www.qemu.org/docs/master/devel/submitting-a-patch.html for how to submit changes to QEMU. Pull Requests are ignored. Please only use release tarballs from the QEMU website. http://www.qemu.org - qemu

QEMU main repository: Please see https://www.qemu.org/docs/master/devel/submitting-a-patch.html for how to submit changes to QEMU. Pull Requests are ignored. Please only use release tarballs from the QEMU website. http://www.qemu.org

emulation kvm qemu virtualization

History

Eric Blake ec2f4f9cae 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: qemu-stable@nongnu.org Fixes: `53047392` ("io: introduce a network socket listener API", v2.12.0) Signed-off-by: Eric Blake <eblake@redhat.com> Message-ID: <20251113011625.878876-20-eblake@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> [eblake: minor commit message wording improvements] Signed-off-by: Eric Blake <eblake@redhat.com> (cherry picked from commit `9d86181874`) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>		9 months ago
..
channel-buffer.c	io: Add support for MSG_PEEK for socket channel	4 years ago
channel-command.c	io: follow coroutine AioContext in qio_channel_yield()	3 years ago
channel-file.c	io: Stop using qemu_open_old in channel-file	2 years ago
channel-null.c	io: follow coroutine AioContext in qio_channel_yield()	3 years ago
channel-socket.c	qio: add support for SO_PEERCRED for socket channel	2 years ago
channel-tls.c	io: release active GSource in TLS channel finalizer	10 months ago
channel-util.c	io: follow coroutine AioContext in qio_channel_yield()	3 years ago
channel-watch.c	win32: avoid mixing SOCKET and file descriptor space	3 years ago
channel-websock.c	io: fix use after free in websocket handshake code	10 months ago
channel.c	io: Add flags argument to qio_channel_readv_full_all_eof	1 year ago
dns-resolver.c	build-sys: add HAVE_IPPROTO_MPTCP	5 years ago
meson.build	Revert "meson: Propagate gnutls dependency"	2 years ago
net-listener.c	qio: Protect NetListener callback with mutex	9 months ago
task.c	io: Fix Lesser GPL version number	6 years ago
trace-events	qio: Remember context of qio_net_listener_set_client_func_full	9 months ago
trace.h	trace: switch position of headers to what Meson requires	6 years ago