linux-kernel - Re: 2.6.19-rc <-> ThinkPads

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, 1 Nov 2006 11:52:52 -0800 (PST)
From:	Linus Torvalds <torvalds@...l.org>
To:	Andi Kleen <ak@...e.de>
cc:	"Michael S. Tsirkin" <mst@...lanox.co.il>,
	Ernst Herzberg <earny@...4u.de>, Len Brown <lenb@...nel.org>,
	Adrian Bunk <bunk@...sta.de>, Hugh Dickins <hugh@...itas.com>,
	Pavel Machek <pavel@...e.cz>, Andrew Morton <akpm@...l.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-acpi@...r.kernel.org, linux-pm@...l.org,
	Martin Lorenz <martin@...enz.eu.org>
Subject: Re: 2.6.19-rc <-> ThinkPads

On Wed, 1 Nov 2006, Andi Kleen wrote:
> 
> Fix race in IO-APIC routing entry setup.
> 
> Interrupt could happen between setting the IO-APIC entry
> and setting its interrupt data.

This doesn't fix anything at all.

The interrupt can come in on another CPU, and if we end up having an 
affinity change due to that, we then have "set_ioapic_affinity_irq()" 
called on that other irq, and it might get to mess with the cpumask 
because we dropped the ioapic_lock.

In other words, the problem is not that interrupts were re-enabled, the 
problem is literally that the locking is _wrong_. 

It's a small window, but we simply should not release the ioapic_lock in 
between setting the routing and doing the "set_native_irq_info()" call.

So I think doing the locking inside "ioapic_write_entry()" is simply 
fundamentally wrong. When you did the cleanup, your commit message talked 
about how it might add a few more lock/unlock things:

    In a few cases the IO APIC lock is taken more often now, but this
    isn't a problem because it's all initialization/shutdown only
    slow path code.

but the point is, this is not about "performance". It's about 
_correctness_.

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