[PATCH v2] qemu: Add missing lock in qemuProcessHandleMonitorEOF

[Michal Privoznik]

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 at redhat.com>
>>> Signed-off-by: Peng Liang <liangpeng10 at huawei.com>
>>> ---
>>> 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