[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <cd6bffc0-2d11-4f2f-be82-0f4504fb26d2@intel.com>
Date: Fri, 30 May 2025 17:14:27 -0700
From: Sohil Mehta <sohil.mehta@...el.com>
To: "Xin Li (Intel)" <xin@...or.com>
CC: <tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
<dave.hansen@...ux.intel.com>, <x86@...nel.org>, <hpa@...or.com>,
<shuah@...nel.org>, <andrew.cooper3@...rix.com>,
<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v1 1/1] selftests/x86: Add a test to detect infinite
sigtrap handler loop
On 5/30/2025 4:07 PM, Xin Li (Intel) wrote:
> When FRED is enabled, if the Trap Flag (TF) is set without an external
> debugger attached, it can lead to an infinite loop in the SIGTRAP
> handler. To avoid this, the software event flag in the augmented SS
> must be cleared, ensuring that no single-step trap remains pending when
> ERETU completes.
>
> This test checks for that specific scenario—verifying whether the kernel
> correctly prevents an infinite SIGTRAP loop in this edge case.
>
It isn't clear from the commit message whether the test is specific to
FRED or a generic one.
> Signed-off-by: Xin Li (Intel) <xin@...or.com>
> ---
> tools/testing/selftests/x86/Makefile | 2 +-
> .../selftests/x86/test_sigtrap_handler.c | 80 +++++++++++++++++++
> 2 files changed, 81 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/x86/test_sigtrap_handler.c
>
> diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
> index f703fcfe9f7c..c486fd88ebb1 100644
> --- a/tools/testing/selftests/x86/Makefile
> +++ b/tools/testing/selftests/x86/Makefile
> @@ -12,7 +12,7 @@ CAN_BUILD_WITH_NOPIE := $(shell ./check_cc.sh "$(CC)" trivial_program.c -no-pie)
>
> TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \
> check_initial_reg_state sigreturn iopl ioperm \
> - test_vsyscall mov_ss_trap \
> + test_vsyscall mov_ss_trap test_sigtrap_handler \
> syscall_arg_fault fsgsbase_restore sigaltstack
> TARGETS_C_BOTHBITS += nx_stack
> TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \
> diff --git a/tools/testing/selftests/x86/test_sigtrap_handler.c b/tools/testing/selftests/x86/test_sigtrap_handler.c
> new file mode 100644
> index 000000000000..9c5c2cf0cf88
> --- /dev/null
> +++ b/tools/testing/selftests/x86/test_sigtrap_handler.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
Curious about your use of GPL-2.0-or-later?
All the files in this directory use GPL-2.0-only or GPL-2.0.
> +/*
> + * Copyright (C) 2025 Intel Corporation
> + */
> +#define _GNU_SOURCE
> +
> +#include <err.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ucontext.h>
> +
> +#ifdef __x86_64__
> +# define REG_IP REG_RIP
> +#else
> +# define REG_IP REG_EIP
> +#endif
> +
> +static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *), int flags)
> +{
> + struct sigaction sa;
> +
> + memset(&sa, 0, sizeof(sa));
> + sa.sa_sigaction = handler;
> + sa.sa_flags = SA_SIGINFO | flags;
> + sigemptyset(&sa.sa_mask);
> +
> + if (sigaction(sig, &sa, 0))
> + err(1, "sigaction");
> +
> + return;
> +}
> +
> +static unsigned int loop_count_on_same_ip;
> +
> +static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
> +{
> + ucontext_t *ctx = (ucontext_t *)ctx_void;
> + static unsigned long last_trap_ip;
> +
> + if (last_trap_ip == ctx->uc_mcontext.gregs[REG_IP]) {
> + printf("trapped on %016lx\n", last_trap_ip);
> +
> + if (++loop_count_on_same_ip > 10) {
> + printf("trap loop detected, test failed\n");
> + exit(2);
> + }
Most of the x86 selftests use the ksft_exit_fail_msg(), ksft_print_msg()
or [RUN, FAIL, OK] style for error messages and other informational prints.
> +
> + return;
> + }
> +
> + loop_count_on_same_ip = 0;
> + last_trap_ip = ctx->uc_mcontext.gregs[REG_IP];
> + printf("trapped on %016lx\n", last_trap_ip);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + sethandler(SIGTRAP, sigtrap, 0);
> +
I would suggest a comment here to explain what the following assembly
code is supposed to do. It isn't obvious from a cursory look.
> + asm volatile(
> +#ifdef __x86_64__
> + /* Avoid clobbering the redzone */
> + "sub $128, %rsp\n\t"
> +#endif
> + "push $0x302\n\t"
> + "popf\n\t"
> + "nop\n\t"
> + "nop\n\t"
> + "push $0x202\n\t"
> + "popf\n\t"
> +#ifdef __x86_64__
> + "add $128, %rsp\n\t"
> +#endif
> + );
> +
> + printf("test passed\n");
> + return 0;
> +}
>
> base-commit: 485d11d84a2452ac16466cc7ae041c93d38929bc
Powered by blists - more mailing lists