[PATCH] ui/gtk: Handle empty notebook state in menu handlers

dongwon.kim@intel.com posted 1 patch 2 weeks, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260907201724.523270-1-dongwon.kim@intel.com
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>
ui/gtk.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 50 insertions(+), 6 deletions(-)
[PATCH] ui/gtk: Handle empty notebook state in menu handlers
Posted by dongwon.kim@intel.com 2 weeks, 4 days ago
From: Dongwon Kim <dongwon.kim@intel.com>

When the GTK notebook has no active page (e.g. before pages are added
or if all pages are detached/removed), gtk_notebook_get_current_page()
returns -1. Previously, this value was passed unchecked to
gd_vc_find_by_page(), which could result in unexpected lookups or NULL
dereferences.

Update gd_vc_find_current() to explicitly return NULL when page < 0.
Additionally, add NULL checks for the current VirtualConsole across all
relevant UI menu callbacks to avoid dereferencing an invalid or NULL
pointer when no console tab is currently focused.

Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 7078d89d67..0b5139ef31 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -184,6 +184,11 @@ static VirtualConsole *gd_vc_find_current(GtkDisplayState *s)
     gint page;
 
     page = gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook));
+
+    if (page < 0) {
+        return NULL;
+    }
+
     return gd_vc_find_by_page(s, page);
 }
 
@@ -1462,7 +1467,10 @@ static void gd_menu_show_tabs(GtkMenuItem *item, void *opaque)
     } else {
         gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
     }
-    gd_update_windowsize(vc);
+
+    if (vc) {
+        gd_update_windowsize(vc);
+    }
 }
 
 static int gd_vc_notebook_pos(GtkDisplayState *s, VirtualConsole *target)
@@ -1535,6 +1543,10 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
     GtkDisplayState *s = opaque;
     VirtualConsole *vc = gd_vc_find_current(s);
 
+    if (!vc) {
+        return;
+    }
+
     if (vc->type == GD_VC_GFX &&
         qemu_console_is_graphic(vc->gfx.dcl.con)) {
         gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
@@ -1588,7 +1600,10 @@ static void gd_menu_show_menubar(GtkMenuItem *item, void *opaque)
     } else {
         gtk_widget_hide(s->menu_bar);
     }
-    gd_update_windowsize(vc);
+
+    if (vc) {
+        gd_update_windowsize(vc);
+    }
 }
 
 static void gd_accel_show_menubar(void *opaque)
@@ -1605,7 +1620,7 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
     if (!s->full_screen) {
         gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
         gtk_widget_hide(s->menu_bar);
-        if (vc->type == GD_VC_GFX) {
+        if (vc && vc->type == GD_VC_GFX) {
             gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
         }
         gtk_window_fullscreen(GTK_WINDOW(s->window));
@@ -1618,14 +1633,16 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
             gtk_widget_show(s->menu_bar);
         }
         s->full_screen = FALSE;
-        if (vc->type == GD_VC_GFX) {
+        if (vc && vc->type == GD_VC_GFX) {
             vc->gfx.scale_x = vc->gfx.preferred_scale;
             vc->gfx.scale_y = vc->gfx.preferred_scale;
             gd_update_windowsize(vc);
         }
     }
 
-    gd_update_cursor(vc);
+    if (vc) {
+        gd_update_cursor(vc);
+    }
 }
 
 static void gd_accel_full_screen(void *opaque)
@@ -1639,6 +1656,10 @@ static void gd_menu_zoom_in(GtkMenuItem *item, void *opaque)
     GtkDisplayState *s = opaque;
     VirtualConsole *vc = gd_vc_find_current(s);
 
+    if (!vc) {
+        return;
+    }
+
     gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->zoom_fit_item),
                                    FALSE);
 
@@ -1659,6 +1680,10 @@ static void gd_menu_zoom_out(GtkMenuItem *item, void *opaque)
     GtkDisplayState *s = opaque;
     VirtualConsole *vc = gd_vc_find_current(s);
 
