[
https://issues.apache.org/jira/browse/GUACAMOLE-428?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16236949#comment-16236949
]
Nick Couchman commented on GUACAMOLE-428:
-----------------------------------------
The behavior described in 424 data that this happens with an incorrect
password, but when I looked at the code, it looks like exactly the same
function and exactly the same line where the null deference occurs. If you
look at pull request 120 [1] you'll see that the change introduced there to fix
424 is exactly the same place as your report, here, which indicates that the
two issues are related, and that this isn't just limited to something with
concurrent connections, but that even an incorrect password can cause the null
dereference. In any case, the changes introduced in that PR will fix this
issue, as well.
-Nick
[1] https://github.com/apache/incubator-guacamole-server/pull/120
> segfault in guac_vnc_user_leave_handler caused by concurrent problem
> --------------------------------------------------------------------
>
> Key: GUACAMOLE-428
> URL: https://issues.apache.org/jira/browse/GUACAMOLE-428
> Project: Guacamole
> Issue Type: Bug
> Components: VNC
> Affects Versions: 0.9.13-incubating
> Environment: guacd with container
> Reporter: Aiden Luo
> Priority: Major
> Attachments: guacamole-server-call-flow.png
>
>
> 1. core dump
> {code:c}
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0 0x00007f932c1763dc in guac_vnc_user_leave_handler (user=0x7f9328004980)
> at user.c:116
> 116 guac_common_cursor_remove_user(vnc_client->display->cursor, user);
> (gdb) bt
> #0 0x00007f932c1763dc in guac_vnc_user_leave_handler (user=0x7f9328004980)
> at user.c:116
> #1 0x00007f93322d1f7f in guac_client_remove_user (client=0x7f92b800ad90,
> user=0x7f9328004980) at client.c:339
> #2 0x0000000000405c60 in guacd_handle_user (user=0x7f9328004980) at
> user.c:304
> #3 0x0000000000404ca2 in guacd_user_thread (data=0x7f92b801bac0) at proc.c:95
> #4 0x00007f9331922064 in start_thread (arg=0x7f92b37fe700) at
> pthread_create.c:309
> #5 0x00007f9330b0562d in clone () at
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
> (gdb) l
> 111 int guac_vnc_user_leave_handler(guac_user* user) {
> 112
> 113 guac_vnc_client* vnc_client = (guac_vnc_client*)
> user->client->data;
> 114
> 115 /* Update shared cursor state */
> 116 guac_common_cursor_remove_user(vnc_client->display->cursor, user);
> 117
> 118 /* Free settings if not owner (owner settings will be freed with
> client) */
> 119 if (!user->owner) {
> 120 guac_vnc_settings* settings = (guac_vnc_settings*) user->data;
> (gdb) p *vnc_client
> $1 = {client_thread = 140268061849344, rfb_client = 0x0,
> rfb_MallocFrameBuffer = 0x7f931f5f5030, copy_rect_used = 0,
> settings = 0x7f9328006890, display = 0x0, clipboard = 0x7f92b8035b60, audio
> = 0x0, sftp_user = 0x0, sftp_session = 0x0,
> sftp_filesystem = 0x0, clipboard_reader = 0x7f932c1782de
> <GUAC_READ_ISO8859_1>,
> clipboard_writer = 0x7f932c178414 <GUAC_WRITE_ISO8859_1>}
> (gdb) p *user
> $2 = {client = 0x7f92b800ad90, socket = 0x7f932800af30, user_id =
> 0x7f9328004c30 "@b4e03b65-8317-4049-9f73-61c9a92272f9",
> owner = 1, active = 0, __prev = 0x0, __next = 0x0, last_received_timestamp
> = 1586735641, last_frame_duration = 0,
> processing_lag = 0, info = {optimal_width = 977, optimal_height = 668,
> audio_mimetypes = 0x7f932800b000,
> video_mimetypes = 0x7f932800b020, image_mimetypes = 0x7f9328004ae0,
> optimal_resolution = 96},
> __stream_pool = 0x7f9328004a90, __output_streams = 0x7f9328005670,
> __input_streams = 0x7f9328004c60,
> __object_pool = 0x7f932800afb0, __objects = 0x7f9328006080, data =
> 0x7f9328006890,
> mouse_handler = 0x7f932c17592e <guac_vnc_user_mouse_handler>, key_handler =
> 0x7f932c1759a7 <guac_vnc_user_key_handler>,
> clipboard_handler = 0x7f932c174ea1 <guac_vnc_clipboard_handler>,
> size_handler = 0x0, file_handler = 0x0,
> pipe_handler = 0x0, ack_handler = 0x0, blob_handler = 0x0, end_handler =
> 0x0, sync_handler = 0x0, leave_handler = 0x0,
> get_handler = 0x0, put_handler = 0x0, audio_handler = 0x0}
> {code}
> 2. analysis
> When user connect to guacd, guacd will create a new process to handle the
> connection. In new process then create +static void* guacd_user_thread(void*
> data)+ thread to do real work.
> In guacd_user_thread thread, will create a +void*
> guac_vnc_client_thread(void* data)+ thread to read frame from VNC server and
> write to guac_socket. +vnc_client->clipboard_reader+,
> +vnc_client->rfb_client+ and +vnc_client->display+ will be setup in this
> thread.
> Also guacd_user_thread will create another +void*
> guacd_user_input_thread(void* data)+ thread to receive data from client and
> send to VNC sever.
> guacd_user_thread will wait guacd_user_input_thread finished, then call +void
> guac_client_remove_user(guac_client* client, guac_user* user)+ function to
> remove user which will call +int guac_vnc_user_leave_handler(guac_user*
> user)+.
> The segfault error caused by line
> +guac_common_cursor_remove_user(vnc_client->display->cursor, user)+ in
> function +guac_vnc_user_leave_handler+，because +vnc_client->display+ is NLL.
> Why this segfault will happen?
> My guess is that there exist concurrent problem. If guacd_user_input_thread
> exit before +vnc_client->display+ gets initialized in guac_vnc_client_thread,
> the +guac_common_cursor_remove_user+ function will do null pointer
> dereference.
> We should check +vnc_client->display+ if is null in function +void
> guac_client_remove_user(guac_client* client, guac_user* user)+.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
