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

Dave Hansen
2025-06-23T15:32:57Z

[-- 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");