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

Reply via email to