xserver: Branch 'master' - 2 commits

[GitLab Mirror]

 hw/xwayland/xwayland-drm-lease.c |   46 +++++++++++++++++++++++++++++++--------
 hw/xwayland/xwayland-output.h    |    1 
 2 files changed, 38 insertions(+), 9 deletions(-)

New commits:
commit 4053782443f99761cf26e1acc31a9712253f4329
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Tue Apr 16 14:09:08 2024 +0200

    xwayland: Do not remove output on withdraw if leased
    
    On DRM lease connector withdrawn event, Xwayland would free the
    corresponding xwl_output offered for lease.
    
    However, the pointer is still referenced from the rrLease->outputs[],
    meaning that trying to clean up the RANDR DRM leases as done with commit
    ef181265 (xwayland: Clean up drm lease when terminating) would cause a
    use after free and random crashes.
    
    To avoid that issue, on the connector withdraw event, set the connector
    withdrawn flag but do not to remove (i.e. free) the xwayland output if
    its is offered for lease.
    
    Then, once the lease is terminated, check for the xwl_outputs with a
    withdrawn connector and remove them (once we have no use for them
    anymore.
    
    Note that we cannot do that cleanup from xwl_randr_terminate_lease() as
    removing the xwl_output will free the RRcrtc resources, which checks for
    leases in XRANDR, and calls RRTerminateLease(), which chains back to
    xwl_randr_terminate_lease().
    
    v2: Use a "withdrawn_connector" flag to mark outputs to remove (Xaver)
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Xaver Hugl <xaver.hugl at kde.org>
    fixes: ef181265 - xwayland: Clean up drm lease when terminating
    
    See-also: https://gitlab.freedesktop.org/xorg/xserver/-/issues/946
    See-also: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1130
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1482>

diff --git a/hw/xwayland/xwayland-drm-lease.c b/hw/xwayland/xwayland-drm-lease.c
index 9d3b90f56..224ad4c73 100644
--- a/hw/xwayland/xwayland-drm-lease.c
+++ b/hw/xwayland/xwayland-drm-lease.c
@@ -44,7 +44,24 @@ xwl_randr_lease_cleanup_outputs(RRLeasePtr rrLease)
 
     for (i = 0; i < rrLease->numOutputs; ++i) {
         output = rrLease->outputs[i]->devPrivate;
-        output->lease = NULL;
+        if (output) {
+            output->lease = NULL;
+        }
+    }
+}
+
+static void
+xwl_randr_lease_free_outputs(RRLeasePtr rrLease)
+{
+    struct xwl_output *xwl_output;
+    int i;
+
+    for (i = 0; i < rrLease->numOutputs; ++i) {
+        xwl_output = rrLease->outputs[i]->devPrivate;
+        if (xwl_output && xwl_output->withdrawn_connector) {
+            rrLease->outputs[i]->devPrivate = NULL;
+            xwl_output_remove(xwl_output);
+        }
     }
 }
 
@@ -71,6 +88,9 @@ drm_lease_handle_finished(void *data,
         AttendClient(lease->client);
         xwl_randr_lease_cleanup_outputs(lease->rrLease);
     }
+
+    /* Free the xwl_outputs that have been withdrawn while lease-able */
+    xwl_randr_lease_free_outputs(lease->rrLease);
 }
 
 static struct wp_drm_lease_v1_listener drm_lease_listener = {
@@ -324,7 +344,15 @@ static void
 lease_connector_handle_withdrawn(void *data,
                                  struct wp_drm_lease_connector_v1 *wp_drm_lease_connector_v1)
 {
-    xwl_output_remove(data);
+    struct xwl_output *xwl_output = data;
+
+    xwl_output->withdrawn_connector = TRUE;
+
+    /* Not removing the xwl_output if currently leased with Wayland */
+    if (xwl_output->lease)
+        return;
+
+    xwl_output_remove(xwl_output);
 }
 
 static const struct wp_drm_lease_connector_v1_listener lease_connector_listener = {
diff --git a/hw/xwayland/xwayland-output.h b/hw/xwayland/xwayland-output.h
index 5e070f90d..0d36f459a 100644
--- a/hw/xwayland/xwayland-output.h
+++ b/hw/xwayland/xwayland-output.h
@@ -67,6 +67,7 @@ struct xwl_output {
     struct wp_drm_lease_connector_v1 *lease_connector;
     struct xwl_drm_lease *lease;
     struct xwl_drm_lease_device *lease_device;
+    Bool withdrawn_connector;
 };
 
 /* Per client per output emulated randr/vidmode resolution info. */
commit 21916ae148393ff09889cc486d8b8b72b0988958
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Thu Apr 25 16:43:16 2024 +0200

    xwayland: Check for outputs before lease devices
    
    In xwl_randr_request_lease(), the code checks first for leased device,
    and then checks for existing output for lease.
    
    The former assumes there are outputs for lease whereas the latter checks
    for the output, connector and lease.
    
    So if there is any existing rrLease->outputs[]->devPrivate unset, the
    code would crash on a NULL pointer dereference on the first sanity check
    before having a chance to reach the second check that would have caught
    the problem.
    
    Invert the sanity checks so that we would catch this first and return a
    BadValue instead of possibly segfaulting.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Xaver Hugl <xaver.hugl at kde.org>
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1482>

diff --git a/hw/xwayland/xwayland-drm-lease.c b/hw/xwayland/xwayland-drm-lease.c
index 4f9004d8c..9d3b90f56 100644
--- a/hw/xwayland/xwayland-drm-lease.c
+++ b/hw/xwayland/xwayland-drm-lease.c
@@ -116,6 +116,13 @@ xwl_randr_request_lease(ClientPtr client, ScreenPtr screen, RRLeasePtr rrLease)
         return BadMatch;
     }
 
+    for (i = 0; i < rrLease->numOutputs; ++i) {
+        output = rrLease->outputs[i]->devPrivate;
+        if (!output || !output->lease_connector || output->lease) {
+            return BadValue;
+        }
+    }
+
     xorg_list_for_each_entry(device_data, &xwl_screen->drm_lease_devices, link) {
         Bool connectors_of_device = FALSE;
         for (i = 0; i < rrLease->numOutputs; ++i) {
@@ -134,13 +141,6 @@ xwl_randr_request_lease(ClientPtr client, ScreenPtr screen, RRLeasePtr rrLease)
         }
     }
 
-    for (i = 0; i < rrLease->numOutputs; ++i) {
-        output = rrLease->outputs[i]->devPrivate;
-        if (!output || !output->lease_connector || output->lease) {
-            return BadValue;
-        }
-    }
-
     req = wp_drm_lease_device_v1_create_lease_request(
             lease_device->drm_lease_device);
     lease_private = calloc(1, sizeof(struct xwl_drm_lease));