From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: qemu-devel@nongnu.org
Cc: kraxel@redhat.com, vsementsov@virtuozzo.com, den@openvz.org
Subject: [PATCH] ui/vnc-clipboard: fix adding notifier twice
Date: Wed, 10 Nov 2021 11:38:00 +0100 [thread overview]
Message-ID: <20211110103800.2266729-1-vsementsov@virtuozzo.com> (raw)
vnc_server_cut_text_caps() is not guaranteed to be called only once.
If it called twice, we finally call notifier_list_add() twice with same
element. Which leads to loopback QLIST. So, on next
notifier_list_notify() we'll loop forever and QEMU stuck.
So, let's only register new notifier if it's not yet registered.
Note, that similar check is used in vdagent_chr_recv_caps() (before
call qemu_clipboard_peer_register()), and also before
qemu_clipboard_peer_unregister() call in vdagent_disconnect() and in
vnc_disconnect_finish().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
Hi all!
After backporting clipboard patches to our Rhel7-based downstream, we
faced Qemu stuck in notifier_list_notify():
(gdb) bt
#0 vnc_clipboard_notify (notifier=0x564427f283f8, data=0x564426c59a70) at ui/vnc-clipboard.c:193
#1 0x0000564423455887 in notifier_list_notify (list=list@entry=0x564423d2b258 <clipboard_notifiers>, data=data@entry=0x564426c59a70) at util/notify.c:40
#2 0x00005644233273bf in qemu_clipboard_update (info=info@entry=0x564426c59a70) at ui/clipboard.c:19
#3 0x000056442334efd2 in vnc_client_cut_text_ext (vs=vs@entry=0x564427f18000, len=len@entry=4, flags=<optimized out>,
data=data@entry=0x5644263cc00c "\002\f\001\251\020\377\377\377!\377\377\377\314\376\377\377\315\376\377\377 \377\377\377\316\345\241\300\307\376\377\377\310\376\377\377\376\376\377\377\a")
at ui/vnc-clipboard.c:256
#4 0x000056442333b172 in protocol_client_msg (vs=0x564427f18000, data=0x5644263cc000 "\006", len=<optimized out>) at ui/vnc.c:2396
#5 0x0000564423338af6 in vnc_client_read (vs=0x564427f18000) at ui/vnc.c:1537
#6 vnc_client_io (ioc=<optimized out>, condition=G_IO_IN, opaque=0x564427f18000) at ui/vnc.c:1559
#7 0x00007f07b02cf147 in g_main_dispatch (context=0x564425546bb0) at gmain.c:3192
#8 g_main_context_dispatch (context=context@entry=0x564425546bb0) at gmain.c:3845
#9 0x00005644234468f7 in glib_pollfds_poll () at util/main-loop.c:215
#10 os_host_main_loop_wait (timeout=<optimized out>) at util/main-loop.c:238
#11 main_loop_wait (nonblocking=nonblocking@entry=0) at util/main-loop.c:497
investigations shows, that notifier list has only one element which points to itself as next. So, we are in the endless loop.
Seems that it's possible, if vnc_server_cut_text_caps() called twice. Then it registers notifier twice and it added to QLIST twice, which leads to the situation.
I don't have any reproducer and not sure that bug may be reproduced on
master.
I'm not familiar with ui code - may be vnc_server_cut_text_caps() should
never be called twice? Or notifier should be removed somehow before the
second call? Maybe this patch just shadows another bug?
But what I do know, is that we should not put same element into QLIST
twice. And if the check I propose is not needed we should add an
assertion instead:
assert(!vs->cbpeer.update.notify);
ui/vnc-clipboard.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
index 9f077965d0..67284b556c 100644
--- a/ui/vnc-clipboard.c
+++ b/ui/vnc-clipboard.c
@@ -316,8 +316,10 @@ void vnc_server_cut_text_caps(VncState *vs)
caps[1] = 0;
vnc_clipboard_send(vs, 2, caps);
- vs->cbpeer.name = "vnc";
- vs->cbpeer.update.notify = vnc_clipboard_notify;
- vs->cbpeer.request = vnc_clipboard_request;
- qemu_clipboard_peer_register(&vs->cbpeer);
+ if (!vs->cbpeer.update.notify) {
+ vs->cbpeer.name = "vnc";
+ vs->cbpeer.update.notify = vnc_clipboard_notify;
+ vs->cbpeer.request = vnc_clipboard_request;
+ qemu_clipboard_peer_register(&vs->cbpeer);
+ }
}
--
2.31.1
next reply other threads:[~2021-11-10 10:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-10 10:38 Vladimir Sementsov-Ogievskiy [this message]
2021-11-21 19:12 ` [PATCH] ui/vnc-clipboard: fix adding notifier twice Nikta Lapshin
2021-11-22 10:03 ` Vladimir Sementsov-Ogievskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211110103800.2266729-1-vsementsov@virtuozzo.com \
--to=vsementsov@virtuozzo.com \
--cc=den@openvz.org \
--cc=kraxel@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).