[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] ui/vnc-clipboard: fix adding notifier twice

From: Nikta Lapshin
Subject: Re: [PATCH] ui/vnc-clipboard: fix adding notifier twice
Date: Sun, 21 Nov 2021 22:12:21 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0

On 11/10/21 13:38, Vladimir Sementsov-Ogievskiy wrote:
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

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 
   #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>,
             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 
   #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 
   #8  g_main_context_dispatch (context=context@entry=0x564425546bb0) at 
   #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 

I don't have any reproducer and not sure that bug may be reproduced on

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:


  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);
+    }

Perhaps QLIST_IS_INSERTED will be suitable for such checks because I couldn't find any initialize of .notify pointer so it can potentially be UB.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]