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:	Tue, 3 May 2011 09:18:20 -0400
From:	Don Zickus <dzickus@...hat.com>
To:	Vaibhav Nagarnaik <vnagarnaik@...gle.com>
Cc:	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	Michael Rubin <mrubin@...gle.com>,
	David Sharp <dhsharp@...gle.com>, linux-kernel@...r.kernel.org,
	x86@...nel.org, Aditya Kali <adityakali@...gle.com>
Subject: Re: [PATCH 1/2] x86: Change trap definitions to enumerated values

On Fri, Apr 29, 2011 at 05:52:52PM -0700, Vaibhav Nagarnaik wrote:
> From: Aditya Kali <adityakali@...gle.com>
> 
> The traps are referred to by their numbers and it can be difficult to
> understand them while reading the code without context. This patch adds
> enumeration of the trap numbers and replaced the numbers to the correct
> enum for x86.

My only problem with this patch is you took a straight forward conversion
which didn't really change the functionality and sprinkled some trace
points in there.

I am ok with the conversion to something readable, but I don't know enough
about the trace point to know if those are correct spots.

Cheers,
Don

> 
> Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@...gle.com>
> ---
>  arch/x86/include/asm/traps.h |   24 ++++++++
>  arch/x86/kernel/traps.c      |  121 +++++++++++++++++++++++-------------------
>  2 files changed, 91 insertions(+), 54 deletions(-)
> 
> diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
> index 0310da6..c561caf 100644
> --- a/arch/x86/include/asm/traps.h
> +++ b/arch/x86/include/asm/traps.h
> @@ -87,4 +87,28 @@ asmlinkage void smp_thermal_interrupt(void);
>  asmlinkage void mce_threshold_interrupt(void);
>  #endif
>  
> +/* Interrupts/Exceptions */
> +enum {
> +	INTR_DIV_BY_ZERO = 0,	/*  0 */
> +	INTR_DEBUG,		/*  1 */
> +	INTR_NMI,		/*  2 */
> +	INTR_BREAKPOINT,	/*  3 */
> +	INTR_OVERFLOW,		/*  4 */
> +	INTR_BOUNDS_CHECK,	/*  5 */
> +	INTR_INVALID_OP,	/*  6 */
> +	INTR_NO_DEV,		/*  7 */
> +	INTR_DBL_FAULT,		/*  8 */
> +	INTR_SEG_OVERRUN,	/*  9 */
> +	INTR_INVALID_TSS,	/* 10 */
> +	INTR_NO_SEG,		/* 11 */
> +	INTR_STACK_FAULT,	/* 12 */
> +	INTR_GPF,		/* 13 */
> +	INTR_PAGE_FAULT,	/* 14 */
> +	INTR_SPURIOUS,		/* 15 */
> +	INTR_COPROCESSOR,	/* 16 */
> +	INTR_ALIGNMENT,		/* 17 */
> +	INTR_MCE,		/* 18 */
> +	INTR_SIMD_COPROCESSOR	/* 19 */
> +};
> +
>  #endif /* _ASM_X86_TRAPS_H */
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index fbfc162..e058e8b 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -129,7 +129,7 @@ do_trap(int trapnr, int signr, char *str, struct pt_regs *regs,
>  		 * traps 0, 1, 3, 4, and 5 should be forwarded to vm86.
>  		 * On nmi (interrupt 2), do_trap should not be called.
>  		 */
> -		if (trapnr < 6)
> +		if (trapnr < INTR_INVALID_OP)
>  			goto vm86_trap;
>  		goto trap_signal;
>  	}
> @@ -213,27 +213,29 @@ dotraplinkage void do_##name(struct pt_regs *regs, long error_code)	\
>  	do_trap(trapnr, signr, str, regs, error_code, &info);		\
>  }
>  
> -DO_ERROR_INFO(0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->ip)
> -DO_ERROR(4, SIGSEGV, "overflow", overflow)
> -DO_ERROR(5, SIGSEGV, "bounds", bounds)
> -DO_ERROR_INFO(6, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN, regs->ip)
> -DO_ERROR(9, SIGFPE, "coprocessor segment overrun", coprocessor_segment_overrun)
> -DO_ERROR(10, SIGSEGV, "invalid TSS", invalid_TSS)
> -DO_ERROR(11, SIGBUS, "segment not present", segment_not_present)
> +DO_ERROR_INFO(INTR_DIV_BY_ZERO, SIGFPE, "divide error",    divide_error,    FPE_INTDIV, regs->ip)
> +DO_ERROR_INFO(INTR_ALIGNMENT,   SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
> +DO_ERROR_INFO(INTR_INVALID_OP,  SIGILL, "invalid opcode",  invalid_op,      ILL_ILLOPN, regs->ip)
> +
> +DO_ERROR(INTR_OVERFLOW,     SIGSEGV, "overflow",                    overflow)
> +DO_ERROR(INTR_BOUNDS_CHECK, SIGSEGV, "bounds",                      bounds)
> +DO_ERROR(INTR_SEG_OVERRUN,  SIGFPE,  "coprocessor segment overrun", coprocessor_segment_overrun)
> +DO_ERROR(INTR_INVALID_TSS,  SIGSEGV, "invalid TSS",                 invalid_TSS)
> +DO_ERROR(INTR_NO_SEG,       SIGBUS,  "segment not present",         segment_not_present)
>  #ifdef CONFIG_X86_32
> -DO_ERROR(12, SIGBUS, "stack segment", stack_segment)
> +DO_ERROR(INTR_STACK_FAULT,  SIGBUS,  "stack segment",               stack_segment)
>  #endif
> -DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
>  
>  #ifdef CONFIG_X86_64
>  /* Runs on IST stack */
>  dotraplinkage void do_stack_segment(struct pt_regs *regs, long error_code)
>  {
>  	if (notify_die(DIE_TRAP, "stack segment", regs, error_code,
> -			12, SIGBUS) == NOTIFY_STOP)
> +			INTR_STACK_FAULT, SIGBUS) == NOTIFY_STOP)
>  		return;
>  	preempt_conditional_sti(regs);
> -	do_trap(12, SIGBUS, "stack segment", regs, error_code, NULL);
> +	do_trap(INTR_STACK_FAULT, SIGBUS, "stack segment", regs, error_code,
> +			NULL);
>  	preempt_conditional_cli(regs);
>  }
>  
> @@ -243,10 +245,10 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)
>  	struct task_struct *tsk = current;
>  
>  	/* Return not checked because double check cannot be ignored */
> -	notify_die(DIE_TRAP, str, regs, error_code, 8, SIGSEGV);
> +	notify_die(DIE_TRAP, str, regs, error_code, INTR_DBL_FAULT, SIGSEGV);
>  
>  	tsk->thread.error_code = error_code;
> -	tsk->thread.trap_no = 8;
> +	tsk->thread.trap_no = INTR_DBL_FAULT;
>  
>  	/*
>  	 * This is always a kernel trap and never fixable (and thus must
> @@ -274,7 +276,7 @@ do_general_protection(struct pt_regs *regs, long error_code)
>  		goto gp_in_kernel;
>  
>  	tsk->thread.error_code = error_code;
> -	tsk->thread.trap_no = 13;
> +	tsk->thread.trap_no = INTR_GPF;
>  
>  	if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
>  			printk_ratelimit()) {
> @@ -301,9 +303,9 @@ gp_in_kernel:
>  		return;
>  
>  	tsk->thread.error_code = error_code;
> -	tsk->thread.trap_no = 13;
> +	tsk->thread.trap_no = INTR_GPF;
>  	if (notify_die(DIE_GPF, "general protection fault", regs,
> -				error_code, 13, SIGSEGV) == NOTIFY_STOP)
> +				error_code, INTR_GPF, SIGSEGV) == NOTIFY_STOP)
>  		return;
>  	die("general protection fault", regs, error_code);
>  }
> @@ -372,7 +374,8 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
>  static notrace __kprobes void
>  unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
>  {
> -	if (notify_die(DIE_NMIUNKNOWN, "nmi", regs, reason, 2, SIGINT) ==
> +	if (notify_die(DIE_NMIUNKNOWN, "nmi", regs, reason, INTR_NMI,
> +				SIGINT) ==
>  			NOTIFY_STOP)
>  		return;
>  #ifdef CONFIG_MCA
> @@ -404,7 +407,8 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
>  	 * NMI, otherwise we may lose it, because the CPU-specific
>  	 * NMI can not be detected/processed on other CPUs.
>  	 */
> -	if (notify_die(DIE_NMI, "nmi", regs, 0, 2, SIGINT) == NOTIFY_STOP)
> +	if (notify_die(DIE_NMI, "nmi", regs, 0, INTR_NMI, SIGINT) ==
> +			NOTIFY_STOP)
>  		return;
>  
>  	/* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
> @@ -460,22 +464,24 @@ void restart_nmi(void)
>  dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
>  {
>  #ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
> -	if (kgdb_ll_trap(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
> -			== NOTIFY_STOP)
> +	if (kgdb_ll_trap(DIE_INT3, "int3", regs, error_code, INTR_BREAKPOINT,
> +			SIGTRAP) == NOTIFY_STOP)
>  		return;
>  #endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
>  #ifdef CONFIG_KPROBES
> -	if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
> +	if (notify_die(DIE_INT3, "int3", regs, error_code, INTR_BREAKPOINT,
> +				SIGTRAP)
>  			== NOTIFY_STOP)
>  		return;
>  #else
> -	if (notify_die(DIE_TRAP, "int3", regs, error_code, 3, SIGTRAP)
> +	if (notify_die(DIE_TRAP, "int3", regs, error_code, INTR_BREAKPOINT,
> +				SIGTRAP)
>  			== NOTIFY_STOP)
>  		return;
>  #endif
>  
>  	preempt_conditional_sti(regs);
> -	do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
> +	do_trap(INTR_BREAKPOINT, SIGTRAP, "int3", regs, error_code, NULL);
>  	preempt_conditional_cli(regs);
>  }
>  
> @@ -574,7 +580,7 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
>  
>  	if (regs->flags & X86_VM_MASK) {
>  		handle_vm86_trap((struct kernel_vm86_regs *) regs,
> -				error_code, 1);
> +				error_code, INTR_DEBUG);
>  		preempt_conditional_cli(regs);
>  		return;
>  	}
> @@ -602,14 +608,15 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
>  /*
>   * Note that we play around with the 'TS' bit in an attempt to get
>   * the correct behaviour even in the presence of the asynchronous
> - * IRQ13 behaviour
> + * INTR_GPF behaviour
>   */
>  void math_error(struct pt_regs *regs, int error_code, int trapnr)
>  {
>  	struct task_struct *task = current;
>  	siginfo_t info;
>  	unsigned short err;
> -	char *str = (trapnr == 16) ? "fpu exception" : "simd exception";
> +	char *str = (trapnr == INTR_COPROCESSOR) ? "fpu exception" :
> +						"simd exception";
>  
>  	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, SIGFPE) == NOTIFY_STOP)
>  		return;
> @@ -634,7 +641,7 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
>  	info.si_signo = SIGFPE;
>  	info.si_errno = 0;
>  	info.si_addr = (void __user *)regs->ip;
> -	if (trapnr == 16) {
> +	if (trapnr == INTR_COPROCESSOR) {
>  		unsigned short cwd, swd;
>  		/*
>  		 * (~cwd & swd) will mask out exceptions that are not set to unmasked
> @@ -678,8 +685,9 @@ void math_error(struct pt_regs *regs, int error_code, int trapnr)
>  		info.si_code = FPE_FLTRES;
>  	} else {
>  		/*
> -		 * If we're using IRQ 13, or supposedly even some trap 16
> -		 * implementations, it's possible we get a spurious trap...
> +		 * If we're using INTR_GPF, or supposedly even some trap
> +		 * INTR_COPROCESSOR implementations, it's possible we get a
> +		 * spurious trap...
>  		 */
>  		return;		/* Spurious trap, no error */
>  	}
> @@ -692,23 +700,25 @@ dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code)
>  	ignore_fpu_irq = 1;
>  #endif
>  
> -	math_error(regs, error_code, 16);
> +	math_error(regs, error_code, INTR_COPROCESSOR);
>  }
>  
>  dotraplinkage void
>  do_simd_coprocessor_error(struct pt_regs *regs, long error_code)
>  {
> -	math_error(regs, error_code, 19);
> +	math_error(regs, error_code, INTR_SIMD_COPROCESSOR);
>  }
>  
>  dotraplinkage void
>  do_spurious_interrupt_bug(struct pt_regs *regs, long error_code)
>  {
> +	trace_trap_entry(INTR_SPURIOUS);
>  	conditional_sti(regs);
>  #if 0
>  	/* No need to warn about this any longer. */
>  	printk(KERN_INFO "Ignoring P6 Local APIC Spurious Interrupt Bug...\n");
>  #endif
> +	trace_trap_exit(INTR_SPURIOUS);
>  }
>  
>  asmlinkage void __attribute__((weak)) smp_thermal_interrupt(void)
> @@ -745,7 +755,7 @@ void __math_state_restore(void)
>   * 'math_state_restore()' saves the current math information in the
>   * old math state array, and gets the new ones from the current task
>   *
> - * Careful.. There are problems with IBM-designed IRQ13 behaviour.
> + * Careful.. There are problems with IBM-designed INTR_GPF behaviour.
>   * Don't touch unless you *really* know how it works.
>   *
>   * Must be called with kernel preemption disabled (in this case,
> @@ -780,6 +790,7 @@ EXPORT_SYMBOL_GPL(math_state_restore);
>  dotraplinkage void __kprobes
>  do_device_not_available(struct pt_regs *regs, long error_code)
>  {
> +	trace_trap_entry(INTR_NO_DEV);
>  #ifdef CONFIG_MATH_EMULATION
>  	if (read_cr0() & X86_CR0_EM) {
>  		struct math_emu_info info = { };
> @@ -788,6 +799,7 @@ do_device_not_available(struct pt_regs *regs, long error_code)
>  
>  		info.regs = regs;
>  		math_emulate(&info);
> +		trace_trap_exit(INTR_NO_DEV);
>  		return;
>  	}
>  #endif
> @@ -795,6 +807,7 @@ do_device_not_available(struct pt_regs *regs, long error_code)
>  #ifdef CONFIG_X86_32
>  	conditional_sti(regs);
>  #endif
> +	trace_trap_exit(INTR_NO_DEV);
>  }
>  
>  #ifdef CONFIG_X86_32
> @@ -817,10 +830,10 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code)
>  /* Set of traps needed for early debugging. */
>  void __init early_trap_init(void)
>  {
> -	set_intr_gate_ist(1, &debug, DEBUG_STACK);
> +	set_intr_gate_ist(INTR_DEBUG, &debug, DEBUG_STACK);
>  	/* int3 can be called from all */
> -	set_system_intr_gate_ist(3, &int3, DEBUG_STACK);
> -	set_intr_gate(14, &page_fault);
> +	set_system_intr_gate_ist(INTR_BREAKPOINT, &int3, DEBUG_STACK);
> +	set_intr_gate(INTR_PAGE_FAULT, &page_fault);
>  	load_idt(&idt_descr);
>  }
>  
> @@ -836,30 +849,30 @@ void __init trap_init(void)
>  	early_iounmap(p, 4);
>  #endif
>  
> -	set_intr_gate(0, &divide_error);
> -	set_intr_gate_ist(2, &nmi, NMI_STACK);
> +	set_intr_gate(INTR_DIV_BY_ZERO,     &divide_error);
> +	set_intr_gate_ist(INTR_NMI,         &nmi, NMI_STACK);
>  	/* int4 can be called from all */
> -	set_system_intr_gate(4, &overflow);
> -	set_intr_gate(5, &bounds);
> -	set_intr_gate(6, &invalid_op);
> -	set_intr_gate(7, &device_not_available);
> +	set_system_intr_gate(INTR_OVERFLOW, &overflow);
> +	set_intr_gate(INTR_BOUNDS_CHECK,    &bounds);
> +	set_intr_gate(INTR_INVALID_OP,      &invalid_op);
> +	set_intr_gate(INTR_NO_DEV,          &device_not_available);
>  #ifdef CONFIG_X86_32
> -	set_task_gate(8, GDT_ENTRY_DOUBLEFAULT_TSS);
> +	set_task_gate(INTR_DBL_FAULT,       GDT_ENTRY_DOUBLEFAULT_TSS);
>  #else
> -	set_intr_gate_ist(8, &double_fault, DOUBLEFAULT_STACK);
> +	set_intr_gate_ist(INTR_DBL_FAULT,   &double_fault, DOUBLEFAULT_STACK);
>  #endif
> -	set_intr_gate(9, &coprocessor_segment_overrun);
> -	set_intr_gate(10, &invalid_TSS);
> -	set_intr_gate(11, &segment_not_present);
> -	set_intr_gate_ist(12, &stack_segment, STACKFAULT_STACK);
> -	set_intr_gate(13, &general_protection);
> -	set_intr_gate(15, &spurious_interrupt_bug);
> -	set_intr_gate(16, &coprocessor_error);
> -	set_intr_gate(17, &alignment_check);
> +	set_intr_gate(INTR_SEG_OVERRUN,     &coprocessor_segment_overrun);
> +	set_intr_gate(INTR_INVALID_TSS,     &invalid_TSS);
> +	set_intr_gate(INTR_NO_SEG,          &segment_not_present);
> +	set_intr_gate_ist(INTR_STACK_FAULT, &stack_segment, STACKFAULT_STACK);
> +	set_intr_gate(INTR_GPF,             &general_protection);
> +	set_intr_gate(INTR_SPURIOUS,        &spurious_interrupt_bug);
> +	set_intr_gate(INTR_COPROCESSOR,     &coprocessor_error);
> +	set_intr_gate(INTR_ALIGNMENT,       &alignment_check);
>  #ifdef CONFIG_X86_MCE
> -	set_intr_gate_ist(18, &machine_check, MCE_STACK);
> +	set_intr_gate_ist(INTR_MCE,         &machine_check, MCE_STACK);
>  #endif
> -	set_intr_gate(19, &simd_coprocessor_error);
> +	set_intr_gate(INTR_SIMD_COPROCESSOR, &simd_coprocessor_error);
>  
>  	/* Reserve all the builtin and the syscall vector: */
>  	for (i = 0; i < FIRST_EXTERNAL_VECTOR; i++)
> -- 
> 1.7.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