+    if (!vc) {
+        return;
+    }
+
     gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->zoom_fit_item),
                                    FALSE);
 
@@ -1676,6 +1701,10 @@ static void gd_menu_zoom_fixed(GtkMenuItem *item, void *opaque)
     GtkDisplayState *s = opaque;
     VirtualConsole *vc = gd_vc_find_current(s);
 
+    if (!vc) {
+        return;
+    }
+
     vc->gfx.scale_x = vc->gfx.preferred_scale;
     vc->gfx.scale_y = vc->gfx.preferred_scale;
 
@@ -1687,6 +1716,10 @@ static void gd_menu_zoom_fit(GtkMenuItem *item, void *opaque)
     GtkDisplayState *s = opaque;
     VirtualConsole *vc = gd_vc_find_current(s);
 
+    if (!vc) {
+        return;
+    }
+
     if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->zoom_fit_item))) {
         s->free_scale = TRUE;
     } else {
@@ -1800,6 +1833,11 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
     VirtualConsole *vc = gd_vc_find_current(s);
 
     if (gd_is_grab_active(s)) {
+        if (!vc) {
+            gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
+                                           FALSE);
+            return;
+        }
         gd_grab_keyboard(vc, "user-request-main-window");
         gd_grab_pointer(vc, "user-request-main-window");
     } else {
@@ -1807,7 +1845,9 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
         gd_ungrab_pointer(s);
     }
 
-    gd_update_cursor(vc);
+    if (vc) {
+        gd_update_cursor(vc);
+    }
 }
 
 static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2,
@@ -1983,6 +2023,10 @@ static void gd_menu_copy(GtkMenuItem *item, void *opaque)
     GtkDisplayState *s = opaque;
     VirtualConsole *vc = gd_vc_find_current(s);
 
+    if (!vc) {
+        return;
+    }
+
 #if VTE_CHECK_VERSION(0, 50, 0)
     vte_terminal_copy_clipboard_format(VTE_TERMINAL(vc->vte.terminal),
                                        VTE_FORMAT_TEXT);
-- 
2.43.0


Re: [PATCH] ui/gtk: Handle empty notebook state in menu handlers
Posted by Marc-André Lureau 2 weeks, 4 days ago
Hi

On Tue, Sep 8, 2026 at 12:18 AM <dongwon.kim@intel.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> When the GTK notebook has no active page (e.g. before pages are added
> or if all pages are detached/removed), gtk_notebook_get_current_page()
> returns -1. Previously, this value was passed unchecked to
> gd_vc_find_by_page(), which could result in unexpected lookups or NULL
> dereferences.
>
> Update gd_vc_find_current() to explicitly return NULL when page < 0.
> Additionally, add NULL checks for the current VirtualConsole across all
> relevant UI menu callbacks to avoid dereferencing an invalid or NULL
> pointer when no console tab is currently focused.

This looks reasonable, but can it happen? if so, how? Rather than
handling a situation that should not happen, I would rather crash or
abort if we broke an invariant.

thanks

