Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate the return value of XListInputDevices #7

Closed
wants to merge 1 commit into from
Closed

Validate the return value of XListInputDevices #7

wants to merge 1 commit into from

Conversation

joukewitteveen
Copy link

Since libxi 1.7.7 XListInputDevices returns NULL when it fails while num_devices will not be set to 0.

Since libxi 1.7.7 XListInputDevices returns NULL when it fails while num_devices will not be set to 0.
gnomesysadmins pushed a commit that referenced this pull request Jan 23, 2020
printer_name_compressed_strv is NULL-terminated array
of gchar*, which means N+1 memory should be allocated.

Otherwise, if the printer name has no empty components
(which is usually the case), printer_name_compressed_strv[N],
which should contain the NULL sentinel, will actually lie
just outside of allocated memory, which is UB.

In my case, it led to crashes inside g_strjoinv
when Print... dialog is opened in evince.

    #0  0x00007fad2ce1bad7 in __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:96
    #1  0x00007fad2d04d88d in g_strjoinv (separator=separator@entry=0x7fad0c9bc508 "-", str_array=str_array@entry=0x556b017f0200) at ../glib-2.60.7/glib/gstrfuncs.c:2585
    #2  0x00007fad0c9b8a89 in avahi_service_resolver_cb (source_object=<optimized out>, res=<optimized out>, user_data=0x7fad08020ee0) at /var/tmp/portage/x11-libs/gtk+-3.24.13/work/gtk+-3.24.13/modules/printbackends/cups/gtkprintbackendcups.c:3223
    #3  0x00007fad2d1f8ed3 in g_task_return_now (task=0x556b017a8b00 [GTask]) at ../glib-2.60.7/gio/gtask.c:1209
    #4  0x00007fad2d1f987d in g_task_return (task=0x556b017a8b00 [GTask], type=<optimized out>) at ../glib-2.60.7/gio/gtask.c:1278
    #5  0x00007fad2d1f9dec in g_task_return (type=G_TASK_RETURN_SUCCESS, task=<optimized out>) at ../glib-2.60.7/gio/gtask.c:1678
    #6  0x00007fad2d1f9dec in g_task_return_pointer (task=<optimized out>, result=<optimized out>, result_destroy=<optimized out>) at ../glib-2.60.7/gio/gtask.c:1683
    #7  0x00007fad2d24b6af in g_dbus_connection_call_done (source=<optimized out>, result=0x556b017a8bc0, user_data=0x556b017a8b00) at ../glib-2.60.7/gio/gdbusconnection.c:5747
    #8  0x00007fad2d1f8ed3 in g_task_return_now (task=0x556b017a8bc0 [GTask]) at ../glib-2.60.7/gio/gtask.c:1209
    #9  0x00007fad2d1f8f09 in complete_in_idle_cb (task=0x556b017a8bc0) at ../glib-2.60.7/gio/gtask.c:1223
    #10 0x00007fad2d02d2c0 in g_main_dispatch (context=0x556b00eee090) at ../glib-2.60.7/glib/gmain.c:3189
    #11 0x00007fad2d02d2c0 in g_main_context_dispatch (context=context@entry=0x556b00eee090) at ../glib-2.60.7/glib/gmain.c:3854
    #12 0x00007fad2d02d658 in g_main_context_iterate (context=context@entry=0x556b00eee090, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib-2.60.7/glib/gmain.c:3927
    #13 0x00007fad2d02d6df in g_main_context_iteration (context=context@entry=0x556b00eee090, may_block=may_block@entry=1) at ../glib-2.60.7/glib/gmain.c:3988
    #14 0x00007fad2d22248d in g_application_run (application=0x556b0116f130 [EvApplication], argc=<optimized out>, argv=<optimized out>) at ../glib-2.60.7/gio/gapplication.c:2519
    #15 0x0000556b002e55a1 in  ()
    #16 0x00007fad2ccd6f1b in __libc_start_main (main=0x556b002e50d0, argc=2, argv=0x7ffe1057fa88, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffe1057fa78) at ../csu/libc-start.c:308
    #17 0x0000556b002e567a in  ()

    (gdb) p printer_name_compressed_strv[0]
    $4 = (gchar *) 0x556d4a4be430 "Brother"
    (gdb) p printer_name_compressed_strv[1]
    $5 = (gchar *) 0x7f9dbc011090 "MFC"
    (gdb) p printer_name_compressed_strv[2]
    $6 = (gchar *) 0x556d4a51ba50 "7860DW"
    (gdb) p printer_name_compressed_strv[3]
    $7 = (gchar *) 0x401 <error: Cannot access memory at address 0x401>
gnomesysadmins pushed a commit that referenced this pull request Jan 23, 2020
printer_name_compressed_strv is NULL-terminated array
of gchar*, which means N+1 memory should be allocated.

Otherwise, if the printer name has no empty components
(which is usually the case), printer_name_compressed_strv[N],
which should contain the NULL sentinel, will actually lie
just outside of allocated memory, which is UB.

In my case, it led to crashes inside g_strjoinv
when Print... dialog is opened in evince.

    #0  0x00007fad2ce1bad7 in __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:96
    #1  0x00007fad2d04d88d in g_strjoinv (separator=separator@entry=0x7fad0c9bc508 "-", str_array=str_array@entry=0x556b017f0200) at ../glib-2.60.7/glib/gstrfuncs.c:2585
    #2  0x00007fad0c9b8a89 in avahi_service_resolver_cb (source_object=<optimized out>, res=<optimized out>, user_data=0x7fad08020ee0) at /var/tmp/portage/x11-libs/gtk+-3.24.13/work/gtk+-3.24.13/modules/printbackends/cups/gtkprintbackendcups.c:3223
    #3  0x00007fad2d1f8ed3 in g_task_return_now (task=0x556b017a8b00 [GTask]) at ../glib-2.60.7/gio/gtask.c:1209
    #4  0x00007fad2d1f987d in g_task_return (task=0x556b017a8b00 [GTask], type=<optimized out>) at ../glib-2.60.7/gio/gtask.c:1278
    #5  0x00007fad2d1f9dec in g_task_return (type=G_TASK_RETURN_SUCCESS, task=<optimized out>) at ../glib-2.60.7/gio/gtask.c:1678
    #6  0x00007fad2d1f9dec in g_task_return_pointer (task=<optimized out>, result=<optimized out>, result_destroy=<optimized out>) at ../glib-2.60.7/gio/gtask.c:1683
    #7  0x00007fad2d24b6af in g_dbus_connection_call_done (source=<optimized out>, result=0x556b017a8bc0, user_data=0x556b017a8b00) at ../glib-2.60.7/gio/gdbusconnection.c:5747
    #8  0x00007fad2d1f8ed3 in g_task_return_now (task=0x556b017a8bc0 [GTask]) at ../glib-2.60.7/gio/gtask.c:1209
    #9  0x00007fad2d1f8f09 in complete_in_idle_cb (task=0x556b017a8bc0) at ../glib-2.60.7/gio/gtask.c:1223
    #10 0x00007fad2d02d2c0 in g_main_dispatch (context=0x556b00eee090) at ../glib-2.60.7/glib/gmain.c:3189
    #11 0x00007fad2d02d2c0 in g_main_context_dispatch (context=context@entry=0x556b00eee090) at ../glib-2.60.7/glib/gmain.c:3854
    #12 0x00007fad2d02d658 in g_main_context_iterate (context=context@entry=0x556b00eee090, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib-2.60.7/glib/gmain.c:3927
    #13 0x00007fad2d02d6df in g_main_context_iteration (context=context@entry=0x556b00eee090, may_block=may_block@entry=1) at ../glib-2.60.7/glib/gmain.c:3988
    #14 0x00007fad2d22248d in g_application_run (application=0x556b0116f130 [EvApplication], argc=<optimized out>, argv=<optimized out>) at ../glib-2.60.7/gio/gapplication.c:2519
    #15 0x0000556b002e55a1 in  ()
    #16 0x00007fad2ccd6f1b in __libc_start_main (main=0x556b002e50d0, argc=2, argv=0x7ffe1057fa88, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffe1057fa78) at ../csu/libc-start.c:308
    #17 0x0000556b002e567a in  ()

    (gdb) p printer_name_compressed_strv[0]
    $4 = (gchar *) 0x556d4a4be430 "Brother"
    (gdb) p printer_name_compressed_strv[1]
    $5 = (gchar *) 0x7f9dbc011090 "MFC"
    (gdb) p printer_name_compressed_strv[2]
    $6 = (gchar *) 0x556d4a51ba50 "7860DW"
    (gdb) p printer_name_compressed_strv[3]
    $7 = (gchar *) 0x401 <error: Cannot access memory at address 0x401>
