From b3b2e61c198dcaaa94a715670ca4ecf7cba54c44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Mon, 1 Nov 2021 10:29:15 +0100 Subject: [PATCH] enable_paranoid_cb: Do not use g_steal_pointer in callee argument list The order in which the argument list is evaluated is unspecified, so it was possible for g_steal_pointer to set self = NULL before self->old_governor was evaluated, resulting in a crash: Program terminated with signal SIGSEGV, Segmentation fault. #0 0x00007f6efc678d84 in enable_paranoid_cb (object=0x561a4d147c80, result=0x561a4d173560, user_data=0x561a4d09d610) at ../src/libsysprof/sysprof-governor-source.c:290 290 self->old_governor, [Current thread is 1 (Thread 0x7f6efae4b600 (LWP 122786))] (gdb) bt #0 0x00007f6efc678d84 in enable_paranoid_cb (object=0x561a4d147c80 [SysprofHelpers], result=0x561a4d173560, user_data=0x561a4d09d610) at ../src/libsysprof/sysprof-governor-source.c:290 #1 0x00007f6efd51b389 in g_task_return_now (task=0x561a4d173560 [GTask]) at ../../../gio/gtask.c:1219 #2 0x00007f6efd51becb in g_task_return (type=, task=0x561a4d173560 [GTask]) at ../../../gio/gtask.c:1289 #3 g_task_return (task=0x561a4d173560 [GTask], type=) at ../../../gio/gtask.c:1245 #4 0x00007f6efc6a6060 in sysprof_helpers_set_paranoid_cb (object=0x561a4cf9df90 [IpcServiceProxy], result=0x561a4d1736e0, user_data=0x561a4d173560) at ../src/libsysprof/sysprof-helpers.c:788 #5 0x00007f6efd51b389 in g_task_return_now (task=0x561a4d1736e0 [GTask]) at ../../../gio/gtask.c:1219 #6 0x00007f6efd51becb in g_task_return (type=, task=0x561a4d1736e0 [GTask]) at ../../../gio/gtask.c:1289 #7 g_task_return (task=0x561a4d1736e0 [GTask], type=) at ../../../gio/gtask.c:1245 #8 0x00007f6efd5830cb in reply_cb (connection=, res=, user_data=user_data@entry=0x561a4d1736e0) at ../../../gio/gdbusproxy.c:2568 #9 0x00007f6efd51b389 in g_task_return_now (task=0x561a4d22b000 [GTask]) at ../../../gio/gtask.c:1219 #10 0x00007f6efd51becb in g_task_return (type=, task=0x561a4d22b000 [GTask]) at ../../../gio/gtask.c:1289 #11 g_task_return (task=0x561a4d22b000 [GTask], type=) at ../../../gio/gtask.c:1245 #12 0x00007f6efd577cbf in g_dbus_connection_call_done (source=, result=0x561a4d22b0c0, user_data=user_data@entry=0x561a4d22b000) at ../../../gio/gdbusconnection.c:5797 #13 0x00007f6efd51b389 in g_task_return_now (task=0x561a4d22b0c0 [GTask]) at ../../../gio/gtask.c:1219 #14 0x00007f6efd51b3c9 in complete_in_idle_cb (task=0x561a4d22b0c0) at ../../../gio/gtask.c:1233 #15 0x00007f6efd32db84 in g_main_dispatch (context=0x561a4cb60af0) at ../../../glib/gmain.c:3381 #16 g_main_context_dispatch (context=0x561a4cb60af0) at ../../../glib/gmain.c:4099 #17 0x00007f6efd32df28 in g_main_context_iterate (context=context@entry=0x561a4cb60af0, block=block@entry=1, dispatch=dispatch@entry=1, self=) at ../../../glib/gmain.c:4175 #18 0x00007f6efd32dfdf in g_main_context_iteration (context=context@entry=0x561a4cb60af0, may_block=may_block@entry=1) at ../../../glib/gmain.c:4240 #19 0x00007f6efd54a06d in g_application_run (application=0x561a4cb56120 [SysprofApplication], argc=, argv=) at ../../../gio/gapplication.c:2569 #20 0x0000561a4b2edd14 in main (argc=1, argv=0x7ffddcebb708) at ../src/sysprof/sysprof.c:44 (gdb) info locals helpers = 0x561a4d147c80 [SysprofHelpers] self = 0x0 error = 0x0 old_governor = -1 __func__ = "enable_paranoid_cb" --- src/libsysprof/sysprof-governor-source.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libsysprof/sysprof-governor-source.c b/src/libsysprof/sysprof-governor-source.c index 90192ad9..5277e01d 100644 --- a/src/libsysprof/sysprof-governor-source.c +++ b/src/libsysprof/sysprof-governor-source.c @@ -286,11 +286,18 @@ enable_paranoid_cb (GObject *object, if (!self->disable_governor) sysprof_source_emit_finished (SYSPROF_SOURCE (self)); else - sysprof_helpers_set_governor_async (helpers, - self->old_governor, - NULL, - enable_governor_cb, - g_steal_pointer (&self)); + { + sysprof_helpers_set_governor_async (helpers, + self->old_governor, + NULL, + enable_governor_cb, + self); + + /* Can't use g_steal_pointer above, as that might set self = NULL before + * self->old_governor is evaluated → crash + */ + self = NULL; + } } static void