On 1/10/2021 12:26 AM, Mike Jumper wrote:
On Sat, Jan 9, 2021 at 8:43 PM Mike Jumper <[email protected]
<mailto:[email protected]>> wrote:
On Sat, Jan 9, 2021 at 4:50 PM Mike Jumper <[email protected]
<mailto:[email protected]>> wrote:
On Sat, Jan 9, 2021 at 6:38 AM Jason Keltz <[email protected]
<mailto:[email protected]>> wrote:
On 1/9/2021 4:40 AM, Mike Jumper wrote:
...
Hi Mike,
Thanks. My system is running CentOS 7.8 so it wasn't
affected by the move to FreeRDP 2.1.1 (which happened in
7.9 I believe). In addition, when I went back to 1.2.0,
since I had overwritten guacd with my
guacamole-server-1.3.0 install, I had to recompile
guacamole-server-1.2.0, and it worked just as before. The
problem has to be something else.
The changes to the RDP support which detect whether FreeRDP
will automatically free the relevant memory were introduced in
1.3.0. Reverting to 1.2.0 would not include the changes that
would result in a double-free if the build detects one type of
behavior but the library installed actually does another. In
the event that the library installed does not automatically
free memory (ie: if 2.0.0-rc4 is installed and then things are
upgraded to 2.1.1), this would mean that 1.2.0 would leak.
See:
https://issues.apache.org/jira/browse/GUACAMOLE-1181
<https://issues.apache.org/jira/browse/GUACAMOLE-1181>
https://issues.apache.org/jira/browse/GUACAMOLE-1241
<https://issues.apache.org/jira/browse/GUACAMOLE-1241>
Welp, testing myself on a fresh CentOS 7 machine and manually
installing the older 2.0.0-rc4 packages, I see the same
double-free behavior. I'm not sure what's happening here, as both
frees appear to be happening within FreeRDP:
guacd[14986]: INFO: Guacamole proxy daemon (guacd) version
1.3.0 started
guacd[14986]: INFO: Listening on host 127.0.0.1, port 4822
guacd[14986]: INFO: Creating new client for protocol "rdp"
guacd[14986]: INFO: Connection ID is
"$825f8c0f-2bff-4a88-87ad-a013711a7f9f"
guacd[14988]: INFO: Security mode: TLS
guacd[14988]: INFO: Resize method: display-update
guacd[14988]: INFO: Loading keymap "base"
guacd[14988]: INFO: Loading keymap "en-us-qwerty"
guacd[14988]: INFO: User
"@fc3f00fc-aa20-4ffb-a9dc-b75f3e4e6cbf" joined connection
"$825f8c0f-2bff-4a88-87ad-a013711a7f9f" (1 users now present)
guacd[14988]: INFO: Connected to RDPDR 1.13 as client 0x0005
==14988== Thread 4:
==14988== Invalid read of size 4
==14988== at 0xCB5B4CD: Stream_Free (stream.c:130)
==14988== by 0xC5E911C: channel_queue_message_free.part.0
(client.c:94) <-- Free #2
==14988== by 0xC5E9711:
freerdp_channels_process_sync.isra.2 (client.c:503)
==14988== by 0xC5EA1B4: freerdp_channels_check_fds
(client.c:573)
==14988== by 0xC5E7D9E: freerdp_check_event_handles
(freerdp.c:388)
==14988== by 0xC34548F: guac_rdp_handle_connection (rdp.c:500)
==14988== by 0xC34548F: guac_rdp_client_thread (rdp.c:747)
==14988== by 0x5A18EA4: start_thread (in
/usr/lib64/libpthread-2.17.so <http://libpthread-2.17.so>)
==14988== by 0x660C96C: clone (in /usr/lib64/libc-2.17.so
<http://libc-2.17.so>)
==14988== Address 0x1615a8f4 is 52 bytes inside a block of
size 56 free'd
==14988== at 0x4C2B06D: free (vg_replace_malloc.c:540)
==14988== by 0xC5E974A: freerdp_channels_process_sync.isra.2
(client.c:497) <-- Free #1
==14988== by 0xC5EA1B4: freerdp_channels_check_fds
(client.c:573)
==14988== by 0xC5E7D9E: freerdp_check_event_handles
(freerdp.c:388)
==14988== by 0xC34548F: guac_rdp_handle_connection (rdp.c:500)
==14988== by 0xC34548F: guac_rdp_client_thread (rdp.c:747)
==14988== by 0x5A18EA4: start_thread (in
/usr/lib64/libpthread-2.17.so <http://libpthread-2.17.so>)
==14988== by 0x660C96C: clone (in /usr/lib64/libc-2.17.so
<http://libc-2.17.so>)
The build /does/ correctly detect the free behavior of 2.0.0-rc4:
checking whether pVirtualChannelWriteEx() frees the wStream
upon completion... yes
While the valgrind results above suggest a problem with FreeRDP,
the fact that things worked fine with 1.2.0 suggest otherwise ...
yet the detected free behavior should result in the new guac-side
free code being omitted. It should be identical to the behavior of
1.2.0.
I'll retest with 1.2.0 and, assuming it does work, I'll do some
bisecting and see whether something jumps out. This looks like a
regression.
Ugh ... OK. I found the problem. Everything is correctly detected, the
code that would cause this is correctly conditional ... all that is
absolutely fine. We're just not including config.h in the relevant
file, so the macro has no effect and the free always happens.
I've opened:
https://issues.apache.org/jira/browse/GUACAMOLE-1259
<https://issues.apache.org/jira/browse/GUACAMOLE-1259>
and:
https://github.com/apache/guacamole-server/pull/325
<https://github.com/apache/guacamole-server/pull/325>
If encountering this issue, the best option is to use a newer version
of FreeRDP. If this is not possible, the next best option is to just
add the missing include (see above PR). Alternatively, you can build
with the relevant macro manually defined by running:
./configure CFLAGS="-DFREERDP_SVC_CORE_FREES_WSTREAM"
when you rebuild.
Hi Mike,
Thanks so much for checking into that for me so quickly! Very appreciated!
I did try to compile with the CFLAGS options because it was the easiest
for a quick compile, but I was puzzled because it would not include the
missing "config.h". As I suspected would happen, I got various errors
about FREERDP_SVC_CORE_FREES_WSTREAM being redefined. It's possible I
wasn't supposed to do that.
It was just as easy to add the: #include "config.h" to
src/protocols/rdp/plugins/guac-common-svc/guac-common-svc.c as per your
patch, and this did indeed work. I managed to get the new 1.3.0 in
version before the beginning of the term tomorrow.
So when I upgrade the guac server to CentOS 7.9, and have the latest
FreeRDP, I should only need to recompile guacamole-server and I
shouldn't get any errors - is that correct?
Thanks!
Jason.