gnomesysadmins pushed a commit that referenced this pull request Dec 8, 2022
Currently, the GdkSurfaceX11 implementation relies that the upper
layers hid the surface before destruction, and that no
GdkSurfaceClass.compute_resize happened between them. If these
circumstances happened, there would be a compute_size timeout left
dangling after the surface got destroyed, poking at incorrect data
later on. Something that looks like this was reported in the
recent mutter-x11-frames "SSD frames server":

    mutter-x11-frames:423016): GLib-GObject-WARNING **: 19:41:16.869: invalid unclassed pointer in cast to 'GtkWindow'

    Thread 1 "mutter-x11-fram" received signal SIGTRAP, Trace/breakpoint trap.
    g_logv (log_domain=0x7ffff7f7c4f8 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=<optimized out>, args=<optimized out>) at ../../../glib/gmessages.c:1433
    1433	../../../glib/gmessages.c: No such file or directory.
    (gdb) bt
    #0  g_logv (log_domain=0x7ffff7f7c4f8 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=<optimized out>, args=<optimized out>) at ../../../glib/gmessages.c:1433
    #1  0x00007ffff73470ff in g_log (log_domain=log_domain@entry=0x7ffff7f7c4f8 "GLib-GObject", log_level=log_level@entry=G_LOG_LEVEL_WARNING, format=format@entry=0x7ffff7f84da8 "invalid unclassed pointer in cast to '%s'")
        at ../../../glib/gmessages.c:1471
    #2  0x00007ffff7f72892 in g_type_check_instance_cast (type_instance=type_instance@entry=0x5555558e04b0, iface_type=<optimized out>) at ../../../gobject/gtype.c:4144
    #3  0x00007ffff791e77d in toplevel_compute_size (toplevel=<optimized out>, size=0x7fffffffe170, widget=0x5555558e04b0) at ../../../gtk/gtkwindow.c:4227
    #4  0x00007ffff7f4f3b0 in g_closure_invoke (closure=0x555555898cc0, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7fffffffdeb0, invocation_hint=invocation_hint@entry=0x7fffffffde30)
        at ../../../gobject/gclosure.c:832
    #5  0x00007ffff7f62076 in signal_emit_unlocked_R
        (node=node@entry=0x55555588feb0, detail=detail@entry=0, instance=instance@entry=0x55555560e990, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffdeb0)
        at ../../../gobject/gsignal.c:3796
    #6  0x00007ffff7f68bf5 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffe050) at ../../../gobject/gsignal.c:3549
    #7  0x00007ffff7f68dbf in <emit signal ??? on instance 0x55555560e990 [GdkX11Toplevel]> (instance=<optimized out>, signal_id=<optimized out>, detail=detail@entry=0) at ../../../gobject/gsignal.c:3606
    #8  0x00007ffff7a8de96 in gdk_toplevel_notify_compute_size (toplevel=<optimized out>, size=size@entry=0x7fffffffe170) at ../../../gdk/gdktoplevel.c:112
    #9  0x00007ffff7a4b15a in compute_toplevel_size (surface=surface@entry=0x55555560e990 [GdkX11Toplevel], update_geometry=update_geometry@entry=1, width=width@entry=0x7fffffffe220, height=height@entry=0x7fffffffe224)
        at ../../../gdk/x11/gdksurface-x11.c:281
    #10 0x00007ffff7a4c3b2 in compute_size_idle (user_data=0x55555560e990) at ../../../gdk/x11/gdksurface-x11.c:356
    #11 0x00007ffff733f67f in g_main_dispatch (context=0x55555563f6e0) at ../../../glib/gmain.c:3444
    #12 g_main_context_dispatch (context=context@entry=0x55555563f6e0) at ../../../glib/gmain.c:4162
    #13 0x00007ffff733fa38 in g_main_context_iterate (context=0x55555563f6e0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../../../glib/gmain.c:4238
    #14 0x00007ffff733fcef in g_main_loop_run (loop=loop@entry=0x5555560874a0) at ../../../glib/gmain.c:4438
    #15 0x0000555555557de0 in main (argc=<optimized out>, argv=<optimized out>) at ../src/frames/main.c:68

