linux-kernel - Re: [Xen-devel] [PATCH] x86: xen: size struct xen_spinlock to always fit in arch_spinlock

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] [day] [month] [year] [list]

Message-ID: <20120124002828.GA18635@andromeda.dapyr.net>
Date:	Mon, 23 Jan 2012 20:28:28 -0400
From:	Konrad Rzeszutek Wilk <konrad@...nok.org>
To:	David Vrabel <dvrabel@...tab.net>
Cc:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Jeremy Fitzhardinge <jeremy@...p.org>,
	xen-devel@...ts.xensource.com,
	David Vrabel <david.vrabel@...rix.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [Xen-devel] [PATCH] x86: xen: size struct xen_spinlock to always fit in arch_spinlock_t

On Mon, Jan 23, 2012 at 09:44:33PM +0000, David Vrabel wrote:
> On 23/01/2012 19:50, Konrad Rzeszutek Wilk wrote:
> >On Mon, Jan 23, 2012 at 07:32:25PM +0000, David Vrabel wrote:
> >>From: David Vrabel<david.vrabel@...rix.com>
> >>
> >>If NR_CPUS<  256 then arch_spinlock_t is only 16 bits wide but struct
> >>xen_spinlock is 32 bits.  When a spin lock is contended and
> >>xl->spinners is modified the two bytes immediately after the spin lock
> >>would be corrupted.
> >>
> >>This is a regression caused by 84eb950db13ca40a0572ce9957e14723500943d6
> >>(x86, ticketlock: Clean up types and accessors) which reduced the size
> >>of arch_spinlock_t.
> >>
> >>Fix this by making xl->spinners a u8 if NR_CPUS<  256.  A
> >>BUILD_BUG_ON() is also added to check the sizes of the two structures
> >>are compatible.
> >>
> >>In many cases this was not noticable as there would often be padding
> >>bytes after the lock (e.g., if any of CONFIG_GENERIC_LOCKBREAK,
> >>CONFIG_DEBUG_SPINLOCK, or CONFIG_DEBUG_LOCK_ALLOC were enabled).
> >>
> >>The bnx2 driver is affected. In struct bnx2, phy_lock and
> >>indirect_lock may have no padding after them.  Contention on phy_lock
> >>would corrupt indirect_lock making it appear locked and the driver
> >>would deadlock.
> >
> >Nice find. I think it also affected the ahci driver, and some of the USB
> >ones - at least those I saw starting to hang with:
> 
> It's possible but keep in mind that this isn't a recent regression.  I 
> think it's been around since 3.0 and maybe even earlier.

konrad@...nom:~/work/linux.mm$ git tag --contains 84eb950db13ca40a0572ce9957e14723500943d6 | grep -v rc
v3.2

So since v3.2, hmm, I should double-check those serial logs
to see if the problems I had were in 3.2 as well.
--
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/