Re: [PATCH v4 3/3] perf tools: fix GTK4 leftover show/hide calls and error-loop list race

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Using sigprocmask() to protect a shared list from a signal handler is insufficient in a multithreaded process, leaving a cross-thread race condition unfixed.
--

commit af57548ca0fbe17de3fec0bdb3270cfd5750a038
Author: Matt Turner <mattst88@gmail.com>

perf tools: fix GTK4 leftover show/hide calls and error-loop list race

This commit updates legacy GTK functions for GTK4 compatibility. It also
attempts to fix a list corruption issue in the error dialog by wrapping list
modifications with a signal block.

> diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
> index 8a922851257e7..f1dbd96e2370e 100644
> --- a/tools/perf/ui/gtk/util.c
> +++ b/tools/perf/ui/gtk/util.c

[ ... ]

> @@ -48,6 +49,25 @@ void perf_gtk__quit_error_dialog(void)
>  	g_slist_foreach(perf_gtk__error_loops, perf_gtk__quit_loop, NULL);
>  }
>  
> +/*
> + * perf_gtk__signal() runs perf_gtk__quit_error_dialog() from a signal
> + * handler, which can preempt perf_gtk__error() while it is updating
> + * perf_gtk__error_loops below. Block those signals around the update so
> + * the list is never traversed half-modified.
> + */
> +static void perf_gtk__block_exit_signals(sigset_t *old_set)
> +{
> +	sigset_t set;
> +
> +	sigemptyset(&set);
> +	sigaddset(&set, SIGSEGV);
> +	sigaddset(&set, SIGFPE);
> +	sigaddset(&set, SIGINT);
> +	sigaddset(&set, SIGQUIT);
> +	sigaddset(&set, SIGTERM);
> +	sigprocmask(SIG_BLOCK, &set, old_set);

[Severity: Medium]
Is sigprocmask() sufficient to prevent signal handler execution in a
multithreaded application?

Since perf uses multiple threads, sigprocmask() will only mask these
signals for the local thread. Can the kernel still deliver a signal like
SIGINT to another unmasked thread, causing it to execute the handler
concurrently?

Additionally, does blocking synchronous signals like SIGSEGV and SIGFPE
using sigprocmask() result in undefined behavior per the POSIX standard?

> +}
> +
>  static void perf_gtk__dialog_response(GtkDialog *dialog,

[ ... ]

> @@ -86,7 +107,9 @@ static int perf_gtk__error(const char *format, va_list args)
>  	 * outlive the dialog and hang.
>  	 */
>  	loop = g_main_loop_new(NULL, FALSE);
> +	perf_gtk__block_exit_signals(&old_set);
>  	perf_gtk__error_loops = g_slist_prepend(perf_gtk__error_loops, loop);
> +	sigprocmask(SIG_SETMASK, &old_set, NULL);

[Severity: Medium]
Because local signal masking doesn't prevent other threads from executing
the signal handler globally, can a user sending SIGINT still trigger the
signal handler on a different thread while the GTK thread is actively
modifying perf_gtk__error_loops here?

Would this concurrent modification and traversal of the GLib linked list
lead to memory corruption or crashes?

>  	g_signal_connect(dialog, "response",
>  			 G_CALLBACK(perf_gtk__dialog_response), NULL);
>  	g_signal_connect_swapped(dialog, "destroy",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-perf-gtk2-v4-0-97e92ee07214@gmail.com?part=3