linux-kernel - Re: [PATCH v1 1/1] selftests/x86: Add a test to detect infinite sigtrap handler loop

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<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