On 2/25/21 2:39 AM, Peng Liang wrote:
On 2/24/2021 9:59 PM, Michal Privoznik wrote:
On 2/24/21 12:28 PM, Peng Liang wrote:
qemuMonitorUnregister will be called in multiple threads (e.g. threads
in rpc worker pool and the vm event thread). In some cases, it isn't
protected by the monitor lock, which may lead to call g_source_unref
more than one time and a use-after-free problem eventually.
Add the missing lock in qemuProcessHandleMonitorEOF (which is the only
position missing lock of monitor I found).
Suggested-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
Signed-off-by: Peng Liang <liangpeng10@xxxxxxxxxx>
---
This patch is v2 of
https://listman.redhat.com/archives/libvir-list/2021-February/msg00945.html.
v1 -> v2:
Locking monitor in qemuProcessHandleMonitorEOF instead of using aotmic
function in qemuMonitorUnregister.
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_process.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d25c26343a7f..14e6b1fe9626 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -427,6 +427,7 @@ bool qemuMonitorWasDisposed(void);
void qemuMonitorRegister(qemuMonitorPtr mon)
ATTRIBUTE_NONNULL(1);
+/* Must be called with monitor locked. */
void qemuMonitorUnregister(qemuMonitorPtr mon)
From this it's not very clear which function the comment belongs to. We
tend to put comments into .c because that's where tags usually take you
first. So you get the memo first.
Hi Michal,
So, do I need to send another patch to document it in
src/qemu/qemu_monitor.c?
No need I just did:
https://listman.redhat.com/archives/libvir-list/2021-February/msg01202.html
I'll merge it later today.
Michal
