linux-kernel - Re: [PATCH 5/6] powerpc/mmu: drop mmap_sem now that locked

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]

Message-ID: <20190403164002.hued52o4mga4yprw@ca-dmjordan1.us.oracle.com>
Date:   Wed, 3 Apr 2019 12:40:02 -0400
From:   Daniel Jordan <daniel.m.jordan@...cle.com>
To:     Christophe Leroy <christophe.leroy@....fr>
Cc:     Daniel Jordan <daniel.m.jordan@...cle.com>,
        akpm@...ux-foundation.org, Davidlohr Bueso <dave@...olabs.net>,
        Alexey Kardashevskiy <aik@...abs.ru>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Paul Mackerras <paulus@...ba.org>,
        Christoph Lameter <cl@...ux.com>, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 5/6] powerpc/mmu: drop mmap_sem now that locked_vm is
 atomic

On Wed, Apr 03, 2019 at 06:58:45AM +0200, Christophe Leroy wrote:
> Le 02/04/2019 à 22:41, Daniel Jordan a écrit :
> > With locked_vm now an atomic, there is no need to take mmap_sem as
> > writer.  Delete and refactor accordingly.
> 
> Could you please detail the change ?

Ok, I'll be more specific in the next version, using some of your language in
fact.  :)

> It looks like this is not the only
> change. I'm wondering what the consequences are.
> 
> Before we did:
> - lock
> - calculate future value
> - check the future value is acceptable
> - update value if future value acceptable
> - return error if future value non acceptable
> - unlock
> 
> Now we do:
> - atomic update with future (possibly too high) value
> - check the new value is acceptable
> - atomic update back with older value if new value not acceptable and return
> error
> 
> So if a concurrent action wants to increase locked_vm with an acceptable
> step while another one has temporarily set it too high, it will now fail.
> 
> I think we should keep the previous approach and do a cmpxchg after
> validating the new value.

That's a good idea, and especially worth doing considering that an arbitrary
number of threads that charge a low amount of locked_vm can fail just because
one thread charges lots of it.

pinned_vm appears to be broken the same way, so I can fix it too unless someone
beats me to it.