linux-kernel - Re: [COUNTERPATCH] mm: avoid overflowing preempt_count() in mmu_take_all

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]

Message-ID: <20100401153205.GO5825@random.random>
Date:	Thu, 1 Apr 2010 17:32:05 +0200
From:	Andrea Arcangeli <aarcange@...hat.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Peter Zijlstra <peterz@...radead.org>, Avi Kivity <avi@...hat.com>,
	Rik van Riel <riel@...hat.com>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org,
	Kent Overstreet <kent.overstreet@...il.com>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [COUNTERPATCH] mm: avoid overflowing preempt_count() in
 mmu_take_all_locks()

On Thu, Apr 01, 2010 at 01:04:04PM +0200, Thomas Gleixner wrote:
> On Thu, 1 Apr 2010, Peter Zijlstra wrote:
> 
> > I'm sure you dropped Ingo and Thomas by accident.
> > 
> > On Thu, 2010-04-01 at 12:40 +0300, Avi Kivity wrote:
> > > mmu_take_all_locks() takes a spinlock for each vma, which means we increase
> > > the preempt count by the number of vmas in an address space.  Since the user
> > > controls the number of vmas, they can cause preempt_count to overflow.
> > > 
> > > Fix by making mmu_take_all_locks() only disable preemption once by making
> > > the spinlocks preempt-neutral.
> > 
> > Right, so while this will get rid of the warning it doesn't make the
> > code any nicer, its still a massive !preempt latency spot.
> 
> I'm not sure whether this is a real well done April 1st joke or if there
> is someone trying to secure the "bad taste patch of the month" price.
> 
> Anyway, I don't see a reason why we can't convert those locks to
> mutexes and get rid of the whole preempt disabled region.

Converting those locks to mutexes will also allow to cleanly handle
XPMEM schedule-in-mmu-notifier-handler requirement the right way.

For now getting rid of the warning is enough though. Changing the
locking would be possible but it'd slowdown the whole kernel all the
time even if nobody would ever load the kvm or gru kernel modules.

Let's be practical, this isn't even a syscall, this is only called by
device driver ioctl and it's about losing 1msec or so in latency, to
keep the whole kernel as fast as if mmu notifier didn't exist. I don't
think we should have 1 single wide lock to take in
mmu_notifier_register and then slowdown the kernel when nobody uses
mmu notifier at all. Losing 1msec when a VM starts isn't a big deal
really. If this wasn't the case it wouldn't have been merged in the
first place I think. Besides with -rt these locks aren't going to hurt
latency AFIK.
--
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/