| Message ID | 20240112135527.57212-1-f.ebner@proxmox.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ui/clipboard: avoid crash upon request when clipboard peer is not initialized | expand |
Am 12.01.24 um 14:55 schrieb Fiona Ebner: > > Then when noVNC sends a VNC_MSG_CLIENT_CUT_TEXT message (it did set > the feature correctly, so the check added by your patch passes), Sorry, forgot to adapt this part. This should read "it did set the VNC_FEATURE_CLIPBOARD_EXT feature correctly, so a check for it passes". > that > clipboard info is passed to qemu_clipboard_request() and the original > segfault still happens. >
Hi On Fri, Jan 12, 2024 at 5:57 PM Fiona Ebner <f.ebner@proxmox.com> wrote: > > With VNC, it can be that a client sends a VNC_MSG_CLIENT_CUT_TEXT > message before sending a VNC_MSG_CLIENT_SET_ENCODINGS message with > VNC_ENCODING_CLIPBOARD_EXT for configuring the clipboard extension. > > This means that qemu_clipboard_request() can be reached (via > vnc_client_cut_text_ext()) before vnc_server_cut_text_caps() was > called and had the chance to initialize the clipboard peer. In that > case, info->owner->request is NULL instead of a function and so > attempting to call it in qemu_clipboard_request() results in a > segfault. > > In particular, this can happen when using the KRDC (22.12.3) VNC > client on Wayland. > > It is not enough to check in ui/vnc.c's protocol_client_msg() if the > VNC_FEATURE_CLIPBOARD_EXT feature is enabled before handling an > extended clipboard message with vnc_client_cut_text_ext(), because of > the following scenario with two clients (say noVNC and KRDC): > > The noVNC client sets the extension VNC_FEATURE_CLIPBOARD_EXT and > initializes its cbpeer. > > The KRDC client does not, but triggers a vnc_client_cut_text() (note > it's not the _ext variant)). There, a new clipboard info with it as > the 'owner' is created and via qemu_clipboard_set_data() is called, > which in turn calls qemu_clipboard_update() with that info. > > In qemu_clipboard_update(), the notifier for the noVNC client will be > called, i.e. vnc_clipboard_notify() and also set vs->cbinfo for the > noVNC client. The 'owner' in that clipboard info is the clipboard peer > for the KRDC client, which did not initialize the 'request' function. > That sounds correct to me, it is the owner of that clipboard info. > > Then when noVNC sends a VNC_MSG_CLIENT_CUT_TEXT message (it did set > the feature correctly, so the check added by your patch passes), that > clipboard info is passed to qemu_clipboard_request() and the original > segfault still happens. > > Fixes: CVE-2023-6683 > Reported-by: Markus Frank <m.frank@proxmox.com> > Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> > Tested-by: Markus Frank <m.frank@proxmox.com> > --- > > This is just a minimal fix. Happy to add some warning/error to not > hide the issue with the missing initialization completely and/or go > for a different approach with a check somewhere in the VNC code. > > ui/clipboard.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/ui/clipboard.c b/ui/clipboard.c > index 3d14bffaf8..c13b54d2e9 100644 > --- a/ui/clipboard.c > +++ b/ui/clipboard.c > @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, > if (info->types[type].data || > info->types[type].requested || > !info->types[type].available || > - !info->owner) > + !info->owner || > + !info->owner->request) > return; While that fixes the crash, I think we should handle the situation earlier. A clipboard peer shouldn't be allowed to hold the clipboard if it doesn't have the data available or a "request" callback set. Iow, we should have an assert(info->owner->request != NULL) here instead. > info->types[type].requested = true; > -- > 2.39.2 > > >
Am 14.01.24 um 14:51 schrieb Marc-André Lureau: >> >> diff --git a/ui/clipboard.c b/ui/clipboard.c >> index 3d14bffaf8..c13b54d2e9 100644 >> --- a/ui/clipboard.c >> +++ b/ui/clipboard.c >> @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, >> if (info->types[type].data || >> info->types[type].requested || >> !info->types[type].available || >> - !info->owner) >> + !info->owner || >> + !info->owner->request) >> return; > > While that fixes the crash, I think we should handle the situation > earlier. A clipboard peer shouldn't be allowed to hold the clipboard > if it doesn't have the data available or a "request" callback set. > Where should initialization of the cbpeer happen so that we are guaranteed to do it also for clients that do not set the VNC_FEATURE_CLIPBOARD_EXT feature? Can the vnc_clipboard_request function be re-used for clients without that feature or will it be necessary to add some kind of "dummy" request callback for those clients? > Iow, we should have an assert(info->owner->request != NULL) here instead. > Your choice of course, but it would be a crash again should the issue ever re-appear. Would error message (so the issue gets noticed) + return be an option too? Best Regards, Fiona
Hi On Mon, Jan 15, 2024 at 2:45 PM Fiona Ebner <f.ebner@proxmox.com> wrote: > > Am 14.01.24 um 14:51 schrieb Marc-André Lureau: > >> > >> diff --git a/ui/clipboard.c b/ui/clipboard.c > >> index 3d14bffaf8..c13b54d2e9 100644 > >> --- a/ui/clipboard.c > >> +++ b/ui/clipboard.c > >> @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, > >> if (info->types[type].data || > >> info->types[type].requested || > >> !info->types[type].available || > >> - !info->owner) > >> + !info->owner || > >> + !info->owner->request) > >> return; > > > > While that fixes the crash, I think we should handle the situation > > earlier. A clipboard peer shouldn't be allowed to hold the clipboard > > if it doesn't have the data available or a "request" callback set. > > > > Where should initialization of the cbpeer happen so that we are > guaranteed to do it also for clients that do not set the > VNC_FEATURE_CLIPBOARD_EXT feature? Can the vnc_clipboard_request > function be re-used for clients without that feature or will it be > necessary to add some kind of "dummy" request callback for those clients? qemu_clipboard_update() shouldn't accept info as current clipboard if the owner doesn't have the data available or a "request" callback set. This should also be assert() somehow and handled earlier. In vnc_client_cut_text_ext() we could detect that situation, but with Daniel's "[PATCH] ui: reject extended clipboard message if not activated", this shouldn't happen anymore iiuc. > > > Iow, we should have an assert(info->owner->request != NULL) here instead. > > > Your choice of course, but it would be a crash again should the issue > ever re-appear. Would error message (so the issue gets noticed) + return > be an option too? > > Best Regards, > Fiona >
Am 15.01.24 um 12:15 schrieb Marc-André Lureau: > Hi > > On Mon, Jan 15, 2024 at 2:45 PM Fiona Ebner <f.ebner@proxmox.com> wrote: >> >> Am 14.01.24 um 14:51 schrieb Marc-André Lureau: >>>> >>>> diff --git a/ui/clipboard.c b/ui/clipboard.c >>>> index 3d14bffaf8..c13b54d2e9 100644 >>>> --- a/ui/clipboard.c >>>> +++ b/ui/clipboard.c >>>> @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, >>>> if (info->types[type].data || >>>> info->types[type].requested || >>>> !info->types[type].available || >>>> - !info->owner) >>>> + !info->owner || >>>> + !info->owner->request) >>>> return; >>> >>> While that fixes the crash, I think we should handle the situation >>> earlier. A clipboard peer shouldn't be allowed to hold the clipboard >>> if it doesn't have the data available or a "request" callback set. >>> >> >> Where should initialization of the cbpeer happen so that we are >> guaranteed to do it also for clients that do not set the >> VNC_FEATURE_CLIPBOARD_EXT feature? Can the vnc_clipboard_request >> function be re-used for clients without that feature or will it be >> necessary to add some kind of "dummy" request callback for those clients? > > qemu_clipboard_update() shouldn't accept info as current clipboard if > the owner doesn't have the data available or a "request" callback set. > This should also be assert() somehow and handled earlier. > The request callback is only initialized in vnc_server_cut_text_caps() when the VNC_FEATURE_CLIPBOARD_EXT is enabled. AFAIU, it's perfectly fine for clients to use the clipboard with non-extended messages and qemu_clipboard_update() should (and currently does) accept those. > In vnc_client_cut_text_ext() we could detect that situation, but with > Daniel's "[PATCH] ui: reject extended clipboard message if not > activated", this shouldn't happen anymore iiuc. > Daniel's patch doesn't change the behavior for non-extended messages. The problem can still happen with two VNC clients. This is the scenario described in the lower half of my commit message (and why Daniel mentions in his patch that it's not sufficient to fix the CVE). In short: client A does not set the VNC_FEATURE_CLIPBOARD_EXT feature and then uses a non-extended VNC_MSG_CLIENT_CUT_TEXT message. This leads to vnc_client_cut_text() being called and setting the clipboard info referencing that client. But here, no request callback is initialized, that only happens in vnc_server_cut_text_caps() when the VNC_FEATURE_CLIPBOARD_EXT is enabled. When client B does set the VNC_FEATURE_CLIPBOARD_EXT feature and does send an extended VNC_MSG_CLIENT_CUT_TEXT message, the request callback will be attempted but it isn't set. Best Regards, Fiona
Hi On Mon, Jan 15, 2024 at 3:26 PM Fiona Ebner <f.ebner@proxmox.com> wrote: > > Am 15.01.24 um 12:15 schrieb Marc-André Lureau: > > Hi > > > > On Mon, Jan 15, 2024 at 2:45 PM Fiona Ebner <f.ebner@proxmox.com> wrote: > >> > >> Am 14.01.24 um 14:51 schrieb Marc-André Lureau: > >>>> > >>>> diff --git a/ui/clipboard.c b/ui/clipboard.c > >>>> index 3d14bffaf8..c13b54d2e9 100644 > >>>> --- a/ui/clipboard.c > >>>> +++ b/ui/clipboard.c > >>>> @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, > >>>> if (info->types[type].data || > >>>> info->types[type].requested || > >>>> !info->types[type].available || > >>>> - !info->owner) > >>>> + !info->owner || > >>>> + !info->owner->request) > >>>> return; > >>> > >>> While that fixes the crash, I think we should handle the situation > >>> earlier. A clipboard peer shouldn't be allowed to hold the clipboard > >>> if it doesn't have the data available or a "request" callback set. > >>> > >> > >> Where should initialization of the cbpeer happen so that we are > >> guaranteed to do it also for clients that do not set the > >> VNC_FEATURE_CLIPBOARD_EXT feature? Can the vnc_clipboard_request > >> function be re-used for clients without that feature or will it be > >> necessary to add some kind of "dummy" request callback for those clients? > > > > qemu_clipboard_update() shouldn't accept info as current clipboard if > > the owner doesn't have the data available or a "request" callback set. > > This should also be assert() somehow and handled earlier. > > > > The request callback is only initialized in vnc_server_cut_text_caps() > when the VNC_FEATURE_CLIPBOARD_EXT is enabled. AFAIU, it's perfectly > fine for clients to use the clipboard with non-extended messages and > qemu_clipboard_update() should (and currently does) accept those. > > > In vnc_client_cut_text_ext() we could detect that situation, but with > > Daniel's "[PATCH] ui: reject extended clipboard message if not > > activated", this shouldn't happen anymore iiuc. > > > > Daniel's patch doesn't change the behavior for non-extended messages. > The problem can still happen with two VNC clients. This is the scenario > described in the lower half of my commit message (and why Daniel > mentions in his patch that it's not sufficient to fix the CVE). > > In short: client A does not set the VNC_FEATURE_CLIPBOARD_EXT feature > and then uses a non-extended VNC_MSG_CLIENT_CUT_TEXT message. This leads > to vnc_client_cut_text() being called and setting the clipboard info > referencing that client. But here, no request callback is initialized, > that only happens in vnc_server_cut_text_caps() when the > VNC_FEATURE_CLIPBOARD_EXT is enabled. > > When client B does set the VNC_FEATURE_CLIPBOARD_EXT feature and does > send an extended VNC_MSG_CLIENT_CUT_TEXT message, the request callback > will be attempted but it isn't set. > 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.
Am 15.01.24 um 12:33 schrieb Marc-André Lureau: > Hi > > On Mon, Jan 15, 2024 at 3:26 PM Fiona Ebner <f.ebner@proxmox.com> wrote: >> >> Am 15.01.24 um 12:15 schrieb Marc-André Lureau: >>> Hi >>> >>> On Mon, Jan 15, 2024 at 2:45 PM Fiona Ebner <f.ebner@proxmox.com> wrote: >>>> >>>> Am 14.01.24 um 14:51 schrieb Marc-André Lureau: >>>>>> >>>>>> diff --git a/ui/clipboard.c b/ui/clipboard.c >>>>>> index 3d14bffaf8..c13b54d2e9 100644 >>>>>> --- a/ui/clipboard.c >>>>>> +++ b/ui/clipboard.c >>>>>> @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, >>>>>> if (info->types[type].data || >>>>>> info->types[type].requested || >>>>>> !info->types[type].available || >>>>>> - !info->owner) >>>>>> + !info->owner || >>>>>> + !info->owner->request) >>>>>> return; >>>>> >>>>> While that fixes the crash, I think we should handle the situation >>>>> earlier. A clipboard peer shouldn't be allowed to hold the clipboard >>>>> if it doesn't have the data available or a "request" callback set. >>>>> >>>> >>>> Where should initialization of the cbpeer happen so that we are >>>> guaranteed to do it also for clients that do not set the >>>> VNC_FEATURE_CLIPBOARD_EXT feature? Can the vnc_clipboard_request >>>> function be re-used for clients without that feature or will it be >>>> necessary to add some kind of "dummy" request callback for those clients? >>> >>> qemu_clipboard_update() shouldn't accept info as current clipboard if >>> the owner doesn't have the data available or a "request" callback set. >>> This should also be assert() somehow and handled earlier. >>> >> >> The request callback is only initialized in vnc_server_cut_text_caps() >> when the VNC_FEATURE_CLIPBOARD_EXT is enabled. AFAIU, it's perfectly >> fine for clients to use the clipboard with non-extended messages and >> qemu_clipboard_update() should (and currently does) accept those. >> >>> In vnc_client_cut_text_ext() we could detect that situation, but with >>> Daniel's "[PATCH] ui: reject extended clipboard message if not >>> activated", this shouldn't happen anymore iiuc. >>> >> >> Daniel's patch doesn't change the behavior for non-extended messages. >> The problem can still happen with two VNC clients. This is the scenario >> described in the lower half of my commit message (and why Daniel >> mentions in his patch that it's not sufficient to fix the CVE). >> >> In short: client A does not set the VNC_FEATURE_CLIPBOARD_EXT feature >> and then uses a non-extended VNC_MSG_CLIENT_CUT_TEXT message. This leads >> to vnc_client_cut_text() being called and setting the clipboard info >> referencing that client. But here, no request callback is initialized, >> that only happens in vnc_server_cut_text_caps() when the >> VNC_FEATURE_CLIPBOARD_EXT is enabled. >> >> When client B does set the VNC_FEATURE_CLIPBOARD_EXT feature and does >> send an extended VNC_MSG_CLIENT_CUT_TEXT message, the request callback >> will be attempted but it isn't set. >> > > 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? Best Regards, Fiona
Hi On Mon, Jan 15, 2024 at 3:48 PM Fiona Ebner <f.ebner@proxmox.com> wrote: > > Am 15.01.24 um 12:33 schrieb Marc-André Lureau: > > Hi > > > > On Mon, Jan 15, 2024 at 3:26 PM Fiona Ebner <f.ebner@proxmox.com> wrote: > >> > >> Am 15.01.24 um 12:15 schrieb Marc-André Lureau: > >>> Hi > >>> > >>> On Mon, Jan 15, 2024 at 2:45 PM Fiona Ebner <f.ebner@proxmox.com> wrote: > >>>> > >>>> Am 14.01.24 um 14:51 schrieb Marc-André Lureau: > >>>>>> > >>>>>> diff --git a/ui/clipboard.c b/ui/clipboard.c > >>>>>> index 3d14bffaf8..c13b54d2e9 100644 > >>>>>> --- a/ui/clipboard.c > >>>>>> +++ b/ui/clipboard.c > >>>>>> @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, > >>>>>> if (info->types[type].data || > >>>>>> info->types[type].requested || > >>>>>> !info->types[type].available || > >>>>>> - !info->owner) > >>>>>> + !info->owner || > >>>>>> + !info->owner->request) > >>>>>> return; > >>>>> > >>>>> While that fixes the crash, I think we should handle the situation > >>>>> earlier. A clipboard peer shouldn't be allowed to hold the clipboard > >>>>> if it doesn't have the data available or a "request" callback set. > >>>>> > >>>> > >>>> Where should initialization of the cbpeer happen so that we are > >>>> guaranteed to do it also for clients that do not set the > >>>> VNC_FEATURE_CLIPBOARD_EXT feature? Can the vnc_clipboard_request > >>>> function be re-used for clients without that feature or will it be > >>>> necessary to add some kind of "dummy" request callback for those clients? > >>> > >>> qemu_clipboard_update() shouldn't accept info as current clipboard if > >>> the owner doesn't have the data available or a "request" callback set. > >>> This should also be assert() somehow and handled earlier. > >>> > >> > >> The request callback is only initialized in vnc_server_cut_text_caps() > >> when the VNC_FEATURE_CLIPBOARD_EXT is enabled. AFAIU, it's perfectly > >> fine for clients to use the clipboard with non-extended messages and > >> qemu_clipboard_update() should (and currently does) accept those. > >> > >>> In vnc_client_cut_text_ext() we could detect that situation, but with > >>> Daniel's "[PATCH] ui: reject extended clipboard message if not > >>> activated", this shouldn't happen anymore iiuc. > >>> > >> > >> Daniel's patch doesn't change the behavior for non-extended messages. > >> The problem can still happen with two VNC clients. This is the scenario > >> described in the lower half of my commit message (and why Daniel > >> mentions in his patch that it's not sufficient to fix the CVE). > >> > >> In short: client A does not set the VNC_FEATURE_CLIPBOARD_EXT feature > >> and then uses a non-extended VNC_MSG_CLIENT_CUT_TEXT message. This leads > >> to vnc_client_cut_text() being called and setting the clipboard info > >> referencing that client. But here, no request callback is initialized, > >> that only happens in vnc_server_cut_text_caps() when the > >> VNC_FEATURE_CLIPBOARD_EXT is enabled. > >> > >> When client B does set the VNC_FEATURE_CLIPBOARD_EXT feature and does > >> send an extended VNC_MSG_CLIENT_CUT_TEXT message, the request callback > >> will be attempted but it isn't set. > >> > > > > 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.
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). Best Regards, Fiona
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
diff --git a/ui/clipboard.c b/ui/clipboard.c index 3d14bffaf8..c13b54d2e9 100644 --- a/ui/clipboard.c +++ b/ui/clipboard.c @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, if (info->types[type].data || info->types[type].requested || !info->types[type].available || - !info->owner) + !info->owner || + !info->owner->request) return; info->types[type].requested = true;