linux-kernel - Re: BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 5 Nov 2010 17:06:55 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Cyrill Gorcunov <gorcunov@...il.com>
Cc:	Don Zickus <dzickus@...hat.com>, Jan Kiszka <jan.kiszka@....de>,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	Eric Paris <eparis@...hat.com>,
	Randy Dunlap <randy.dunlap@...cle.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler

2010/11/1 Cyrill Gorcunov <gorcunov@...il.com>:
> On Mon, Nov 01, 2010 at 11:45:36AM -0400, Don Zickus wrote:
> ...
>> Heh.  Yeah when I migrated the code, I completely forgot the notifier
>> chain could be called from a preemptible context (ie not NMI).
>>
>> This patch should fix it and I think it is the correct fix.  Let me know
>> how it works out.
>>
>> Cheers,
>> Don
>>
>> diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
>> index c7c9ae4..1bdd0b5 100644
>> --- a/arch/x86/kernel/apic/hw_nmi.c
>> +++ b/arch/x86/kernel/apic/hw_nmi.c
>> @@ -63,7 +63,7 @@ arch_trigger_all_cpu_backtrace_handler(struct notifier_block *self,
>>  {
>>       struct die_args *args = __args;
>>       struct pt_regs *regs;
>> -     int cpu = smp_processor_id();
>> +     int cpu;
>>
>>       switch (cmd) {
>>       case DIE_NMI:
>> @@ -74,6 +74,7 @@ arch_trigger_all_cpu_backtrace_handler(struct notifier_block *self,
>>       }
>>
>>       regs = args->regs;
>> +     cpu = smp_processor_id();
>>
>>       if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
>>               static arch_spinlock_t lock = __ARCH_SPIN_LOCK_UNLOCKED;
>>
>
>  yup, this will do the trick for a while. In general I believe we might have
> kind of NMI exclusive chain so we wouldn't need the 'case:'s.


Yeah.  And seperating NMIs from the rest of the DIE notifiers would
probably improve
performance of things like PMI handling.

And I've always been confused with this "die" notifier semantic. We
are not dying when
we handle a counter overflow interrupt.
The same applies to DIE_INT3, DIE_TRAP, DIE_DEBUG, ....

But until then, as having a seperate notifier is quite a refactoring,
we should enqueue Don's fix.
Don, can you resend it with usual SOB and changelog?

Thanks.
--
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/