layercak3
commented
3 weeks ago I didn't see any memory leaks, but I was able to sometimes trigger 'Invalid read of size 8' in wayvnc with both an unpatched and patched neatvnc (so the issue isn't caused by this patch) when doing silly things like:
- connect with vncviewer using
-Log '*:stderr:100' -MaxCutText 8388608 - on the server:
# base64 /dev/urandom | head -c 8388608 > /tmp/data.txt # while true; do wl-copy -t 'text/plain;charset=utf-8' < /tmp/data.txt; doneCtrl+c the loop after a few seconds and close vncviewer, re-open vncviewer. Maybe even try closing without ctrl+c first.
without patch, line numbers correspond to latest neatvnc/wayvnc commit and aml release as of writing
``` Info: Client 0x19293460 (1) hung up Info: Closing client connection 0x19293460: ref 0 DEBUG: ../wayvnc/src/main.c: 1309: Client disconnected, new client count: 0 DEBUG: ../wayvnc/src/ctl-server.c: 380: Don't know how to convert sa_family 0 to string DEBUG: ../wayvnc/src/ctl-server.c: 941: Enqueueing client-disconnected event: {"id":"1","address":null,"username":null,"seat":"seat0","connection_count":0} DEBUG: ../wayvnc/src/ctl-server.c: 968: Enqueued client-disconnected event for 0 clients Info: Stopping screen capture ==1797605== Invalid read of size 8 ==1797605== at 0x1128D9: on_receive (data-control.c:69) ==1797605== by 0x4A1361E: UnknownInlinedFun (aml.c:801) ==1797605== by 0x4A1361E: aml_dispatch (aml.c:853) ==1797605== by 0x10E516: main (main.c:2000) ==1797605== Address 0x266ef7f8 is 168 bytes inside a block of size 240 free'd ==1797605== at 0x48478EF: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==1797605== by 0x4A5E20A: client_close (server.c:141) ==1797605== by 0x4A12DBE: UnknownInlinedFun (aml.c:928) ==1797605== by 0x4A12DBE: UnknownInlinedFun (aml.c:991) ==1797605== by 0x4A12DBE: aml_unref (aml.c:966) ==1797605== by 0x4A1342B: aml_stop (aml.c:731) ==1797605== by 0x4A136DA: UnknownInlinedFun (aml.c:784) ==1797605== by 0x4A136DA: aml_dispatch (aml.c:859) ==1797605== by 0x10E516: main (main.c:2000) ==1797605== Block was alloc'd at ==1797605== at 0x484BC13: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==1797605== by 0x1180B9: UnknownInlinedFun (main.c:1277) ==1797605== by 0x1180B9: on_nvnc_client_new (main.c:1354) ==1797605== by 0x4A64815: UnknownInlinedFun (server.c:452) ==1797605== by 0x4A64815: UnknownInlinedFun (server.c:1244) ==1797605== by 0x4A64815: on_client_event.lto_priv.0 (server.c:1307) ==1797605== by 0x4A5D969: UnknownInlinedFun (tcp.c:156) ==1797605== by 0x4A5D969: stream_tcp__on_event.lto_priv.0 (tcp.c:183) ==1797605== by 0x4A1361E: UnknownInlinedFun (aml.c:801) ==1797605== by 0x4A1361E: aml_dispatch (aml.c:853) ==1797605== by 0x10E516: main (main.c:2000) ==1797605== Info: New client connection from (address): 0x2651ec90 (ref 1) Info: Client 0x2651ec90 (1) hung up Info: Closing client connection 0x2651ec90: ref 0 ```
with patch
``` Info: Choosing tight encoding for client 0x27753120 DEBUG: ../wayvnc/src/main.c: 810: Client resolution changed: 1189x971, capturing output HEADLESS-1 which is headless: yes DEBUG: ../wayvnc/src/main.c: 1055: Output dimensions changed. Restarting frame capturer... DEBUG: ../wayvnc/src/main.c: 1018: screencopy_start_immediate ==2012716== Invalid read of size 8 ==2012716== at 0x1128D9: on_receive (data-control.c:69) ==2012716== by 0x4A1361E: UnknownInlinedFun (aml.c:801) ==2012716== by 0x4A1361E: aml_dispatch (aml.c:853) ==2012716== by 0x10E516: main (main.c:2000) ==2012716== Address 0x19299cf8 is 8 bytes before a block of size 32 free'd ==2012716== at 0x48478EF: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==2012716== by 0x4A7D559: FreeStmt (ast-build.c:712) ==2012716== by 0x4A7D579: FreeStmt (ast-build.c:688) ==2012716== by 0x4A7D570: FreeStmt (ast-build.c:687) ==2012716== by 0x4A7D579: FreeStmt (ast-build.c:688) ==2012716== by 0x4A7D570: FreeStmt (ast-build.c:687) ==2012716== by 0x4A7D550: FreeStmt (ast-build.c:706) ==2012716== by 0x4A7D6F8: FreeXkbFile (ast-build.c:736) ==2012716== by 0x4A8F2AA: HandleIncludeKeyTypes (types.c:252) ==2012716== by 0x4A8E487: HandleKeyTypesFile (types.c:682) ==2012716== by 0x4A8F329: HandleIncludeKeyTypes (types.c:247) ==2012716== by 0x4A8E487: HandleKeyTypesFile (types.c:682) ==2012716== Block was alloc'd at ==2012716== at 0x48447A8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==2012716== by 0x4A8225D: UnknownInlinedFun (ast-build.c:63) ==2012716== by 0x4A8225D: UnknownInlinedFun (ast-build.c:127) ==2012716== by 0x4A8225D: _xkbcommon_parse (parser.y:690) ==2012716== by 0x4A8BB8A: UnknownInlinedFun (parser.y:829) ==2012716== by 0x4A8BB8A: XkbParseString (scanner.c:216) ==2012716== by 0x4A8BDDC: XkbParseFile (scanner.c:236) ==2012716== by 0x4A85FA2: ProcessIncludeFile (include.c:316) ==2012716== by 0x4A8F2CC: HandleIncludeKeyTypes (types.c:237) ==2012716== by 0x4A8E487: HandleKeyTypesFile (types.c:682) ==2012716== by 0x4A8F329: HandleIncludeKeyTypes (types.c:247) ==2012716== by 0x4A8E487: HandleKeyTypesFile (types.c:682) ==2012716== by 0x4A8F46E: CompileKeyTypes (types.c:777) ==2012716== by 0x4A92F32: UnknownInlinedFun (keymap.c:295) ==2012716== by 0x4A92F32: compile_keymap_file (xkbcomp.c:45) ==2012716== by 0x4A93B36: text_v1_keymap_new_from_names (xkbcomp.c:96) ==2012716== Info: Client 0x27753120 (1) hung up Info: Closing client connection 0x27753120: ref 0 ```
without patch
``` DEBUG: ../wayvnc/src/main.c: 1018: screencopy_start_immediate DEBUG: ../neatvnc/src/server.c: 1988: Keyboard LED state changed: ffffffff -> 0 Info: Choosing tight encoding for client 0x1914fe90 ==1982663== Invalid read of size 8 ==1982663== at 0x4A5D3F5: nvnc_send_cut_text (server.c:956) ==1982663== by 0x1128E2: on_receive (data-control.c:69) ==1982663== by 0x4A1361E: UnknownInlinedFun (aml.c:801) ==1982663== by 0x4A1361E: aml_dispatch (aml.c:853) ==1982663== by 0x10E516: main (main.c:2000) ==1982663== Address 0x20 is not stack'd, malloc'd or (recently) free'd ==1982663== ==1982663== ==1982663== Process terminating with default action of signal 11 (SIGSEGV): dumping core ==1982663== Access not within mapped region at address 0x20 ==1982663== at 0x4A5D3F5: nvnc_send_cut_text (server.c:956) ==1982663== by 0x1128E2: on_receive (data-control.c:69) ==1982663== by 0x4A1361E: UnknownInlinedFun (aml.c:801) ==1982663== by 0x4A1361E: aml_dispatch (aml.c:853) ==1982663== by 0x10E516: main (main.c:2000) ==1982663== If you believe this happened as a result of a stack ==1982663== overflow in your program's main thread (unlikely but ==1982663== possible), you can try to increase the size of the ==1982663== main thread stack using the --main-stacksize= flag. ==1982663== The main thread stack size used in this run was 8388608. ```
That last one finally read an unmapped area crashing the process, and tells me it happened at LIST_FOREACH (client, &server->clients, link).
any1
This patch implements the extended clipboard pseudo-encoding, specifically only the text format. The appeal of this protocol is that it specifies that the text must be UTF-8 rather than Latin-1. In addition, the text is encoded by zlib which can reduce the length of the freeze that would occur when pushing a large clipboard update over a low-bandwidth link so long as the text is compressible.
The library user/wayvnc doesn't need to do anything to enable the protocol. The protocol is request-based, but I integrated it with the existing interface by simply caching the response in a buffer in nvnc_send_cut_text which can be sent when the client sends a request. Alternatively, the API could be extended in some way and wayvnc's data control code would need to adapt to it.
The protocol states that text must be transmitted in CRLF, so I convert the client's text to LF before running the nvnc_cut_text_fn. The normal ServerCutText/ClientCutText states that line endings must be LF, so it's not like preserving original line endings was possible with strict clients in the past anyway. neatvnc didn't convert server text to Latin-1 before sending it to the client, so I don't convert text to CRLF before sending to the client here.
Tested with TigerVNC's client (vncviewer). I have read and understood CONTRIBUTING.md.