Uploaded image for project: 'Guacamole'
  1. Guacamole
  2. GUACAMOLE-428

segfault in guac_vnc_user_leave_handler caused by concurrent problem

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Duplicate
    • Affects Version/s: 0.9.13-incubating
    • Fix Version/s: None
    • Component/s: VNC
    • Labels:
      None
    • Environment:
      guacd with container

      Description

      1. core dump

      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}
      

      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).

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                aiden0z Aiden Luo
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: