linux-kernel - Re: [PATCHv4 09/12] x86/mm: Provide pmdp

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: <20171213163639.7e1fb5c4082888d2e399b310@linux-foundation.org>
Date:   Wed, 13 Dec 2017 16:36:39 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     "Kirill A. Shutemov" <kirill@...temov.name>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Michal Hocko <mhocko@...nel.org>, linux-arch@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Ingo Molnar <mingo@...nel.org>,
        "H . Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCHv4 09/12] x86/mm: Provide pmdp_establish() helper

On Thu, 14 Dec 2017 03:33:18 +0300 "Kirill A. Shutemov" <kirill@...temov.name> wrote:

> On Wed, Dec 13, 2017 at 04:09:51PM -0800, Andrew Morton wrote:
> > > @@ -181,6 +182,40 @@ static inline pmd_t native_pmdp_get_and_clear(pmd_t *pmdp)
> > >  #define native_pmdp_get_and_clear(xp) native_local_pmdp_get_and_clear(xp)
> > >  #endif
> > >  
> > > +#ifndef pmdp_establish
> > > +#define pmdp_establish pmdp_establish
> > > +static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
> > > +		unsigned long address, pmd_t *pmdp, pmd_t pmd)
> > > +{
> > > +	pmd_t old;
> > > +
> > > +	/*
> > > +	 * If pmd has present bit cleared we can get away without expensive
> > > +	 * cmpxchg64: we can update pmdp half-by-half without racing with
> > > +	 * anybody.
> > > +	 */
> > > +	if (!(pmd_val(pmd) & _PAGE_PRESENT)) {
> > > +		union split_pmd old, new, *ptr;
> > > +
> > > +		ptr = (union split_pmd *)pmdp;
> > > +
> > > +		new.pmd = pmd;
> > > +
> > > +		/* xchg acts as a barrier before setting of the high bits */
> > > +		old.pmd_low = xchg(&ptr->pmd_low, new.pmd_low);
> > > +		old.pmd_high = ptr->pmd_high;
> > > +		ptr->pmd_high = new.pmd_high;
> > > +		return old.pmd;
> > > +	}
> > > +
> > > +	{
> > > +		old = *pmdp;
> > > +	} while (cmpxchg64(&pmdp->pmd, old.pmd, pmd.pmd) != old.pmd);
> > 
> > um, what happened here?
> 
> Ouch.. Yeah, we need 'do' here. :-/
> 
> Apparently, it's a valid C code that would run the body once and it worked for
> me because I didn't hit the race condition.

So how the heck do we test this?  Add an artificial delay on the other
side to open the race window?