Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Dave Hansen <dave.hansen@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]

On 6/23/25 05:41, Kirill A. Shutemov wrote:
> So, IIUC, that's dependency of vsyscall PF on NX. Do we want to disable
> vsyscall on boot if NX is not available?

Well, vsyscall=none can break old userspace, so forcing it on old
hardware doesn't seem like a great idea.

But, either way, this doesn't really appear to be a LASS issue. This code:

>         if (!(error_code & X86_PF_INSTR)) {
>                 /* Failed vsyscall read */
>                 if (vsyscall_mode == EMULATE)
>                         return false;

Is really asking the question:

	Is this #PF from an instruction fetch in the vsyscall page?

That _should_ be able to be done by comparing CR2 and regs->rip. In
fact, that's done just below anyway:

	WARN_ON_ONCE(address != regs->ip);

So I think we can fix this up with something like the attached patch
which just drives the if() from regs->rip and make the warning NX-only.

But this code has been like this a long time and I'm 99% sure the x86
selftests poke at all these cases. I'm curious what they do on those old
P4's (or a 64-bit VM with NX turned off), but it's not super important
either way.

[-- Attachment #2: vsyscall-nx.patch --]
[-- Type: text/x-patch, Size: 1152 bytes --]

diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
index c9103a6fa06e..0b0e0283994f 100644
--- a/arch/x86/entry/vsyscall/vsyscall_64.c
+++ b/arch/x86/entry/vsyscall/vsyscall_64.c
@@ -124,7 +124,8 @@ bool emulate_vsyscall(unsigned long error_code,
 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
 		return false;
 
-	if (!(error_code & X86_PF_INSTR)) {
+	/* Avoid emulation unless userspace was executing from vsyscall page: */
+	if (address != regs->ip) {
 		/* Failed vsyscall read */
 		if (vsyscall_mode == EMULATE)
 			return false;
@@ -136,13 +137,16 @@ bool emulate_vsyscall(unsigned long error_code,
 		return false;
 	}
 
+
+	/* X86_PF_INSTR is only set when NX is supported: */
+	if (cpu_feature_enabled(X86_FEATURE_NX))
+		WARN_ON_ONCE(!(error_code & X86_PF_INSTR));
+
 	/*
 	 * No point in checking CS -- the only way to get here is a user mode
 	 * trap to a high address, which means that we're in 64-bit user code.
 	 */
 
-	WARN_ON_ONCE(address != regs->ip);
-
 	if (vsyscall_mode == NONE) {
 		warn_bad_vsyscall(KERN_INFO, regs,
 				  "vsyscall attempted with vsyscall=none");