>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  ui/gtk.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 7078d89d67..0b5139ef31 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -184,6 +184,11 @@ static VirtualConsole *gd_vc_find_current(GtkDisplayState *s)
>      gint page;
>
>      page = gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook));
> +
> +    if (page < 0) {
> +        return NULL;
> +    }
> +
>      return gd_vc_find_by_page(s, page);
>  }
>
> @@ -1462,7 +1467,10 @@ static void gd_menu_show_tabs(GtkMenuItem *item, void *opaque)
>      } else {
>          gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
>      }
> -    gd_update_windowsize(vc);
> +
> +    if (vc) {
> +        gd_update_windowsize(vc);
> +    }
>  }
>
>  static int gd_vc_notebook_pos(GtkDisplayState *s, VirtualConsole *target)
> @@ -1535,6 +1543,10 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
>      GtkDisplayState *s = opaque;
>      VirtualConsole *vc = gd_vc_find_current(s);
>
> +    if (!vc) {
> +        return;
> +    }
> +
>      if (vc->type == GD_VC_GFX &&
>          qemu_console_is_graphic(vc->gfx.dcl.con)) {
>          gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> @@ -1588,7 +1600,10 @@ static void gd_menu_show_menubar(GtkMenuItem *item, void *opaque)
>      } else {
>          gtk_widget_hide(s->menu_bar);
>      }
> -    gd_update_windowsize(vc);
> +
> +    if (vc) {
> +        gd_update_windowsize(vc);
> +    }
>  }
>
>  static void gd_accel_show_menubar(void *opaque)
> @@ -1605,7 +1620,7 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
>      if (!s->full_screen) {
>          gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
>          gtk_widget_hide(s->menu_bar);
> -        if (vc->type == GD_VC_GFX) {
> +        if (vc && vc->type == GD_VC_GFX) {
>              gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
>          }
>          gtk_window_fullscreen(GTK_WINDOW(s->window));
> @@ -1618,14 +1633,16 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
>              gtk_widget_show(s->menu_bar);
>          }
>          s->full_screen = FALSE;
> -        if (vc->type == GD_VC_GFX) {
> +        if (vc && vc->type == GD_VC_GFX) {
>              vc->gfx.scale_x = vc->gfx.preferred_scale;
>              vc->gfx.scale_y = vc->gfx.preferred_scale;
>              gd_update_windowsize(vc);
>          }
>      }
>
> -    gd_update_cursor(vc);
> +    if (vc) {
> +        gd_update_cursor(vc);
> +    }
>  }
>
>  static void gd_accel_full_screen(void *opaque)
> @@ -1639,6 +1656,10 @@ static void gd_menu_zoom_in(GtkMenuItem *item, void *opaque)
>      GtkDisplayState *s = opaque;
>      VirtualConsole *vc = gd_vc_find_current(s);
>
> +    if (!vc) {
> +        return;
> +    }
> +
>      gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->zoom_fit_item),
>                                     FALSE);
>
> @@ -1659,6 +1680,10 @@ static void gd_menu_zoom_out(GtkMenuItem *item, void *opaque)
>      GtkDisplayState *s = opaque;
>      VirtualConsole *vc = gd_vc_find_current(s);
>
> +    if (!vc) {
> +        return;
> +    }
> +
>      gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->zoom_fit_item),
>                                     FALSE);
>
> @@ -1676,6 +1701,10 @@ static void gd_menu_zoom_fixed(GtkMenuItem *item, void *opaque)
>      GtkDisplayState *s = opaque;
>      VirtualConsole *vc = gd_vc_find_current(s);
>
> +    if (!vc) {
> +        return;
> +    }
> +
>      vc->gfx.scale_x = vc->gfx.preferred_scale;
>      vc->gfx.scale_y = vc->gfx.preferred_scale;
>
> @@ -1687,6 +1716,10 @@ static void gd_menu_zoom_fit(GtkMenuItem *item, void *opaque)
>      GtkDisplayState *s = opaque;
>      VirtualConsole *vc = gd_vc_find_current(s);
>
> +    if (!vc) {
> +        return;
> +    }
> +
>      if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->zoom_fit_item))) {
>          s->free_scale = TRUE;
>      } else {
> @@ -1800,6 +1833,11 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
>      VirtualConsole *vc = gd_vc_find_current(s);
>
>      if (gd_is_grab_active(s)) {
> +        if (!vc) {
> +            gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> +                                           FALSE);
> +            return;
> +        }
>          gd_grab_keyboard(vc, "user-request-main-window");
>          gd_grab_pointer(vc, "user-request-main-window");
>      } else {
> @@ -1807,7 +1845,9 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
>          gd_ungrab_pointer(s);
>      }
>
> -    gd_update_cursor(vc);
> +    if (vc) {
> +        gd_update_cursor(vc);
> +    }
>  }
>
>  static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2,
> @@ -1983,6 +2023,10 @@ static void gd_menu_copy(GtkMenuItem *item, void *opaque)
>      GtkDisplayState *s = opaque;
>      VirtualConsole *vc = gd_vc_find_current(s);
>
> +    if (!vc) {
> +        return;
> +    }
> +
>  #if VTE_CHECK_VERSION(0, 50, 0)
>      vte_terminal_copy_clipboard_format(VTE_TERMINAL(vc->vte.terminal),
>                                         VTE_FORMAT_TEXT);
> --
> 2.43.0
>
>


