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: <CAMzpN2iOGKLN99MC6zgzLumysnR5q-M-jZ3y14cp5TYCW1mQWg@mail.gmail.com>
Date: Tue, 18 Mar 2025 09:53:22 -0400
From: Brian Gerst <brgerst@...il.com>
To: "Xin Li (Intel)" <xin@...or.com>
Cc: linux-kernel@...r.kernel.org, luto@...nel.org, tglx@...utronix.de, 
	mingo@...nel.org, bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, 
	hpa@...or.com, peterz@...radead.org
Subject: Re: [RESEND PATCH v2 1/3] x86/fred: Allow variable-sized event frame

On Tue, Mar 18, 2025 at 3:20 AM Xin Li (Intel) <xin@...or.com> wrote:
>
> A FRED event frame could contain different amount of information for
> different event types, or perhaps even for different instances of the
> same event type. Thus the size of an event frame pushed by a FRED CPU
> is not fixed and the address of the pt_regs structure that is used to
> save a user level context of current task is not at a fixed offset
> from top of current task kernel stack.
>
> Add a new field named 'user_pt_regs' in the thread_info structure to
> save the address of user level context pt_regs structure, thus to
> eliminate the need of any advance information of event frame size
> and allow a FRED CPU to push variable-sized event frame.
>
> For IDT user level event delivery, a pt_regs structure is pushed by
> hardware and software _always_ at a fixed offset from top of current
> task kernel stack, so simply initialize user_pt_regs to point to the
> pt_regs structure no matter whether one is pushed or not.
>
> While for FRED user level event delivery, user_pt_regs is updated with
> a pt_regs structure pointer generated in asm_fred_entrypoint_user().
>
> Suggested-by: H. Peter Anvin (Intel) <hpa@...or.com>
> Signed-off-by: Xin Li (Intel) <xin@...or.com>
> ---
>  arch/x86/entry/entry_fred.c        | 10 ++++++++++
>  arch/x86/include/asm/processor.h   | 12 ++++++------
>  arch/x86/include/asm/thread_info.h |  9 ++++++---
>  arch/x86/kernel/process.c          | 22 ++++++++++++++++++++++
>  include/linux/thread_info.h        |  1 +
>  kernel/fork.c                      |  6 ++++++
>  6 files changed, 51 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
> index f004a4dc74c2..a5f5bdd16ad8 100644
> --- a/arch/x86/entry/entry_fred.c
> +++ b/arch/x86/entry/entry_fred.c
> @@ -228,6 +228,16 @@ __visible noinstr void fred_entry_from_user(struct pt_regs *regs)
>         /* Invalidate orig_ax so that syscall_get_nr() works correctly */
>         regs->orig_ax = -1;
>
> +       /*
> +        * A FRED event frame could contain different amount of information
> +        * for different event types, or perhaps even for different instances
> +        * of the same event type. Thus the size of an event frame pushed by
> +        * a FRED CPU is not fixed and the address of the pt_regs structure
> +        * that is used to save a user level context of current task is not
> +        * at a fixed offset from top of current task stack.
> +        */
> +       current->thread_info.user_pt_regs = regs;
> +
>         switch (regs->fred_ss.type) {
>         case EVENT_TYPE_EXTINT:
>                 return fred_extint(regs);
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 5d2f7e5aff26..7ff3443eb57d 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -646,12 +646,12 @@ static __always_inline void prefetchw(const void *x)
>
>  #define task_top_of_stack(task) ((unsigned long)(task_pt_regs(task) + 1))
>
> -#define task_pt_regs(task) \
> -({                                                                     \
> -       unsigned long __ptr = (unsigned long)task_stack_page(task);     \
> -       __ptr += THREAD_SIZE - TOP_OF_KERNEL_STACK_PADDING;             \
> -       ((struct pt_regs *)__ptr) - 1;                                  \
> -})
> +/*
> + * Note, this can't be converted to an inline function as this header
> + * file defines 'struct thread_struct' which is used in the task_struct
> + * structure definition.
> + */
> +#define task_pt_regs(task) ((task)->thread_info.user_pt_regs)
>
>  #ifdef CONFIG_X86_32
>  #define INIT_THREAD  {                                                   \
> diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
> index 9282465eea21..4372f171c65f 100644
> --- a/arch/x86/include/asm/thread_info.h
> +++ b/arch/x86/include/asm/thread_info.h
> @@ -56,6 +56,7 @@
>   */
>  #ifndef __ASSEMBLER__
>  struct task_struct;
> +struct pt_regs;
>  #include <asm/cpufeature.h>
>  #include <linux/atomic.h>
>
> @@ -66,11 +67,13 @@ struct thread_info {
>  #ifdef CONFIG_SMP
>         u32                     cpu;            /* current CPU */
>  #endif
> +       struct pt_regs          *user_pt_regs;
>  };
>
> -#define INIT_THREAD_INFO(tsk)                  \
> -{                                              \
> -       .flags          = 0,                    \
> +#define INIT_THREAD_INFO(tsk)                                          \
> +{                                                                      \
> +       .flags          = 0,                                            \
> +       .user_pt_regs   = (struct pt_regs *)TOP_OF_INIT_STACK - 1,      \

Use __top_init_kernel_stack here.

>  }
>
>  #else /* !__ASSEMBLER__ */
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 91f6ff618852..58c1cd4ca60a 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -108,6 +108,28 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
>         return 0;
>  }
>
> +/*
> + * Initialize thread_info.user_pt_regs for IDT event delivery.
> + *
> + * For IDT user level event delivery, a pt_regs structure is pushed by both
> + * hardware and software and always resides at a fixed offset from top of
> + * current task kernel stack, thus thread_info.user_pt_regs is a per-task
> + * constant and NEVER changes after initialization.
> + *
> + * While for FRED user level event delivery, user_pt_regs is updated in
> + * fred_entry_from_user() immediately after user level event delivery.
> + *
> + * Note: thread_info.user_pt_regs of the init task is initialized at build
> + * time.
> + */
> +void arch_init_user_pt_regs(struct task_struct *tsk)
> +{
> +       unsigned long top_of_stack = (unsigned long)task_stack_page(tsk) + THREAD_SIZE;
> +
> +       top_of_stack -= TOP_OF_KERNEL_STACK_PADDING;
> +       tsk->thread_info.user_pt_regs = (struct pt_regs *)top_of_stack - 1;
> +}

Can this be put into arch_dup_task_struct() instead of creating another hook?


Brian Gerst

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