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
| ||
|
Date: Wed, 11 Feb 2015 18:24:34 +0100 From: Oleg Nesterov <oleg@...hat.com> To: Jeremy Fitzhardinge <jeremy@...p.org> Cc: Raghavendra K T <raghavendra.kt@...ux.vnet.ibm.com>, Linus Torvalds <torvalds@...ux-foundation.org>, Sasha Levin <sasha.levin@...cle.com>, Davidlohr Bueso <dave@...olabs.net>, Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Peter Anvin <hpa@...or.com>, Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>, Paolo Bonzini <pbonzini@...hat.com>, Paul McKenney <paulmck@...ux.vnet.ibm.com>, Waiman Long <waiman.long@...com>, Dave Jones <davej@...hat.com>, the arch/x86 maintainers <x86@...nel.org>, Paul Gortmaker <paul.gortmaker@...driver.com>, Andi Kleen <ak@...ux.intel.com>, Jason Wang <jasowang@...hat.com>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, KVM list <kvm@...r.kernel.org>, virtualization <virtualization@...ts.linux-foundation.org>, xen-devel@...ts.xenproject.org, Rik van Riel <riel@...hat.com>, Christian Borntraeger <borntraeger@...ibm.com>, Andrew Morton <akpm@...ux-foundation.org>, Andrey Ryabinin <a.ryabinin@...sung.com> Subject: Re: [PATCH] x86 spinlock: Fix memory corruption on completing completions On 02/10, Jeremy Fitzhardinge wrote: > > On 02/10/2015 05:26 AM, Oleg Nesterov wrote: > > On 02/10, Raghavendra K T wrote: > >> Unfortunately xadd could result in head overflow as tail is high. > >> > >> The other option was repeated cmpxchg which is bad I believe. > >> Any suggestions? > > Stupid question... what if we simply move SLOWPATH from .tail to .head? > > In this case arch_spin_unlock() could do xadd(tickets.head) and check > > the result > > Well, right now, "tail" is manipulated by locked instructions by CPUs > who are contending for the ticketlock, but head can be manipulated > unlocked by the CPU which currently owns the ticketlock. If SLOWPATH > moved into head, then non-owner CPUs would be touching head, requiring > everyone to use locked instructions on it. > > That's the theory, but I don't see much (any?) code which depends on that. > > Ideally we could find a way so that pv ticketlocks could use a plain > unlocked add for the unlock like the non-pv case, but I just don't see a > way to do it. I agree, and I have to admit I am not sure I fully understand why unlock uses the locked add. Except we need a barrier to avoid the race with the enter_slowpath() users, of course. Perhaps this is the only reason? Anyway, I suggested this to avoid the overflow if we use xadd(), and I guess we need the locked insn anyway if we want to eliminate the unsafe read-after-unlock... > > BTW. If we move "clear slowpath" into "lock" path, then probably trylock > > should be changed too? Something like below, we just need to clear SLOWPATH > > before cmpxchg. > > How important / widely used is trylock these days? I am not saying this is that important. Just this looks more consistent imo and we can do this for free. Oleg. -- 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/
Powered by blists - more mailing lists