[mutter] wayland/xdg-shell: Warn when invalid geometry is set

[Marge Bot <marge-bot src gnome org>]

commit d2c798838e6710bf296f7bea78bd60efc7a24b5e
Author: Jonas Ådahl <jadahl gmail com>
Date:   Wed Nov 11 10:49:25 2020 +0100

    wayland/xdg-shell: Warn when invalid geometry is set
    
    A client is not allowed to send an empty window geometry, and it is
    specified that if it does so an error should be raised. Respect this
    rule, ignore bogus geometries sent by clients with a warning.
    
    Also add a soft assert that we don't try to "resend" a configuration
    that was never sent, as doing so would result in SIGFPE as the geometry
    scale is 0.
    
    This fixes a SIGFPE crash occurring when a client did this.
    
    Related: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/2808
    Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/1527
    Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1557>

 .../invalid-xdg-shell-actions.c                    | 280 +++++++++++++++++++++
 src/tests/wayland-test-clients/meson.build         |   1 +
 src/tests/wayland-unit-tests.c                     |  14 ++
 src/wayland/meta-wayland-legacy-xdg-shell.c        |   9 +
 src/wayland/meta-wayland-xdg-shell.c               |   9 +
 src/wayland/meta-window-wayland.c                  |   4 +
 6 files changed, 317 insertions(+)
