linux-kernel - Re: [tip:x86/urgent] x86/mce: Fix the MCE poll timer logic

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:	Wed, 6 Jun 2012 11:27:18 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Chen Gong <gong.chen@...ux.intel.com>
cc:	"Luck, Tony" <tony.luck@...el.com>, "bp@...64.org" <bp@...64.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [tip:x86/urgent] x86/mce: Fix the MCE poll timer logic

On Wed, 6 Jun 2012, Chen Gong wrote:
> In fact, there still exists another potential issue:
> 
> static void __mcheck_cpu_init_timer(void)
> {
>         struct timer_list *t = &__get_cpu_var(mce_timer);
>         unsigned long iv = __this_cpu_read(mce_next_interval);
> 
>         setup_timer(t, mce_timer_fn, smp_processor_id());
> 
>         if (mce_ignore_ce)
>                 return;
> 
>         __this_cpu_write(mce_next_interval, iv);
>         if (!iv)
>                 return;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Because the 2nd patch is not merged yet, so here iv is zero when this
> function is called, which means at the beginning, the poll timers are
> not registered until some other conditions trigger *add_timer_on*.

Dammit. I dropped the 

	iv = check_interval * HZ;

line before __this_cpu_write() and nobody noticed. :(
 
>         t->expires = round_jiffies(jiffies + iv);
>         add_timer_on(t, smp_processor_id());
> }
> 
> Another potential issue is in this function two smp_processor_id()
> are called. If conext changes during this procedure (I'm not sure
> if it can hapen, besides secondary_cpu kickoff, online/offline will

No. This code is always called with preemption disabled.

> call these functions, even in virtualization envrionment, etc.).

What has virtualization to do with that ?

> So I think it will be better saving the value in the beginning of
> this function. Make sense?

No. Otherwise all the __this_cpu_read/write accesses are bogus as
well.

Thanks,

	tglx
--
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/