linux-kernel - Re: [PATCH] arch: x86: take precautions against spurious timer interrupts

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]

Message-ID: <4F96B0EB.1010601@windriver.com>
Date:	Tue, 24 Apr 2012 09:55:55 -0400
From:	Paul Gortmaker <paul.gortmaker@...driver.com>
To:	Vladimir Davydov <vdavydov@...allels.com>
CC:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@....edu>,
	Jan Beulich <jbeulich@...e.com>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arch: x86: take precautions against spurious timer interrupts

On 12-04-23 08:18 AM, Vladimir Davydov wrote:
> If hpet is enabled by hpet_late_init() - this usually occurs on systems
> with buggy BIOS, which does not report about hpet presence through ACPI,
> hpet_clockevent's event_handler can be left uninitialized by
> clockevents_register_device() because of hpet_clockevent low rating (by
> the time hpet_late_init() is called, high prio apic timers have already
> been setup). The event_handler is then initialized a bit later by the
> clocksource_done_booting() procedure.
> 
> Normally, timer interrupts should not be delivered between these two
> calls, but if e.g. the kernel is booted using kexec, there might be some
> pending interrupts from the previous kernel's context, which can lead to
> a NULL pointer dereference.

Reading between the lines here, I'm guessing that this is specific
to the kexec use case, and never seen anywhere else?  In which case,
it seems a shame to add another conditional to the main timer_interrupt
for an event that may only happen once at boot, and even then, only
in a corner use-case.  Can you deal with the invalid state somewhere
in an _init section instead, perhaps even within CONFIG_KEXEC?  Or
at least ensure a dummy no-op handler is attached early enough?

Paul.
--

> 
> So, take precautions against spurious timer interrupts by checking the
> event_handler value before calling it.
> ---
>  arch/x86/kernel/time.c |   18 +++++++++++++++++-
>  1 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c
> index c6eba2b..43bdd3a 100644
> --- a/arch/x86/kernel/time.c
> +++ b/arch/x86/kernel/time.c
> @@ -57,7 +57,23 @@ EXPORT_SYMBOL(profile_pc);
>   */
>  static irqreturn_t timer_interrupt(int irq, void *dev_id)
>  {
> -	global_clock_event->event_handler(global_clock_event);
> +	/*
> +	 * If hpet is enabled by hpet_late_init(), event_handler can be left
> +	 * uninitialized by clockevents_register_device() because of
> +	 * hpet_clockevent low rating (by the time hpet_late_init() is called,
> +	 * high prio apic timers have already been setup). The event_handler is
> +	 * then initialized a bit later by the clocksource_done_booting()
> +	 * procedure.
> +	 *
> +	 * Normally, timer interrupts should not be delivered between these two
> +	 * calls, but if e.g. the kernel is booted using kexec, there might be
> +	 * some pending interrupts from the previous kernel's context, which
> +	 * can lead to a NULL pointer dereference.
> +	 *
> +	 * So, take precautions against spurious timer interrupts.
> +	 */
> +	if (global_clock_event->event_handler)
> +		global_clock_event->event_handler(global_clock_event);
>  
>  	/* MCA bus quirk: Acknowledge irq0 by setting bit 7 in port 0x61 */
>  	if (MCA_bus)
--
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/