-- 
Marc-André Lureau
RE: [PATCH] ui/gtk: Handle empty notebook state in menu handlers
Posted by Kim, Dongwon 2 weeks, 4 days ago
Hi Marc-André,

> Subject: Re: [PATCH] ui/gtk: Handle empty notebook state in menu handlers
> 
> Hi
> 
> On Tue, Sep 8, 2026 at 12:18 AM <dongwon.kim@intel.com> wrote:
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > When the GTK notebook has no active page (e.g. before pages are added
> > or if all pages are detached/removed), gtk_notebook_get_current_page()
> > returns -1. Previously, this value was passed unchecked to
> > gd_vc_find_by_page(), which could result in unexpected lookups or NULL
> > dereferences.
> >
> > Update gd_vc_find_current() to explicitly return NULL when page < 0.
> > Additionally, add NULL checks for the current VirtualConsole across
> > all relevant UI menu callbacks to avoid dereferencing an invalid or
> > NULL pointer when no console tab is currently focused.
> 
> This looks reasonable, but can it happen? if so, how? Rather than handling a
> situation that should not happen, I would rather crash or abort if we broke an
> invariant.
> 
> thanks

The mention of "NULL dereferences" in the commit message was inaccurate.
It's not causing a crash, but rather logic bugs. This happens when all VC tabs
are detached (untabified). In that state:

gtk_notebook_get_current_page() returns -1.

For any detached tab, gtk_notebook_page_num() also returns -1.

Consequently, gd_vc_find_by_page(s, -1) matches the very first detached
console in s->vcs. Menu actions triggered on the empty main window mistakenly
route to that detached window instead of being ignored.

Thanks!

> 
> >
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  ui/gtk.c | 56
> > ++++++++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 50 insertions(+), 6 deletions(-)
> >
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 7078d89d67..0b5139ef31 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -184,6 +184,11 @@ static VirtualConsole
> *gd_vc_find_current(GtkDisplayState *s)
> >      gint page;
> >
> >      page = gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook));
> > +
> > +    if (page < 0) {
> > +        return NULL;
> > +    }
> > +
> >      return gd_vc_find_by_page(s, page);  }
> >
> > @@ -1462,7 +1467,10 @@ static void gd_menu_show_tabs(GtkMenuItem
> *item, void *opaque)
> >      } else {
> >          gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
> >      }
> > -    gd_update_windowsize(vc);
> > +
> > +    if (vc) {
> > +        gd_update_windowsize(vc);
> > +    }
> >  }
> >
> >  static int gd_vc_notebook_pos(GtkDisplayState *s, VirtualConsole
> > *target) @@ -1535,6 +1543,10 @@ static void
> gd_menu_untabify(GtkMenuItem *item, void *opaque)
> >      GtkDisplayState *s = opaque;
> >      VirtualConsole *vc = gd_vc_find_current(s);
> >
> > +    if (!vc) {
> > +        return;
> > +    }
> > +
> >      if (vc->type == GD_VC_GFX &&
> >          qemu_console_is_graphic(vc->gfx.dcl.con)) {
> >
> > gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> > @@ -1588,7 +1600,10 @@ static void
> gd_menu_show_menubar(GtkMenuItem *item, void *opaque)
> >      } else {
> >          gtk_widget_hide(s->menu_bar);
> >      }
> > -    gd_update_windowsize(vc);
> > +
> > +    if (vc) {
> > +        gd_update_windowsize(vc);
> > +    }
> >  }
> >
> >  static void gd_accel_show_menubar(void *opaque) @@ -1605,7 +1620,7
> @@
> > static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
> >      if (!s->full_screen) {
> >          gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
> >          gtk_widget_hide(s->menu_bar);
> > -        if (vc->type == GD_VC_GFX) {
> > +        if (vc && vc->type == GD_VC_GFX) {
> >              gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
> >          }
> >          gtk_window_fullscreen(GTK_WINDOW(s->window));
> > @@ -1618,14 +1633,16 @@ static void gd_menu_full_screen(GtkMenuItem
> *item, void *opaque)
> >              gtk_widget_show(s->menu_bar);
> >          }
> >          s->full_screen = FALSE;
> > -        if (vc->type == GD_VC_GFX) {
> > +        if (vc && vc->type == GD_VC_GFX) {
> >              vc->gfx.scale_x = vc->gfx.preferred_scale;
> >              vc->gfx.scale_y = vc->gfx.preferred_scale;
> >              gd_update_windowsize(vc);
> >          }
> >      }
> >
> > -    gd_update_cursor(vc);
> > +    if (vc) {
> > +        gd_update_cursor(vc);
> > +    }
> >  }
> >
> >  static void gd_accel_full_screen(void *opaque) @@ -1639,6 +1656,10 @@
> > static void gd_menu_zoom_in(GtkMenuItem *item, void *opaque)
> >      GtkDisplayState *s = opaque;
> >      VirtualConsole *vc = gd_vc_find_current(s);
> >
> > +    if (!vc) {
> > +        return;
> > +    }
> > +
> >      gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> >zoom_fit_item),
> >                                     FALSE);
> >
> > @@ -1659,6 +1680,10 @@ static void gd_menu_zoom_out(GtkMenuItem
> *item, void *opaque)
> >      GtkDisplayState *s = opaque;
> >      VirtualConsole *vc = gd_vc_find_current(s);
> >
> > +    if (!vc) {
> > +        return;
> > +    }
> > +
> >      gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> >zoom_fit_item),
> >                                     FALSE);
> >
> > @@ -1676,6 +1701,10 @@ static void gd_menu_zoom_fixed(GtkMenuItem
> *item, void *opaque)
> >      GtkDisplayState *s = opaque;
> >      VirtualConsole *vc = gd_vc_find_current(s);
> >
> > +    if (!vc) {
> > +        return;
> > +    }
> > +
> >      vc->gfx.scale_x = vc->gfx.preferred_scale;
> >      vc->gfx.scale_y = vc->gfx.preferred_scale;
> >
> > @@ -1687,6 +1716,10 @@ static void gd_menu_zoom_fit(GtkMenuItem
> *item, void *opaque)
> >      GtkDisplayState *s = opaque;
> >      VirtualConsole *vc = gd_vc_find_current(s);
> >
> > +    if (!vc) {
> > +        return;
> > +    }
> > +
> >      if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s-
> >zoom_fit_item))) {
> >          s->free_scale = TRUE;
> >      } else {
> > @@ -1800,6 +1833,11 @@ static void gd_menu_grab_input(GtkMenuItem
> *item, void *opaque)
> >      VirtualConsole *vc = gd_vc_find_current(s);
> >
> >      if (gd_is_grab_active(s)) {
> > +        if (!vc) {
> > +            gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> >grab_item),
> > +                                           FALSE);
> > +            return;
> > +        }
> >          gd_grab_keyboard(vc, "user-request-main-window");
> >          gd_grab_pointer(vc, "user-request-main-window");
> >      } else {
> > @@ -1807,7 +1845,9 @@ static void gd_menu_grab_input(GtkMenuItem
> *item, void *opaque)
> >          gd_ungrab_pointer(s);
> >      }
> >
> > -    gd_update_cursor(vc);
> > +    if (vc) {
> > +        gd_update_cursor(vc);
> > +    }
> >  }
> >
> >  static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint
> > arg2, @@ -1983,6 +2023,10 @@ static void gd_menu_copy(GtkMenuItem
> *item, void *opaque)
> >      GtkDisplayState *s = opaque;
> >      VirtualConsole *vc = gd_vc_find_current(s);
> >
> > +    if (!vc) {
> > +        return;
> > +    }
> > +
> >  #if VTE_CHECK_VERSION(0, 50, 0)
> >      vte_terminal_copy_clipboard_format(VTE_TERMINAL(vc->vte.terminal),
> >                                         VTE_FORMAT_TEXT);
> > --
> > 2.43.0
> >
> >
> 
> 
> --
> Marc-André Lureau
Re: [PATCH] ui/gtk: Handle empty notebook state in menu handlers
Posted by Marc-André Lureau 2 weeks, 3 days ago
Hi

On Tue, Sep 8, 2026 at 8:49 PM Kim, Dongwon <dongwon.kim@intel.com> wrote:
>
> Hi Marc-André,
>
> > Subject: Re: [PATCH] ui/gtk: Handle empty notebook state in menu handlers
> >
> > Hi
> >
> > On Tue, Sep 8, 2026 at 12:18 AM <dongwon.kim@intel.com> wrote:
> > >
> > > From: Dongwon Kim <dongwon.kim@intel.com>
> > >
> > > When the GTK notebook has no active page (e.g. before pages are added
> > > or if all pages are detached/removed), gtk_notebook_get_current_page()
> > > returns -1. Previously, this value was passed unchecked to
> > > gd_vc_find_by_page(), which could result in unexpected lookups or NULL
> > > dereferences.
> > >
> > > Update gd_vc_find_current() to explicitly return NULL when page < 0.
> > > Additionally, add NULL checks for the current VirtualConsole across
> > > all relevant UI menu callbacks to avoid dereferencing an invalid or
> > > NULL pointer when no console tab is currently focused.
> >
> > This looks reasonable, but can it happen? if so, how? Rather than handling a
> > situation that should not happen, I would rather crash or abort if we broke an
> > invariant.
> >
> > thanks
>
> The mention of "NULL dereferences" in the commit message was inaccurate.
> It's not causing a crash, but rather logic bugs. This happens when all VC tabs
> are detached (untabified). In that state:
>
> gtk_notebook_get_current_page() returns -1.
>
> For any detached tab, gtk_notebook_page_num() also returns -1.
>
> Consequently, gd_vc_find_by_page(s, -1) matches the very first detached
> console in s->vcs. Menu actions triggered on the empty main window mistakenly
> route to that detached window instead of being ignored.
>

Please correct the commit message then
thanks

> Thanks!
>
> >
> > >
> > > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > > ---
> > >  ui/gtk.c | 56
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++------
> > >  1 file changed, 50 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > index 7078d89d67..0b5139ef31 100644
> > > --- a/ui/gtk.c
> > > +++ b/ui/gtk.c
> > > @@ -184,6 +184,11 @@ static VirtualConsole
> > *gd_vc_find_current(GtkDisplayState *s)
> > >      gint page;
> > >
> > >      page = gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook));
> > > +
> > > +    if (page < 0) {
> > > +        return NULL;
> > > +    }
> > > +
> > >      return gd_vc_find_by_page(s, page);  }
> > >
> > > @@ -1462,7 +1467,10 @@ static void gd_menu_show_tabs(GtkMenuItem
> > *item, void *opaque)
> > >      } else {
> > >          gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
> > >      }
> > > -    gd_update_windowsize(vc);
> > > +
> > > +    if (vc) {
> > > +        gd_update_windowsize(vc);
> > > +    }
> > >  }
> > >
> > >  static int gd_vc_notebook_pos(GtkDisplayState *s, VirtualConsole
> > > *target) @@ -1535,6 +1543,10 @@ static void
> > gd_menu_untabify(GtkMenuItem *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      if (vc->type == GD_VC_GFX &&
> > >          qemu_console_is_graphic(vc->gfx.dcl.con)) {
> > >
> > > gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> > > @@ -1588,7 +1600,10 @@ static void
> > gd_menu_show_menubar(GtkMenuItem *item, void *opaque)
> > >      } else {
> > >          gtk_widget_hide(s->menu_bar);
> > >      }
> > > -    gd_update_windowsize(vc);
> > > +
> > > +    if (vc) {
> > > +        gd_update_windowsize(vc);
> > > +    }
> > >  }
> > >
> > >  static void gd_accel_show_menubar(void *opaque) @@ -1605,7 +1620,7
> > @@
> > > static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
> > >      if (!s->full_screen) {
> > >          gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
> > >          gtk_widget_hide(s->menu_bar);
> > > -        if (vc->type == GD_VC_GFX) {
> > > +        if (vc && vc->type == GD_VC_GFX) {
> > >              gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
> > >          }
> > >          gtk_window_fullscreen(GTK_WINDOW(s->window));
> > > @@ -1618,14 +1633,16 @@ static void gd_menu_full_screen(GtkMenuItem
> > *item, void *opaque)
> > >              gtk_widget_show(s->menu_bar);
> > >          }
> > >          s->full_screen = FALSE;
> > > -        if (vc->type == GD_VC_GFX) {
> > > +        if (vc && vc->type == GD_VC_GFX) {
> > >              vc->gfx.scale_x = vc->gfx.preferred_scale;
> > >              vc->gfx.scale_y = vc->gfx.preferred_scale;
> > >              gd_update_windowsize(vc);
> > >          }
> > >      }
> > >
> > > -    gd_update_cursor(vc);
> > > +    if (vc) {
> > > +        gd_update_cursor(vc);
> > > +    }
> > >  }
> > >
> > >  static void gd_accel_full_screen(void *opaque) @@ -1639,6 +1656,10 @@
> > > static void gd_menu_zoom_in(GtkMenuItem *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> > >zoom_fit_item),
> > >                                     FALSE);
> > >
> > > @@ -1659,6 +1680,10 @@ static void gd_menu_zoom_out(GtkMenuItem
> > *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> > >zoom_fit_item),
> > >                                     FALSE);
> > >
> > > @@ -1676,6 +1701,10 @@ static void gd_menu_zoom_fixed(GtkMenuItem
> > *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      vc->gfx.scale_x = vc->gfx.preferred_scale;
> > >      vc->gfx.scale_y = vc->gfx.preferred_scale;
> > >
> > > @@ -1687,6 +1716,10 @@ static void gd_menu_zoom_fit(GtkMenuItem
> > *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s-
> > >zoom_fit_item))) {
> > >          s->free_scale = TRUE;
> > >      } else {
> > > @@ -1800,6 +1833,11 @@ static void gd_menu_grab_input(GtkMenuItem
> > *item, void *opaque)
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > >      if (gd_is_grab_active(s)) {
> > > +        if (!vc) {
> > > +            gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> > >grab_item),
> > > +                                           FALSE);
> > > +            return;
> > > +        }
> > >          gd_grab_keyboard(vc, "user-request-main-window");
> > >          gd_grab_pointer(vc, "user-request-main-window");
> > >      } else {
> > > @@ -1807,7 +1845,9 @@ static void gd_menu_grab_input(GtkMenuItem
> > *item, void *opaque)
> > >          gd_ungrab_pointer(s);
> > >      }
> > >
> > > -    gd_update_cursor(vc);
> > > +    if (vc) {
> > > +        gd_update_cursor(vc);
> > > +    }
> > >  }
> > >
> > >  static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint
> > > arg2, @@ -1983,6 +2023,10 @@ static void gd_menu_copy(GtkMenuItem
> > *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >  #if VTE_CHECK_VERSION(0, 50, 0)
> > >      vte_terminal_copy_clipboard_format(VTE_TERMINAL(vc->vte.terminal),
> > >                                         VTE_FORMAT_TEXT);
> > > --
> > > 2.43.0
> > >
> > >
> >
> >
> > --
> > Marc-André Lureau



-- 
Marc-André Lureau