It perhaps makes sense to warn in these situations, but either way
it sounds like gdk_surface_x11_finalize() could enforce the correct
behavior by ensuring there is no dangling timeouts/data. This commit
does that.
gnomesysadmins pushed a commit that referenced this pull request Dec 14, 2022
Currently, the GdkSurfaceX11 implementation relies that the upper
layers hid the surface before destruction, and that no
GdkSurfaceClass.compute_resize happened between them. If these
circumstances happened, there would be a compute_size timeout left
dangling after the surface got destroyed, poking at incorrect data
later on. Something that looks like this was reported in the
recent mutter-x11-frames "SSD frames server":

    mutter-x11-frames:423016): GLib-GObject-WARNING **: 19:41:16.869: invalid unclassed pointer in cast to 'GtkWindow'

    Thread 1 "mutter-x11-fram" received signal SIGTRAP, Trace/breakpoint trap.
    g_logv (log_domain=0x7ffff7f7c4f8 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=<optimized out>, args=<optimized out>) at ../../../glib/gmessages.c:1433
    1433	../../../glib/gmessages.c: No such file or directory.
    (gdb) bt
    #0  g_logv (log_domain=0x7ffff7f7c4f8 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=<optimized out>, args=<optimized out>) at ../../../glib/gmessages.c:1433
    #1  0x00007ffff73470ff in g_log (log_domain=log_domain@entry=0x7ffff7f7c4f8 "GLib-GObject", log_level=log_level@entry=G_LOG_LEVEL_WARNING, format=format@entry=0x7ffff7f84da8 "invalid unclassed pointer in cast to '%s'")
        at ../../../glib/gmessages.c:1471
    #2  0x00007ffff7f72892 in g_type_check_instance_cast (type_instance=type_instance@entry=0x5555558e04b0, iface_type=<optimized out>) at ../../../gobject/gtype.c:4144
    #3  0x00007ffff791e77d in toplevel_compute_size (toplevel=<optimized out>, size=0x7fffffffe170, widget=0x5555558e04b0) at ../../../gtk/gtkwindow.c:4227
    #4  0x00007ffff7f4f3b0 in g_closure_invoke (closure=0x555555898cc0, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7fffffffdeb0, invocation_hint=invocation_hint@entry=0x7fffffffde30)
        at ../../../gobject/gclosure.c:832
    #5  0x00007ffff7f62076 in signal_emit_unlocked_R
        (node=node@entry=0x55555588feb0, detail=detail@entry=0, instance=instance@entry=0x55555560e990, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffdeb0)
        at ../../../gobject/gsignal.c:3796
    #6  0x00007ffff7f68bf5 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffe050) at ../../../gobject/gsignal.c:3549
    #7  0x00007ffff7f68dbf in <emit signal ??? on instance 0x55555560e990 [GdkX11Toplevel]> (instance=<optimized out>, signal_id=<optimized out>, detail=detail@entry=0) at ../../../gobject/gsignal.c:3606
    #8  0x00007ffff7a8de96 in gdk_toplevel_notify_compute_size (toplevel=<optimized out>, size=size@entry=0x7fffffffe170) at ../../../gdk/gdktoplevel.c:112
    #9  0x00007ffff7a4b15a in compute_toplevel_size (surface=surface@entry=0x55555560e990 [GdkX11Toplevel], update_geometry=update_geometry@entry=1, width=width@entry=0x7fffffffe220, height=height@entry=0x7fffffffe224)
        at ../../../gdk/x11/gdksurface-x11.c:281
    #10 0x00007ffff7a4c3b2 in compute_size_idle (user_data=0x55555560e990) at ../../../gdk/x11/gdksurface-x11.c:356
    #11 0x00007ffff733f67f in g_main_dispatch (context=0x55555563f6e0) at ../../../glib/gmain.c:3444
    #12 g_main_context_dispatch (context=context@entry=0x55555563f6e0) at ../../../glib/gmain.c:4162
    #13 0x00007ffff733fa38 in g_main_context_iterate (context=0x55555563f6e0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../../../glib/gmain.c:4238
    #14 0x00007ffff733fcef in g_main_loop_run (loop=loop@entry=0x5555560874a0) at ../../../glib/gmain.c:4438
    #15 0x0000555555557de0 in main (argc=<optimized out>, argv=<optimized out>) at ../src/frames/main.c:68

It perhaps makes sense to warn in these situations, but either way
it sounds like gdk_surface_x11_finalize() could enforce the correct
behavior by ensuring there is no dangling timeouts/data. This commit
does that.
gnomesysadmins pushed a commit that referenced this pull request Dec 15, 2022
Currently, the GdkSurfaceX11 implementation relies that the upper
layers hid the surface before destruction, and that no
GdkSurfaceClass.compute_resize happened between them. If these
circumstances happened, there would be a compute_size timeout left
dangling after the surface got destroyed, poking at incorrect data
later on. Something that looks like this was reported in the
recent mutter-x11-frames "SSD frames server":

    mutter-x11-frames:423016): GLib-GObject-WARNING **: 19:41:16.869: invalid unclassed pointer in cast to 'GtkWindow'

    Thread 1 "mutter-x11-fram" received signal SIGTRAP, Trace/breakpoint trap.
    g_logv (log_domain=0x7ffff7f7c4f8 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=<optimized out>, args=<optimized out>) at ../../../glib/gmessages.c:1433
    1433	../../../glib/gmessages.c: No such file or directory.
    (gdb) bt
    #0  g_logv (log_domain=0x7ffff7f7c4f8 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=<optimized out>, args=<optimized out>) at ../../../glib/gmessages.c:1433
    #1  0x00007ffff73470ff in g_log (log_domain=log_domain@entry=0x7ffff7f7c4f8 "GLib-GObject", log_level=log_level@entry=G_LOG_LEVEL_WARNING, format=format@entry=0x7ffff7f84da8 "invalid unclassed pointer in cast to '%s'")
        at ../../../glib/gmessages.c:1471
    #2  0x00007ffff7f72892 in g_type_check_instance_cast (type_instance=type_instance@entry=0x5555558e04b0, iface_type=<optimized out>) at ../../../gobject/gtype.c:4144
    #3  0x00007ffff791e77d in toplevel_compute_size (toplevel=<optimized out>, size=0x7fffffffe170, widget=0x5555558e04b0) at ../../../gtk/gtkwindow.c:4227
    #4  0x00007ffff7f4f3b0 in g_closure_invoke (closure=0x555555898cc0, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7fffffffdeb0, invocation_hint=invocation_hint@entry=0x7fffffffde30)
        at ../../../gobject/gclosure.c:832
    #5  0x00007ffff7f62076 in signal_emit_unlocked_R
        (node=node@entry=0x55555588feb0, detail=detail@entry=0, instance=instance@entry=0x55555560e990, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffdeb0)
        at ../../../gobject/gsignal.c:3796
    #6  0x00007ffff7f68bf5 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffe050) at ../../../gobject/gsignal.c:3549
    #7  0x00007ffff7f68dbf in <emit signal ??? on instance 0x55555560e990 [GdkX11Toplevel]> (instance=<optimized out>, signal_id=<optimized out>, detail=detail@entry=0) at ../../../gobject/gsignal.c:3606
    #8  0x00007ffff7a8de96 in gdk_toplevel_notify_compute_size (toplevel=<optimized out>, size=size@entry=0x7fffffffe170) at ../../../gdk/gdktoplevel.c:112
    #9  0x00007ffff7a4b15a in compute_toplevel_size (surface=surface@entry=0x55555560e990 [GdkX11Toplevel], update_geometry=update_geometry@entry=1, width=width@entry=0x7fffffffe220, height=height@entry=0x7fffffffe224)
        at ../../../gdk/x11/gdksurface-x11.c:281
    #10 0x00007ffff7a4c3b2 in compute_size_idle (user_data=0x55555560e990) at ../../../gdk/x11/gdksurface-x11.c:356
    #11 0x00007ffff733f67f in g_main_dispatch (context=0x55555563f6e0) at ../../../glib/gmain.c:3444
    #12 g_main_context_dispatch (context=context@entry=0x55555563f6e0) at ../../../glib/gmain.c:4162
    #13 0x00007ffff733fa38 in g_main_context_iterate (context=0x55555563f6e0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../../../glib/gmain.c:4238
    #14 0x00007ffff733fcef in g_main_loop_run (loop=loop@entry=0x5555560874a0) at ../../../glib/gmain.c:4438
    #15 0x0000555555557de0 in main (argc=<optimized out>, argv=<optimized out>) at ../src/frames/main.c:68

