linux-kernel - Re: [PATCH v2 01/17] mm/gup: Fixup p*_access

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]

Date:   Thu, 14 Dec 2017 13:41:17 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     linux-kernel@...r.kernel.org, tglx@...utronix.de
Cc:     x86@...nel.org, Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Lutomirsky <luto@...nel.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Borislav Petkov <bpetkov@...e.de>,
        Greg KH <gregkh@...uxfoundation.org>, keescook@...gle.com,
        hughd@...gle.com, Brian Gerst <brgerst@...il.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        David Laight <David.Laight@...lab.com>,
        Eduardo Valentin <eduval@...zon.com>, aliguori@...zon.com,
        Will Deacon <will.deacon@....com>, linux-mm@...ck.org,
        kirill.shutemov@...ux.intel.com, dan.j.williams@...el.com
Subject: Re: [PATCH v2 01/17] mm/gup: Fixup p*_access_permitted()

On Thu, Dec 14, 2017 at 12:27:27PM +0100, Peter Zijlstra wrote:
> The gup_*_range() functions which implement __get_user_pages_fast() do
> a p*_access_permitted() test to see if the memory is at all accessible
> (tests both _PAGE_USER|_PAGE_RW as well as architectural things like
> pkeys).
> 
> But the follow_*() functions which implement __get_user_pages() do not
> have this test. Recently, commit:
> 
>   5c9d2d5c269c ("mm: replace pte_write with pte_access_permitted in fault + gup paths")
> 
> added it to a few specific write paths, but it failed to consistently
> apply it (I've not audited anything outside of gup).
> 
> Revert the change from that patch and insert the tests in the right
> locations such that they cover all READ / WRITE accesses for all
> pte/pmd/pud levels.
> 
> In particular I care about the _PAGE_USER test, we should not ever,
> allow access to pages not marked with it, but it also makes the pkey
> accesses more consistent.

This should probably go on top. These are now all superfluous and
slightly wrong.

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 2f2f5e774902..1797368cc83a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -870,9 +870,6 @@ struct page *follow_devmap_pmd(struct vm_area_struct *vma, unsigned long addr,
 	 */
 	WARN_ONCE(flags & FOLL_COW, "mm: In follow_devmap_pmd with FOLL_COW set");
 
-	if (!pmd_access_permitted(*pmd, flags & FOLL_WRITE))
-		return NULL;
-
 	if (pmd_present(*pmd) && pmd_devmap(*pmd))
 		/* pass */;
 	else
@@ -1012,9 +1009,6 @@ struct page *follow_devmap_pud(struct vm_area_struct *vma, unsigned long addr,
 
 	assert_spin_locked(pud_lockptr(mm, pud));
 
-	if (!pud_access_permitted(*pud, flags & FOLL_WRITE))
-		return NULL;
-
 	if (pud_present(*pud) && pud_devmap(*pud))
 		/* pass */;
 	else
@@ -1386,7 +1380,7 @@ int do_huge_pmd_wp_page(struct vm_fault *vmf, pmd_t orig_pmd)
  */
 static inline bool can_follow_write_pmd(pmd_t pmd, unsigned int flags)
 {
-	return pmd_access_permitted(pmd, WRITE) ||
+	return pmd_write(pmd) ||
 	       ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pmd_dirty(pmd));
 }