linux-kernel - Re: thp and memory barrier assumptions

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]

Date:	Fri, 3 Aug 2012 21:30:20 +0200
From:	Andrea Arcangeli <aarcange@...hat.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Rik van Riel <riel@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	paulmck <paulmck@...ux.vnet.ibm.com>,
	Oleg Nesterov <oleg@...hat.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Hugh Dickins <hughd@...gle.com>
Subject: Re: thp and memory barrier assumptions

On Thu, Jul 26, 2012 at 10:33:25PM +0200, Peter Zijlstra wrote:
> On Thu, 2012-07-26 at 22:31 +0200, Peter Zijlstra wrote:
> > __do_huge_pmd_anonymous_page() contains:
> > 
> >                 /*
> >                  * The spinlocking to take the lru_lock inside
> >                  * page_add_new_anon_rmap() acts as a full memory
> >                  * barrier to be sure clear_huge_page writes become
> >                  * visible after the set_pmd_at() write.
> >                  */
> >                 page_add_new_anon_rmap(page, vma, haddr);
> > 
> > 
> > page_add_new_anon_rmap() doesn't look to actually do a LOCK+UNLOCK
> > except for unevictable pages.
> > 
> > But even if it did do an unconditional LOCK+UNLOCK that doesn't make a
> > full memory barrier, see Documentation/memory-barriers.txt.
> > 
> > In particular:
> > 
> >         *A = a;
> >         LOCK
> >         UNLOCK
> >         *B = b;
> > 
> > may occur as:
> > 
> >         LOCK, STORE *B, STORE *A, UNLOCK
> > 
> 

I fixed that last year (I think Mel pointed out the bug) but I've been
so busy with other things I forgot to push that theoretical fix from
aa.git to -mm. As soon as autonuma is merged, I'll return to focus on
pushing the other pending patches in my queue that are being starved.

http://git.kernel.org/?p=linux/kernel/git/andrea/aa.git;a=commitdiff;h=d598a3f7ae4ca9d2c2a8653fbe790aab9b1a3141

Can you review it? If ok I'll submit it so it won't starve no
more. Also note the other bugfix that was in fair.c I think is only
needed with AutoNUMA applied this is why I didn't submit it
separately.

This can't affect x86 where even a locked bitop is the equivalent of a
full memory barrier.

> Also, what is that barrier() in handle_mm_fault() doing? And why doesn't
> it have a comment explaining that?

I added the docs below:

=====
>From ad51771a2c3fa697fa0267edda23b48d0b85f023 Mon Sep 17 00:00:00 2001
From: Andrea Arcangeli <aarcange@...hat.com>
Date: Fri, 3 Aug 2012 21:10:44 +0200
Subject: [PATCH] thp: document barrier() in wrprotect THP fault path

Inline doc.

Signed-off-by: Andrea Arcangeli <aarcange@...hat.com>
---
 mm/memory.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 420a449..9ec5bba 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3522,6 +3522,12 @@ retry:
 		pmd_t orig_pmd = *pmd;
 		int ret;
 
+		/*
+		 * flush orig_pmd on the stack to avoid invalidating
+		 * the pmd_trans_huge(orig_pmd) check and to allow
+		 * do_huge_pmd_wp_page to run a reliable
+		 * pmd_same(*pmd, orig_pmd).
+		 */
 		barrier();
 		if (pmd_trans_huge(orig_pmd)) {
 			if (flags & FAULT_FLAG_WRITE &&
--
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/