It perhaps makes sense to warn in these situations, but either way
it sounds like gdk_surface_x11_finalize() could enforce the correct
behavior by ensuring there is no dangling timeouts/data. This commit
does that.
gnomesysadmins pushed a commit that referenced this pull request Jan 9, 2023
`gtk_at_spi_cache_add_context()` checks if the GtkAtSpiContext's path
is NULL before inserting the context object into the hash table.
Do the same in `gtk_at_spi_cache_remove_context()` to avoid a NULL
pointer dereference in `g_str_hash()` during the hash table lookup
if a context with NULL path is removed. That can happen when the
GtkAtSpiRoot::base_path is NULL, which, in turn, can happen if
`g_application_get_dbus_object_path()` returns NULL.

  ==394047==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fd1966f8b84 bp 0x7fff11e3ded0 sp 0x7fff11e3de58 T0)
  ==394047==The signal is caused by a READ memory access.
  ==394047==Hint: address points to the zero page.
      #0 0x7fd1966f8b84 in g_str_hash (/usr/lib/libglib-2.0.so.0+0x37b84)
      #1 0x7fd1966f9c09 in g_hash_table_contains (/usr/lib/libglib-2.0.so.0+0x38c09)
      #2 0x7fd196062c10 in gtk_at_spi_cache_remove_context ../gtk/a11y/gtkatspicache.c:447
      #3 0x7fd19606e0a9 in gtk_at_spi_root_unregister ../gtk/a11y/gtkatspiroot.c:653
      #4 0x7fd196067f58 in gtk_at_spi_context_unrealize ../gtk/a11y/gtkatspicontext.c:1559
      #5 0x7fd195ced97f in gtk_at_context_unrealize ../gtk/gtkatcontext.c:668
      #6 0x7fd195f5576e in gtk_widget_unroot_at_context ../gtk/gtkwidget.c:2399
      #7 0x7fd195f55bd2 in gtk_widget_unroot ../gtk/gtkwidget.c:2499
      ...
