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] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 28 Feb 2020 11:42:11 +0100
From:   Alexandre Chartre <alexandre.chartre@...cle.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>
Cc:     x86@...nel.org, Steven Rostedt <rostedt@...dmis.org>,
        Brian Gerst <brgerst@...il.com>,
        Juergen Gross <jgross@...e.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [patch 06/24] x86/idtentry: Provide macros to define/declare IDT
 entry points


On 2/25/20 11:16 PM, Thomas Gleixner wrote:
> Provide DECLARE/DEFINE_IDTENTRY() macros.
> 
> DEFINE_IDTENTRY() provides a wrapper which acts as the function
> definition. The exception handler body is just appended to it with curly
> brackets. The entry point is marked notrace/noprobe so that irq tracing and
> the enter_from_user_mode() can be moved into the C-entry point. As all
> C-entries use the same macro (or a later variant) the necessary entry
> handling can be implemented at one central place.
> 
> DECLARE_IDTENTRY() provides the function prototypes:
>    - The C entry point 	    	cfunc
>    - The ASM entry point		asm_cfunc
>    - The XEN/PV entry point	xen_asm_cfunc
> 
> They all follow the same naming convention.
> 
> When included from ASM code DECLARE_IDTENTRY() is a macro which emits the
> low level entry point in assembly by instantiating idtentry.
> 
> IDTENTRY is the simplest variant which just has a pt_regs argument. It's
> going to be used for all exceptions which have no error code.
> 
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>   arch/x86/entry/entry_32.S       |    6 +++
>   arch/x86/entry/entry_64.S       |    6 +++
>   arch/x86/include/asm/idtentry.h |   79 ++++++++++++++++++++++++++++++++++++++++
>   arch/x86/include/asm/traps.h    |    2 -
>   4 files changed, 92 insertions(+), 1 deletion(-)
> 
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -769,6 +769,12 @@ SYM_CODE_END(\asmsym)
>   .endm
>   
>   /*
> + * Include the defines which emit the idt entries which are shared
> + * shared between 32 and 64 bit.
> + */
> +#include <asm/idtentry.h>
> +
> +/*
>    * %eax: prev task
>    * %edx: next task
>    */
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -688,6 +688,12 @@ SYM_CODE_END(\asmsym)
>   .endm
>   
>   /*
> + * Include the defines which emit the idt entries which are shared
> + * shared between 32 and 64 bit.
> + */
> +#include <asm/idtentry.h>
> +
> +/*
>    * Interrupt entry helper function.
>    *
>    * Entry runs with interrupts off. Stack layout at entry:
> --- /dev/null
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_IDTENTRY_H
> +#define _ASM_X86_IDTENTRY_H
> +
> +/* Interrupts/Exceptions */
> +#include <asm/trapnr.h>
> +
> +#ifndef __ASSEMBLY__
> +
> +/**
> + * idtentry_enter - Handle state tracking on idtentry
> + * @regs:	Pointer to pt_regs of interrupted context
> + *
> + * Place holder for now.
> + */
> +static __always_inline void idtentry_enter(struct pt_regs *regs)
> +{
> +}
> +
> +/**
> + * idtentry_exit - Prepare returning to low level ASM code
> + *
> + * Place holder for now.
> + */
> +static __always_inline void idtentry_exit(struct pt_regs *regs)
> +{
> +}
> +
> +/**
> + * DECLARE_IDTENTRY - Declare functions for simple IDT entry points
> + *		      No error code pushed by hardware
> + * @vector:	Vector number (ignored for C)
> + * @func:	Function name of the entry point
> + *
> + * Declares three functions:
> + * - The ASM entry point: asm_##func
> + * - The XEN PV trap entry point: xen_##func (maybe unused)
> + * - The C handler called from the ASM entry point
> + */
> +#define DECLARE_IDTENTRY(vector, func)					\
> +	asmlinkage void asm_##func(void);				\
> +	asmlinkage void xen_asm_##func(void);				\
> +	__visible void func(struct pt_regs *regs)
> +
> +/**
> + * DEFINE_IDTENTRY - Emit code for simple IDT entry points
> + * @func:	Function name of the entry point
> + *
> + * @func is called from ASM entry code with interrupts disabled.
> + *
> + * The macro is written so it acts as function definition. Append the
> + * body with a pair of curly brackets.
> + *
> + * idtentry_enter() contains common code which has to be invoked before
> + * arbitrary code in the body. idtentry_exit() contains common code
> + * which has to run before returning to the low level assembly code.
> + */
> +#define DEFINE_IDTENTRY(func)						\
> +static __always_inline void __##func(struct pt_regs *regs);		\
> +									\
> +__visible notrace void func(struct pt_regs *regs)			\
> +{									\
> +	idtentry_enter(regs);						\
> +	__##func (regs);						\
> +	idtentry_exit(regs);						\
> +}									\
> +NOKPROBE_SYMBOL(func);							\
> +									\
> +static __always_inline void __##func(struct pt_regs *regs)
> +
> +#else /* !__ASSEMBLY__ */
> +
> +/* Defines for ASM code to construct the IDT entries */
> +#define DECLARE_IDTENTRY(vector, func)				\
> +	idtentry vector asm_##func func has_error_code=0

Should be DEFINE_IDENTRY(), no? Like the comment says: "Defines for ..."

In that case, the commit comment also needs to be updated.

Reviewed-by: Alexandre Chartre <alexandre.chartre@...cle.com>

alex.

> +#endif /* __ASSEMBLY__ */
> +
> +#endif
> --- a/arch/x86/include/asm/traps.h
> +++ b/arch/x86/include/asm/traps.h
> @@ -6,8 +6,8 @@
>   #include <linux/kprobes.h>
>   
>   #include <asm/debugreg.h>
> +#include <asm/idtentry.h>
>   #include <asm/siginfo.h>			/* TRAP_TRACE, ... */
> -#include <asm/trapnr.h>
>   
>   #define dotraplinkage __visible
>   
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