---
diff --git a/src/tests/wayland-test-clients/invalid-xdg-shell-actions.c 
b/src/tests/wayland-test-clients/invalid-xdg-shell-actions.c
new file mode 100644
index 0000000000..67ff3671e1
--- /dev/null
+++ b/src/tests/wayland-test-clients/invalid-xdg-shell-actions.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "config.h"
+
+#include <glib.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <wayland-client.h>
+
+#include "wayland-test-client-utils.h"
+
+#include "test-driver-client-protocol.h"
+#include "xdg-shell-client-protocol.h"
+
+static struct wl_display *display;
+static struct wl_registry *registry;
+static struct wl_compositor *compositor;
+static struct xdg_wm_base *xdg_wm_base;
+static struct wl_shm *shm;
+
+static struct wl_surface *surface;
+static struct xdg_surface *xdg_surface;
+static struct xdg_toplevel *xdg_toplevel;
+
+static gboolean running;
+
+static void
+init_surface (void)
+{
+  xdg_toplevel_set_title (xdg_toplevel, "bogus window geometry");
+  wl_surface_commit (surface);
+}
+
+static void
+handle_buffer_release (void             *data,
+                       struct wl_buffer *buffer)
+{
+  wl_buffer_destroy (buffer);
+}
+
+static const struct wl_buffer_listener buffer_listener = {
+  handle_buffer_release
+};
+
+static gboolean
+create_shm_buffer (int                width,
+                   int                height,
+                   struct wl_buffer **out_buffer,
+                   void             **out_data,
+                   int               *out_size)
+{
+  struct wl_shm_pool *pool;
+  static struct wl_buffer *buffer;
+  int fd, size, stride;
+  int bytes_per_pixel;
+  void *data;
+
+  bytes_per_pixel = 4;
+  stride = width * bytes_per_pixel;
+  size = stride * height;
+
+  fd = create_anonymous_file (size);
+  if (fd < 0)
+    {
+      fprintf (stderr, "Creating a buffer file for %d B failed: %m\n",
+               size);
+      return FALSE;
+    }
+
+  data = mmap (NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+  if (data == MAP_FAILED)
+    {
+      fprintf (stderr, "mmap failed: %m\n");
+      close (fd);
+      return FALSE;
+    }
+
+  pool = wl_shm_create_pool (shm, fd, size);
+  buffer = wl_shm_pool_create_buffer (pool, 0,
+                                      width, height,
+                                      stride,
+                                      WL_SHM_FORMAT_ARGB8888);
+  wl_buffer_add_listener (buffer, &buffer_listener, buffer);
+  wl_shm_pool_destroy (pool);
+  close (fd);
+
+  *out_buffer = buffer;
+  *out_data = data;
+  *out_size = size;
+
+  return TRUE;
+}
+
+static void
+fill (void    *buffer_data,
+      int      width,
+      int      height,
+      uint32_t color)
+{
+  uint32_t *pixels = buffer_data;
+  int x, y;
+
+  for (y = 0; y < height; y++)
+    {
+      for (x = 0; x < width; x++)
+        pixels[y * width + x] = color;
+    }
+}
+
+static void
+draw (struct wl_surface *surface,
+      int                width,
+      int                height,
+      uint32_t           color)
+{
+  struct wl_buffer *buffer;
+  void *buffer_data;
+  int size;
+
+  if (!create_shm_buffer (width, height,
+                          &buffer, &buffer_data, &size))
+    g_error ("Failed to create shm buffer");
+
+  fill (buffer_data, width, height, color);
+
+  wl_surface_attach (surface, buffer, 0, 0);
+}
+
+static void
+draw_main (void)
+{
+  draw (surface, 700, 500, 0xff00ff00);
+}
+
+static void
+handle_xdg_toplevel_configure (void                *data,
+                               struct xdg_toplevel *xdg_toplevel,
+                               int32_t              width,
+                               int32_t              height,
+                               struct wl_array     *state)
+{
+}
+
+static void
+handle_xdg_toplevel_close (void                *data,
+                           struct xdg_toplevel *xdg_toplevel)
+{
+  g_assert_not_reached ();
+}
+
+static const struct xdg_toplevel_listener xdg_toplevel_listener = {
+  handle_xdg_toplevel_configure,
+  handle_xdg_toplevel_close,
+};
+
+static void
+handle_xdg_surface_configure (void               *data,
+                              struct xdg_surface *xdg_surface,
+                              uint32_t            serial)
+{
+  xdg_surface_set_window_geometry (xdg_surface, 0, 0, 0, 0);
+  draw_main ();
+  wl_surface_commit (surface);
+
+  g_assert_cmpint (wl_display_roundtrip (display), !=, -1);
+  running = FALSE;
+}
+
+static const struct xdg_surface_listener xdg_surface_listener = {
+  handle_xdg_surface_configure,
+};
+
+static void
+handle_xdg_wm_base_ping (void               *data,
+                         struct xdg_wm_base *xdg_wm_base,
+                         uint32_t            serial)
+{
+  xdg_wm_base_pong (xdg_wm_base, serial);
+}
+
+static const struct xdg_wm_base_listener xdg_wm_base_listener = {
+  handle_xdg_wm_base_ping,
+};
+
+static void
+handle_registry_global (void               *data,
+                        struct wl_registry *registry,
+                        uint32_t            id,
+                        const char         *interface,
+                        uint32_t            version)
+{
+  if (strcmp (interface, "wl_compositor") == 0)
+    {
+      compositor = wl_registry_bind (registry, id, &wl_compositor_interface, 1);
+    }
+  else if (strcmp (interface, "xdg_wm_base") == 0)
+    {
+      xdg_wm_base = wl_registry_bind (registry, id,
+                                      &xdg_wm_base_interface, 1);
+      xdg_wm_base_add_listener (xdg_wm_base, &xdg_wm_base_listener, NULL);
+    }
+  else if (strcmp (interface, "wl_shm") == 0)
+    {
+      shm = wl_registry_bind (registry,
+                              id, &wl_shm_interface, 1);
+    }
+}
+
+static void
+handle_registry_global_remove (void               *data,
+                               struct wl_registry *registry,
+                               uint32_t            name)
+{
+}
+
+static const struct wl_registry_listener registry_listener = {
+  handle_registry_global,
+  handle_registry_global_remove
+};
+
+static void
+test_empty_window_geometry (void)
+{
+  display = wl_display_connect (NULL);
+  registry = wl_display_get_registry (display);
+  wl_registry_add_listener (registry, &registry_listener, NULL);
+  wl_display_roundtrip (display);
+
+  g_assert_nonnull (shm);
+  g_assert_nonnull (xdg_wm_base);
+
+  wl_display_roundtrip (display);
+
+  surface = wl_compositor_create_surface (compositor);
+  xdg_surface = xdg_wm_base_get_xdg_surface (xdg_wm_base, surface);
+  xdg_surface_add_listener (xdg_surface, &xdg_surface_listener, NULL);
+  xdg_toplevel = xdg_surface_get_toplevel (xdg_surface);
+  xdg_toplevel_add_listener (xdg_toplevel, &xdg_toplevel_listener, NULL);
+
+  init_surface ();
+
+  running = TRUE;
+  while (running)
+    {
+      if (wl_display_dispatch (display) == -1)
+        return;
+    }
+
+  g_clear_pointer (&xdg_toplevel, xdg_toplevel_destroy);
+  g_clear_pointer (&xdg_surface, xdg_surface_destroy);
+  g_clear_pointer (&xdg_wm_base, xdg_wm_base_destroy);
+  g_clear_pointer (&compositor, wl_compositor_destroy);
+  g_clear_pointer (&shm, wl_shm_destroy);
+  g_clear_pointer (&registry, wl_registry_destroy);
+  g_clear_pointer (&display, wl_display_disconnect);
+}
+
+int
+main (int    argc,
+      char **argv)
+{
+  test_empty_window_geometry ();
+
+  return EXIT_SUCCESS;
+}
diff --git a/src/tests/wayland-test-clients/meson.build b/src/tests/wayland-test-clients/meson.build
index 4442b0aea0..5646493430 100644
--- a/src/tests/wayland-test-clients/meson.build
+++ b/src/tests/wayland-test-clients/meson.build
@@ -48,6 +48,7 @@ common_sources = [
 wayland_test_clients = [
   'subsurface-remap-toplevel',
   'invalid-subsurfaces',
+  'invalid-xdg-shell-actions',
 ]
 
 foreach test : wayland_test_clients
diff --git a/src/tests/wayland-unit-tests.c b/src/tests/wayland-unit-tests.c
index ad9179766c..507c9dba4d 100644
--- a/src/tests/wayland-unit-tests.c
+++ b/src/tests/wayland-unit-tests.c
@@ -143,6 +143,18 @@ subsurface_invalid_subsurfaces (void)
   g_test_assert_expected_messages ();
 }
 
+static void
+subsurface_invalid_xdg_shell_actions (void)
+{
+  WaylandTestClient *wayland_test_client;
+
+  wayland_test_client = wayland_test_client_new ("invalid-xdg-shell-actions");
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
+                         "Invalid geometry * set on xdg_surface*");
+  wayland_test_client_finish (wayland_test_client);
+  g_test_assert_expected_messages ();
+}
+
 static void
 on_actor_destroyed (ClutterActor       *actor,
                     struct wl_resource *callback)