gnomesysadmins pushed a commit that referenced this pull request Mar 1, 2023
`gtk_at_spi_cache_add_context()` checks if the GtkAtSpiContext's path
is NULL before inserting the context object into the hash table.
Do the same in `gtk_at_spi_cache_remove_context()` to avoid a NULL
pointer dereference in `g_str_hash()` during the hash table lookup
if a context with NULL path is removed. That can happen when the
GtkAtSpiRoot::base_path is NULL, which, in turn, can happen if
`g_application_get_dbus_object_path()` returns NULL.

  ==394047==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fd1966f8b84 bp 0x7fff11e3ded0 sp 0x7fff11e3de58 T0)
  ==394047==The signal is caused by a READ memory access.
  ==394047==Hint: address points to the zero page.
      #0 0x7fd1966f8b84 in g_str_hash (/usr/lib/libglib-2.0.so.0+0x37b84)
      #1 0x7fd1966f9c09 in g_hash_table_contains (/usr/lib/libglib-2.0.so.0+0x38c09)
      #2 0x7fd196062c10 in gtk_at_spi_cache_remove_context ../gtk/a11y/gtkatspicache.c:447
      #3 0x7fd19606e0a9 in gtk_at_spi_root_unregister ../gtk/a11y/gtkatspiroot.c:653
      #4 0x7fd196067f58 in gtk_at_spi_context_unrealize ../gtk/a11y/gtkatspicontext.c:1559
      #5 0x7fd195ced97f in gtk_at_context_unrealize ../gtk/gtkatcontext.c:668
      #6 0x7fd195f5576e in gtk_widget_unroot_at_context ../gtk/gtkwidget.c:2399
      #7 0x7fd195f55bd2 in gtk_widget_unroot ../gtk/gtkwidget.c:2499
      ...
gnomesysadmins pushed a commit that referenced this pull request Mar 1, 2023
If the early return path in `emit_property_changed()` is taken, and
`value` is floating, it will be leaked. Fix that by sinking `value` on
entry to the function.

