Am 16.01.24 um 13:11 schrieb Fiona Ebner: > Am 15.01.24 um 13:00 schrieb Marc-André Lureau: >>>>> >>>> >>>> The trouble is when qemu_clipboard_update() is called without data & >>>> without a request callback set. We shouldn't allow that as we have no >>>> means to get the clipboard data then. >>>> >>> >>> In the above scenario, I'm pretty sure there is data when >>> qemu_clipboard_update() is called. Just no request callback. If we'd >>> reject this, won't that break clients that do not set the >>> VNC_FEATURE_CLIPBOARD_EXT feature and only use non-extended >>> VNC_MSG_CLIENT_CUT_TEXT messages? >> >> If "data" is already set, then qemu_clipboard_request() returns. >> >> Inverting the condition I suggested above: it's allowed to be the >> clipboard owner if either "data" is set, or a request callback is set. >> > > Oh, sorry. Yes, it seems the problematic case is where data is not set. > But isn't that legitimate when clearing the clipboard? Or is a > VNC_MSG_CLIENT_CUT_TEXT message not valid when len is 0 and should be > rejected? In my testing KRDC does send such a message when the clipboard > is cleared: > >> #1 0x0000558f1e6a0dac in vnc_client_cut_text (vs=0x558f207754d0, len=0, >> text=0x558f2046e008 "\003 \002\377\005") at ../ui/vnc-clipboard.c:313 >> #2 0x0000558f1e68e067 in protocol_client_msg (vs=0x558f207754d0, >> data=0x558f2046e000 "\006", len=8) at ../ui/vnc.c:2454 > > Your suggestion would disallow this for clients that do not set the > VNC_FEATURE_CLIPBOARD_EXT feature (and only use non-extended > VNC_MSG_CLIENT_CUT_TEXT messages). >
I noticed that there is yet another code path leading to qemu_clipboard_request() and dereferencing the NULL owner->request even if only non-extended VNC_MSG_CLIENT_CUT_TEXT messages are used: > Thread 1 "qemu-system-x86" hit Breakpoint 1, vnc_client_cut_text ( > vs=0x5555593e6c00, len=0, > text=0x5555575ea608 > "404078,bus=ahci0.0,drive=drive-sata0,id=sata0,bootindex=100\377\001") at > ../ui/vnc-clipboard.c:310 > (gdb) c > Continuing. And later: > qemu_clipboard_request (info=0x555557e576e0, type=QEMU_CLIPBOARD_TYPE_TEXT) > at ../ui/clipboard.c:129 > 129 if (info->types[type].data || > (gdb) p *info->owner > $65 = {name = 0x0, notifier = {notify = 0x0, node = {le_next = 0x0, le_prev = > 0x0}}, request = 0x0} > (gdb) bt > #0 qemu_clipboard_request (info=0x555557e576e0, > type=QEMU_CLIPBOARD_TYPE_TEXT) at ../ui/clipboard.c:129 > #1 0x00005555558952ce in vdagent_chr_recv_chunk (vd=0x555556f67800) at > ../ui/vdagent.c:769 > #2 vdagent_chr_write (chr=<optimized out>, buf=0x7fff4ab263e4 "", len=0) at > ../ui/vdagent.c:840 > #3 0x0000555555d98830 in qemu_chr_write_buffer (s=s@entry=0x555556f67800, > buf=buf@entry=0x7fff4ab263c0 "\001", len=36, > offset=offset@entry=0x7fffffffd030, write_all=false) at > ../chardev/char.c:122 > #4 0x0000555555d98c3c in qemu_chr_write (s=0x555556f67800, > buf=buf@entry=0x7fff4ab263c0 "\001", len=len@entry=36, > write_all=<optimized out>, write_all@entry=false) at ../chardev/char.c:186 > #5 0x0000555555d9109f in qemu_chr_fe_write (be=be@entry=0x555556e59040, > buf=buf@entry=0x7fff4ab263c0 "\001", len=len@entry=36) > at ../chardev/char-fe.c:42 > #6 0x0000555555900045 in flush_buf (port=0x555556e58f40, buf=0x7fff4ab263c0 > "\001", len=36) at ../hw/char/virtio-console.c:63 > #7 0x0000555555bea4f3 in do_flush_queued_data (port=0x555556e58f40, > vq=0x555559272558, vdev=0x55555926a4d0) > at ../hw/char/virtio-serial-bus.c:188 > #8 0x0000555555c201ff in virtio_queue_notify_vq (vq=0x555559272558) at > ../hw/virtio/virtio.c:2268 > #9 0x0000555555e36dd5 in aio_dispatch_handler (ctx=ctx@entry=0x555556e51b10, > node=0x7ffed812b9f0) at ../util/aio-posix.c:372 > #10 0x0000555555e37662 in aio_dispatch_handlers (ctx=0x555556e51b10) at > ../util/aio-posix.c:414 > #11 aio_dispatch (ctx=0x555556e51b10) at ../util/aio-posix.c:424 > #12 0x0000555555e4d44e in aio_ctx_dispatch (source=<optimized out>, > callback=<optimized out>, user_data=<optimized out>) > at ../util/async.c:358 > #13 0x00007ffff753e7a9 in g_main_context_dispatch () from > /lib/x86_64-linux-gnu/libglib-2.0.so.0 > #14 0x0000555555e4ecb8 in glib_pollfds_poll () at ../util/main-loop.c:287 > #15 os_host_main_loop_wait (timeout=58675786) at ../util/main-loop.c:310 > #16 main_loop_wait (nonblocking=nonblocking@entry=0) at > ../util/main-loop.c:589 > #17 0x0000555555aa5f63 in qemu_main_loop () at ../system/runstate.c:791 > #18 0x0000555555cadc16 in qemu_default_main () at ../system/main.c:37 > #19 0x00007ffff60801ca in __libc_start_call_main > (main=main@entry=0x5555558819d0 <main>, argc=argc@entry=102, > argv=argv@entry=0x7fffffffd458) at > ../sysdeps/nptl/libc_start_call_main.h:58 > #20 0x00007ffff6080285 in __libc_start_main_impl (main=0x5555558819d0 <main>, > argc=102, argv=0x7fffffffd458, init=<optimized out>, > fini=<optimized out>, rtld_fini=<optimized out>, > stack_end=0x7fffffffd448) at ../csu/libc-start.c:360 > #21 0x0000555555883581 in _start () So such non-extended VNC_MSG_CLIENT_CUT_TEXT messages with len=0 are already problematic. Is clearing the clipboard supposed to be done differently? Your suggestion would prevent this scenario too. I'll send a patch with that shortly. Best Regards, Fiona