@@ -216,4 +228,6 @@ init_wayland_tests (void)
                    subsurface_remap_toplevel);
   g_test_add_func ("/wayland/subsurface/invalid-subsurfaces",
                    subsurface_invalid_subsurfaces);
+  g_test_add_func ("/wayland/subsurface/invalid-xdg-shell-actions",
+                   subsurface_invalid_xdg_shell_actions);
 }
diff --git a/src/wayland/meta-wayland-legacy-xdg-shell.c b/src/wayland/meta-wayland-legacy-xdg-shell.c
index 926ee14e32..6dc8cc402f 100644
--- a/src/wayland/meta-wayland-legacy-xdg-shell.c
+++ b/src/wayland/meta-wayland-legacy-xdg-shell.c
@@ -1316,6 +1316,15 @@ zxdg_surface_v6_set_window_geometry (struct wl_client   *client,
   MetaWaylandSurface *surface = surface_from_xdg_surface_resource (resource);
   MetaWaylandSurfaceState *pending;
 
+  if (width == 0 || height == 0)
+    {
+      g_warning ("Invalid geometry %dx%d+%d+%d set on xdg_surface@%d. Ignoring for "
+                 "now, but this will result in client termination in the future.",
+                 width, height, x, y,
+                 wl_resource_get_id (resource));
+      return;
+    }
+
   pending = meta_wayland_surface_get_pending_state (surface);
   pending->has_new_geometry = TRUE;
   pending->new_geometry.x = x;
diff --git a/src/wayland/meta-wayland-xdg-shell.c b/src/wayland/meta-wayland-xdg-shell.c
index 176f8a908f..4a173d3133 100644
--- a/src/wayland/meta-wayland-xdg-shell.c
+++ b/src/wayland/meta-wayland-xdg-shell.c
@@ -1486,6 +1486,15 @@ xdg_surface_set_window_geometry (struct wl_client   *client,
   MetaWaylandSurface *surface = surface_from_xdg_surface_resource (resource);
   MetaWaylandSurfaceState *pending;
 
+  if (width == 0 || height == 0)
+    {
+      g_warning ("Invalid geometry %dx%d+%d+%d set on xdg_surface@%d. Ignoring for "
+                 "now, but this will result in client termination in the future.",
+                 width, height, x, y,
+                 wl_resource_get_id (resource));
+      return;
+    }
+
   pending = meta_wayland_surface_get_pending_state (surface);
   pending->has_new_geometry = TRUE;
   pending->new_geometry.x = x;
diff --git a/src/wayland/meta-window-wayland.c b/src/wayland/meta-window-wayland.c
index 977526fe2f..0f6c8832f0 100644
--- a/src/wayland/meta-window-wayland.c
+++ b/src/wayland/meta-window-wayland.c
@@ -53,6 +53,7 @@ struct _MetaWindowWayland
   GList *pending_configurations;
   gboolean has_pending_state_change;
 
+  gboolean has_last_sent_configuration;
   int last_sent_x;
   int last_sent_y;
   int last_sent_width;
@@ -188,6 +189,8 @@ surface_state_changed (MetaWindow *window)
   if (window->unmanaging)
     return;
 
+  g_return_if_fail (wl_window->has_last_sent_configuration);
+
   configuration =
     meta_wayland_window_configuration_new (wl_window->last_sent_x,
                                            wl_window->last_sent_y,
@@ -385,6 +388,7 @@ meta_window_wayland_move_resize_internal (MetaWindow                *window,
         }
     }
 
+  wl_window->has_last_sent_configuration = TRUE;
   wl_window->last_sent_x = configured_x;
   wl_window->last_sent_y = configured_y;
   wl_window->last_sent_width = configured_width;