Spotted by asan:
```
Direct leak of 128 byte(s) in 2 object(s) allocated from:
    #0 0x7f44774ba6af in __interceptor_malloc (/lib64/libasan.so.8+0xba6af)
    #1 0x7f44764c941a in g_malloc ../../source/glib/glib/gmem.c:130
    #2 0x7f44764f6d8a in g_slice_alloc ../../source/glib/glib/gslice.c:252
    #3 0x7f447654655d in g_variant_alloc ../../source/glib/glib/gvariant-core.c:565
    #4 0x7f447654664c in g_variant_new_from_bytes ../../source/glib/glib/gvariant-core.c:608
    #5 0x7f4476536ed5 in g_variant_new_take_string ../../source/glib/glib/gvariant.c:1307
    #6 0x7f4475c75ada in gtk_at_spi_context_state_change ../../source/gtk4/gtk/a11y/gtkatspicontext.c:1112
    #7 0x7f44758ee194 in gtk_at_context_update ../../source/gtk4/gtk/gtkatcontext.c:694
    #8 0x7f44758dbfcf in gtk_accessible_update_property ../../source/gtk4/gtk/gtkaccessible.c:326
    #9 0x7f4475b5abe3 in gtk_widget_set_tooltip_text ../../source/gtk4/gtk/gtkwidget.c:9740
    #10 0x58439d in gs_updates_page_update_ui_state ../../source/gnome-software/src/gs-updates-page.c:302
    #11 0x5857dc in gs_updates_page_set_state ../../source/gnome-software/src/gs-updates-page.c:403
    #12 0x5879f1 in gs_updates_page_load ../../source/gnome-software/src/gs-updates-page.c:636
    #13 0x58822d in gs_updates_page_reload ../../source/gnome-software/src/gs-updates-page.c:678
    #14 0x50ff48 in gs_page_reload ../../source/gnome-software/src/gs-page.c:731
    #15 0x5491ce in gs_shell_reload_cb ../../source/gnome-software/src/gs-shell.c:830
    #16 0x7f4477363f54 in g_cclosure_marshal_VOID__VOID ../../source/glib/gobject/gmarshal.c:117
    #17 0x7f447735e0ad in g_closure_invoke ../../source/glib/gobject/gclosure.c:832
    #18 0x7f4477391f3f in signal_emit_unlocked_R ../../source/glib/gobject/gsignal.c:3802
    #19 0x7f4477390c13 in g_signal_emit_valist ../../source/glib/gobject/gsignal.c:3555
    #20 0x7f4477391324 in g_signal_emit ../../source/glib/gobject/gsignal.c:3612
    #21 0x7f447705b3c3 in gs_plugin_loader_reload_delay_cb ../../source/gnome-software/lib/gs-plugin-loader.c:1538
    #22 0x7f44764bd140 in g_timeout_dispatch ../../source/glib/glib/gmain.c:5054
    #23 0x7f44764b9eb1 in g_main_dispatch ../../source/glib/glib/gmain.c:3460
    #24 0x7f44764bb72c in g_main_context_dispatch ../../source/glib/glib/gmain.c:4200
    #25 0x7f44764bba15 in g_main_context_iterate ../../source/glib/glib/gmain.c:4276
    #26 0x7f44764bbbfa in g_main_context_iteration ../../source/glib/glib/gmain.c:4343
    #27 0x7f44769ef655 in g_application_run ../../source/glib/gio/gapplication.c:2589
    #28 0x4f2da5 in main ../../source/gnome-software/src/gs-main.c:49
    #29 0x7f4474e4a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f)
```

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
gnomesysadmins pushed a commit that referenced this pull request Sep 13, 2023
I was profiling Firefox startup on Wayland, and I realized that we were
doing too much work because we get extra settings changed events, posted
from here:

    #0  gdk_event_copy (event=0x7ffe9c4a7fd0) at ../gtk/gdk/gdkevents.c:659
    #1  0x00007f8b8a5060c6 in gdk_display_put_event (event=0x7ffe9c4a7fd0, display=0x7f8b8d551a00) at ../gtk/gdk/gdkdisplay.c:503
    #2  gdk_display_put_event (display=0x7f8b8d551a00, event=0x7ffe9c4a7fd0) at ../gtk/gdk/gdkdisplay.c:497
    #3  0x00007f8b8a5456e6 in notify_setting (setting=0x7f8b8a59b6d6 "gtk-xft-dpi", screen=0x7f8b8d517700) at ../gtk/gdk/wayland/gdkscreen-wayland.c:237
    #4  update_xft_settings (screen=0x7f8b8d517700) at ../gtk/gdk/wayland/gdkscreen-wayland.c:513
    #5  0x00007f8b8a53ef6d in init_settings (screen=0x7f8b8d517700) at ../gtk/gdk/wayland/gdkscreen-wayland.c:842
    #6  _gdk_wayland_screen_new (display=0x7f8b8d551a00) at ../gtk/gdk/wayland/gdkscreen-wayland.c:1367
    #7  _gdk_wayland_display_open (display_name=<optimized out>) at ../gtk/gdk/wayland/gdkdisplay-wayland.c:618
    #8  0x00007f8b8a507bc7 in gdk_display_manager_open_display (manager=<optimized out>, name=0x0) at ../gtk/gdk/gdkdisplaymanager.c:462
    #9  0x00007f8b82bda2f8 in XREMain::XRE_mainStartup(bool*) (this=this@entry=0x7ffe9c4a8398, aExitFlag=aExitFlag@entry=0x7ffe9c4a830f) at /home/emilio/src/moz/gecko-6/toolkit/xre/nsAppRunner.cpp:4760
    #10 0x00007f8b82be1742 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=this@entry=0x7ffe9c4a8398, argc=argc@entry=4, argv=argv@entry=0x7ffe9c4a9698, aConfig=...) at /home/emilio/src/moz/gecko-6/toolkit/xre/nsAppRunner.cpp:5874
    #11 0x00007f8b82be1c2c in XRE_main(int, char**, mozilla::BootstrapConfig const&) (argc=4, argv=0x7ffe9c4a9698, aConfig=...) at /home/emilio/src/moz/gecko-6/toolkit/xre/nsAppRunner.cpp:5942
    #12 0x000055631ef3b3e9 in do_main(int, char**, char**) (argc=4, argv=0x7ffe9c4a9698, envp=<optimized out>) at /home/emilio/src/moz/gecko-6/browser/app/nsBrowserApp.cpp:227
    #13 main(int, char**, char**) (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at /home/emilio/src/moz/gecko-6/browser/app/nsBrowserApp.cpp:445

We shouldn't post events for a screen we're just creating, because it
can make apps do too much work during startup. X11 had code for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant