From: Andy Lutomirski <luto@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>,
linux-arch <linux-arch@vger.kernel.org>,
Will Deacon <will@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
Mark Rutland <mark.rutland@arm.com>,
Kees Cook <keescook@chromium.org>,
Keno Fischer <keno@juliacomputing.com>,
Paolo Bonzini <pbonzini@redhat.com>,
kvm list <kvm@vger.kernel.org>
Subject: Re: [patch V3 01/13] entry: Provide generic syscall entry functionality
Date: Mon, 27 Jul 2020 15:28:40 -0700 [thread overview]
Message-ID: <CALCETrW_CrpYnBWipDQkznRakbb2FnayiN4PLWH7Mid5-ryBYw@mail.gmail.com> (raw)
In-Reply-To: <20200716185424.011950288@linutronix.de>
On Thu, Jul 16, 2020 at 12:50 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> From: Thomas Gleixner <tglx@linutronix.de>
>
> On syscall entry certain work needs to be done:
>
> - Establish state (lockdep, context tracking, tracing)
> - Conditional work (ptrace, seccomp, audit...)
>
> This code is needlessly duplicated and different in all
> architectures.
>
> +
> +/**
> + * arch_syscall_enter_seccomp - Architecture specific seccomp invocation
> + * @regs: Pointer to currents pt_regs
> + *
> + * Returns: The original or a modified syscall number
> + *
> + * Invoked from syscall_enter_from_user_mode(). Can be replaced by
> + * architecture specific code.
> + */
> +static inline long arch_syscall_enter_seccomp(struct pt_regs *regs);
Ick. I'd rather see arch_populate_seccomp_data() and kill this hook.
But we can clean this up later.
> +/**
> + * arch_syscall_enter_audit - Architecture specific audit invocation
> + * @regs: Pointer to currents pt_regs
> + *
> + * Invoked from syscall_enter_from_user_mode(). Must be replaced by
> + * architecture specific code if the architecture supports audit.
> + */
> +static inline void arch_syscall_enter_audit(struct pt_regs *regs);
> +
Let's pass u32 arch here.
> +/**
> + * syscall_enter_from_user_mode - Check and handle work before invoking
> + * a syscall
> + * @regs: Pointer to currents pt_regs
> + * @syscall: The syscall number
> + *
> + * Invoked from architecture specific syscall entry code with interrupts
> + * disabled. The calling code has to be non-instrumentable. When the
> + * function returns all state is correct and the subsequent functions can be
> + * instrumented.
> + *
> + * Returns: The original or a modified syscall number
> + *
> + * If the returned syscall number is -1 then the syscall should be
> + * skipped. In this case the caller may invoke syscall_set_error() or
> + * syscall_set_return_value() first. If neither of those are called and -1
> + * is returned, then the syscall will fail with ENOSYS.
> + *
> + * The following functionality is handled here:
> + *
> + * 1) Establish state (lockdep, RCU (context tracking), tracing)
> + * 2) TIF flag dependent invocations of arch_syscall_enter_tracehook(),
> + * arch_syscall_enter_seccomp(), trace_sys_enter()
> + * 3) Invocation of arch_syscall_enter_audit()
> + */
> +long syscall_enter_from_user_mode(struct pt_regs *regs, long syscall);
This should IMO also take u32 arch.
I'm also uneasy about having this do the idtentry/irqentry stuff as
well as the syscall stuff. Is there a particular reason you did it
this way instead of having callers do:
idtentry_enter();
instrumentation_begin();
syscall_enter_from_user_mode();
FWIW, I think we could make this even better -- couldn't this get
folded together with syscall *exit* and become:
idtentry_enter();
instrumentation_begin();
generic_syscall();
instrumentation_end();
idtentry_exit();
and generic_syscall() would call arch_dispatch_syscall(regs, arch, syscall_nr);
> +
> +/**
> + * irqentry_enter_from_user_mode - Establish state before invoking the irq handler
> + * @regs: Pointer to currents pt_regs
> + *
> + * Invoked from architecture specific entry code with interrupts disabled.
> + * Can only be called when the interrupt entry came from user mode. The
> + * calling code must be non-instrumentable. When the function returns all
> + * state is correct and the subsequent functions can be instrumented.
> + *
> + * The function establishes state (lockdep, RCU (context tracking), tracing)
> + */
> +void irqentry_enter_from_user_mode(struct pt_regs *regs);
Unless the rest of the series works differently from what I expect, I
don't love this name. How about normal_entry_from_user_mode() or
ordinary_entry_from_user_mode()? After all, this seems to cover IRQ,
all the non-horrible exceptions, and (internally) syscalls.
--Andy
next prev parent reply other threads:[~2020-07-27 22:28 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-16 18:22 [patch V3 00/13] entry, x86, kvm: Generic entry/exit functionality for host and guest Thomas Gleixner
2020-07-16 18:22 ` [patch V3 01/13] entry: Provide generic syscall entry functionality Thomas Gleixner
2020-07-16 20:52 ` Kees Cook
2020-07-16 21:55 ` Thomas Gleixner
2020-07-17 17:49 ` Kees Cook
2020-07-17 19:29 ` Thomas Gleixner
2020-07-17 21:56 ` Andy Lutomirski
2020-07-18 14:16 ` Thomas Gleixner
2020-07-18 14:41 ` Andy Lutomirski
2020-07-19 10:17 ` Thomas Gleixner
2020-07-19 15:25 ` Andy Lutomirski
2020-07-20 6:50 ` Thomas Gleixner
2020-07-27 22:28 ` Andy Lutomirski [this message]
2020-07-16 18:22 ` [patch V3 02/13] entry: Provide generic syscall exit function Thomas Gleixner
2020-07-16 20:55 ` Kees Cook
2020-07-16 21:28 ` Thomas Gleixner
2020-07-16 18:22 ` [patch V3 03/13] entry: Provide generic interrupt entry/exit code Thomas Gleixner
2020-07-16 18:22 ` [patch V3 04/13] entry: Provide infrastructure for work before exiting to guest mode Thomas Gleixner
2020-07-16 18:22 ` [patch V3 05/13] x86/entry: Consolidate check_user_regs() Thomas Gleixner
2020-07-16 20:56 ` Kees Cook
2020-07-16 18:22 ` [patch V3 06/13] x86/entry: Consolidate 32/64 bit syscall entry Thomas Gleixner
2020-07-16 18:22 ` [patch V3 07/13] x86/ptrace: Provide pt_regs helpers for entry/exit Thomas Gleixner
2020-07-16 20:57 ` Kees Cook
2020-07-16 18:22 ` [patch V3 08/13] x86/entry: Use generic syscall entry function Thomas Gleixner
2020-07-16 21:13 ` Kees Cook
2020-07-16 21:33 ` Thomas Gleixner
2020-07-16 18:22 ` [patch V3 09/13] x86/entry: Use generic syscall exit functionality Thomas Gleixner
2020-07-16 18:22 ` [patch V3 10/13] x86/entry: Cleanup idtentry_entry/exit_user Thomas Gleixner
2020-07-16 18:22 ` [patch V3 11/13] x86/entry: Use generic interrupt entry/exit code Thomas Gleixner
2020-07-16 18:22 ` [patch V3 12/13] x86/entry: Cleanup idtentry_enter/exit Thomas Gleixner
2020-07-16 18:22 ` [patch V3 13/13] x86/kvm: Use generic exit to guest work function Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALCETrW_CrpYnBWipDQkznRakbb2FnayiN4PLWH7Mid5-ryBYw@mail.gmail.com \
--to=luto@kernel.org \
--cc=arnd@arndb.de \
--cc=keescook@chromium.org \
--cc=keno@juliacomputing.com \
--cc=kvm@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).