Message-ID: <CAMzpN2iu5NRe2PE-TYS9beJp7Ze439C9k8AWHjfcibXi_mGuug@mail.gmail.com>
Date: Thu, 13 Mar 2025 21:25:06 -0400
From: Brian Gerst <brgerst@...il.com>
To: Sohil Mehta <sohil.mehta@...el.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, 
	Ingo Molnar <mingo@...nel.org>, "H . Peter Anvin" <hpa@...or.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Borislav Petkov <bp@...en8.de>, Andy Lutomirski <luto@...nel.org>, Juergen Gross <jgross@...e.com>, 
	Boris Ostrovsky <boris.ostrovsky@...cle.com>
Subject: Re: [PATCH 2/5] x86/syscall/32: Move 32-bit syscall dispatch code

On Thu, Mar 13, 2025 at 7:45 PM Sohil Mehta <sohil.mehta@...el.com> wrote:
>
> On 3/13/2025 11:22 AM, Brian Gerst wrote:
>
> >  SYSCALL_DEFINE0(ni_syscall)
> >  {
> >       return -ENOSYS;
> > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c
> > index 8cc9950d7104..7c286e89fd04 100644
> > --- a/arch/x86/entry/syscall_32.c
> > +++ b/arch/x86/entry/syscall_32.c
> > @@ -1,10 +1,23 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -/* System call table for i386. */
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * 32-bit system call dispatch
> > + *
> > + * Copyright (c) 2015 Andrew Lutomirski
> > + *
> > + * Based on asm and ptrace code by many authors.  The code here originated
> > + * in ptrace.c and signal.c.
>
> Wondering if we can skip copying over some of these old comments? As the
> file grows, it's hard to discerne what specific code originated where.
> For example, the FRED code is all new.

I wasn't sure what the appropriate thing would be so I just copied it
over.  The git history is probably a better way to attribute this
though.

>
> > +#ifdef CONFIG_IA32_EMULATION
> > +static __always_inline bool int80_is_external(void)
> > +{
> > +     const unsigned int offs = (0x80 / 32) * 0x10;
> > +     const u32 bit = BIT(0x80 % 32);
> > +
> > +     /* The local APIC on XENPV guests is fake */
> > +     if (cpu_feature_enabled(X86_FEATURE_XENPV))
> > +             return false;
> > +
> > +     /*
> > +      * If vector 0x80 is set in the APIC ISR then this is an external
> > +      * interrupt. Either from broken hardware or injected by a VMM.
> > +      *
> > +      * Note: In guest mode this is only valid for secure guests where
> > +      * the secure module fully controls the vAPIC exposed to the guest.
> > +      */
> > +     return apic_read(APIC_ISR + offs) & bit;
> > +}
> > +
> > +/**
> > + * do_int80_emulation - 32-bit legacy syscall C entry from asm
> > + * @regs: syscall arguments in struct pt_args on the stack.
> > + *
> > + * This entry point can be used by 32-bit and 64-bit programs to perform
> > + * 32-bit system calls.  Instances of INT $0x80 can be found inline in
> > + * various programs and libraries.  It is also used by the vDSO's
> > + * __kernel_vsyscall fallback for hardware that doesn't support a faster
> > + * entry method.  Restarted 32-bit system calls also fall back to INT
> > + * $0x80 regardless of what instruction was originally used to do the
> > + * system call.
> > + *
> > + * This is considered a slow path.  It is not used by most libc
> > + * implementations on modern hardware except during process startup.
> > + *
> > + * The arguments for the INT $0x80 based syscall are on stack in the
> > + * pt_regs structure:
> > + *   eax:                            system call number
> > + *   ebx, ecx, edx, esi, edi, ebp:   arg1 - arg 6
> > + */
> > +__visible noinstr void do_int80_emulation(struct pt_regs *regs)
> > +{
> > +     int nr;
> > +
> > +     /* Kernel does not use INT $0x80! */
> > +     if (unlikely(!user_mode(regs))) {
> > +             irqentry_enter(regs);
> > +             instrumentation_begin();
> > +             panic("Unexpected external interrupt 0x80\n");
> > +     }
> > +
> > +     /*
> > +      * Establish kernel context for instrumentation, including for
> > +      * int80_is_external() below which calls into the APIC driver.
> > +      * Identical for soft and external interrupts.
> > +      */
> > +     enter_from_user_mode(regs);
> > +
> > +     instrumentation_begin();
> > +     add_random_kstack_offset();
> > +
> > +     /* Validate that this is a soft interrupt to the extent possible */
> > +     if (unlikely(int80_is_external()))
> > +             panic("Unexpected external interrupt 0x80\n");
> > +
> > +     /*
> > +      * The low level idtentry code pushed -1 into regs::orig_ax
> > +      * and regs::ax contains the syscall number.
> > +      *
> > +      * User tracing code (ptrace or signal handlers) might assume
> > +      * that the regs::orig_ax contains a 32-bit number on invoking
> > +      * a 32-bit syscall.
> > +      *
> > +      * Establish the syscall convention by saving the 32bit truncated
> > +      * syscall number in regs::orig_ax and by invalidating regs::ax.
> > +      */
> > +     regs->orig_ax = regs->ax & GENMASK(31, 0);
> > +     regs->ax = -ENOSYS;
> > +
> > +     nr = syscall_32_enter(regs);
> > +
> > +     local_irq_enable();
> > +     nr = syscall_enter_from_user_mode_work(regs, nr);
> > +     do_syscall_32_irqs_on(regs, nr);
> > +
> > +     instrumentation_end();
> > +     syscall_exit_to_user_mode(regs);
> > +}
> > +
> > +#ifdef CONFIG_X86_FRED
> > +/*
> > + * A FRED-specific INT80 handler is warranted for the follwing reasons:
> > + *
> > + * 1) As INT instructions and hardware interrupts are separate event
> > + *    types, FRED does not preclude the use of vector 0x80 for external
> > + *    interrupts. As a result, the FRED setup code does not reserve
> > + *    vector 0x80 and calling int80_is_external() is not merely
> > + *    suboptimal but actively incorrect: it could cause a system call
> > + *    to be incorrectly ignored.
> > + *
> > + * 2) It is called only for handling vector 0x80 of event type
> > + *    EVENT_TYPE_SWINT and will never be called to handle any external
> > + *    interrupt (event type EVENT_TYPE_EXTINT).
> > + *
> > + * 3) FRED has separate entry flows depending on if the event came from
> > + *    user space or kernel space, and because the kernel does not use
> > + *    INT insns, the FRED kernel entry handler fred_entry_from_kernel()
> > + *    falls through to fred_bad_type() if the event type is
> > + *    EVENT_TYPE_SWINT, i.e., INT insns. So if the kernel is handling
> > + *    an INT insn, it can only be from a user level.
> > + *
> > + * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY. While FRED will
> > + *    likely take a different approach if it is ever needed: it
> > + *    probably belongs in either fred_intx()/ fred_other() or
> > + *    asm_fred_entrypoint_user(), depending on if this ought to be done
> > + *    for all entries from userspace or only system
> > + *    calls.
> > + *
> > + * 5) INT $0x80 is the fast path for 32-bit system calls under FRED.
> > + */
> > +DEFINE_FREDENTRY_RAW(int80_emulation)
> > +{
> > +     int nr;
> > +
> > +     enter_from_user_mode(regs);
> > +
> > +     instrumentation_begin();
> > +     add_random_kstack_offset();
> > +
> > +     /*
> > +      * FRED pushed 0 into regs::orig_ax and regs::ax contains the
> > +      * syscall number.
> > +      *
> > +      * User tracing code (ptrace or signal handlers) might assume
> > +      * that the regs::orig_ax contains a 32-bit number on invoking
> > +      * a 32-bit syscall.
> > +      *
> > +      * Establish the syscall convention by saving the 32bit truncated
> > +      * syscall number in regs::orig_ax and by invalidating regs::ax.
> > +      */
> > +     regs->orig_ax = regs->ax & GENMASK(31, 0);
> > +     regs->ax = -ENOSYS;
> > +
> > +     nr = syscall_32_enter(regs);
> > +
> > +     local_irq_enable();
> > +     nr = syscall_enter_from_user_mode_work(regs, nr);
> > +     do_syscall_32_irqs_on(regs, nr);
> > +
> > +     instrumentation_end();
> > +     syscall_exit_to_user_mode(regs);
> > +}
> > +#endif
>
> Nit: Would it be useful to add /* CONFIG_X86_FRED */ here since there
> are nested #ifdefs?

These patches should be as close to a copy and paste as possible, so
that it's easy to prove that nothing changes.  This could be a
followup patch later though.


Brian Gerst

Powered by blists - more mailing lists