weberr13 commented on code in PR #560: URL: https://github.com/apache/guacamole-server/pull/560#discussion_r2190819261
########## src/protocols/rdp/channels/disp.c: ########## @@ -221,32 +387,40 @@ void guac_rdp_disp_update_size(guac_rdp_disp* disp, } - else if (settings->resize_method == GUAC_RESIZE_DISPLAY_UPDATE) { - DISPLAY_CONTROL_MONITOR_LAYOUT monitors[1] = {{ - .Flags = 0x1, /* DISPLAYCONTROL_MONITOR_PRIMARY */ - .Left = 0, - .Top = 0, - .Width = width, - .Height = height, - .PhysicalWidth = 0, - .PhysicalHeight = 0, - .Orientation = 0, - .DesktopScaleFactor = 0, - .DeviceScaleFactor = 0 - }}; - - /* Send display update notification if display channel is connected */ - if (disp->disp != NULL) { - - guac_client* client = disp->client; - guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; - - pthread_mutex_lock(&(rdp_client->message_lock)); - disp->disp->SendMonitorLayout(disp->disp, 1, monitors); - pthread_mutex_unlock(&(rdp_client->message_lock)); + /* Send display update notification if display channel is connected */ + else if (settings->resize_method == GUAC_RESIZE_DISPLAY_UPDATE + && disp->disp != NULL) { + + /* Init monitors layout */ + DISPLAY_CONTROL_MONITOR_LAYOUT* monitors; + monitors = guac_mem_alloc(monitors_count * sizeof(DISPLAY_CONTROL_MONITOR_LAYOUT)); + + for (int i = 0; i < monitors_count; i++) { + /* First monitor is the primary */ + int primary_monitor = (i == 0 ? 1 : 0); + + /* Set current monitor properties */ + monitors[i].Flags = primary_monitor; + monitors[i].Left = disp->monitors[i].left_offset; + monitors[i].Top = disp->monitors[i].top_offset; + monitors[i].Width = disp->monitors[i].requested_width; + monitors[i].Height = disp->monitors[i].requested_height; + monitors[i].Orientation = 0; + monitors[i].PhysicalWidth = 0; + monitors[i].PhysicalHeight = 0; } + /* Send display update notification */ + guac_client* client = disp->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + + pthread_mutex_lock(&(rdp_client->message_lock)); + disp->disp->SendMonitorLayout(disp->disp, monitors_count, monitors); Review Comment: I may not understand the API, but naively I feel like when a pointer (monitors) is sent via a signal that memory must either be given to the receiver (eg it is now responsible for freeing monitors) or it would have to signal it is "done" with the memory somehow (which I cannot see happening). I believe from line 422 that there is a possible segmentation violation with the receiver of SendMonitorLayout reading free'ed memory. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org