linux-kernel - RE: [PATCH v2] mm/gup: fix FOLL_FORCE COW security issue and remove FOLL

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:   Wed, 10 Aug 2022 09:12:57 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'David Hildenbrand' <david@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Nadav Amit <nadav.amit@...il.com>,
        Peter Xu <peterx@...hat.com>, Hugh Dickins <hughd@...gle.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Matthew Wilcox <willy@...radead.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        John Hubbard <jhubbard@...dia.com>,
        Jason Gunthorpe <jgg@...dia.com>
Subject: RE: [PATCH v2] mm/gup: fix FOLL_FORCE COW security issue and remove
 FOLL_COW

From: David Hildenbrand
> Sent: 09 August 2022 21:57
...

These two functions seem to contain a lot of the same tests.
They also seem a bit large for 'inline'.

> -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
> +/* FOLL_FORCE can write to even unwritable PTEs in COW mappings. */
> +static inline bool can_follow_write_pte(pte_t pte, struct page *page,
> +					struct vm_area_struct *vma,
> +					unsigned int flags)
>  {
> -	return pte_write(pte) ||
> -		((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
> +	/* If the pte is writable, we can write to the page. */
> +	if (pte_write(pte))
> +		return true;
> +
> +	/* Maybe FOLL_FORCE is set to override it? */
> +	if (!(flags & FOLL_FORCE))
> +		return false;
> +
> +	/* But FOLL_FORCE has no effect on shared mappings */
> +	if (vma->vm_flags & (VM_MAYSHARE | VM_SHARED))
> +		return false;
> +
> +	/* ... or read-only private ones */
> +	if (!(vma->vm_flags & VM_MAYWRITE))
> +		return false;
> +
> +	/* ... or already writable ones that just need to take a write fault */
> +	if (vma->vm_flags & VM_WRITE)
> +		return false;
> +
> +	/*
> +	 * See can_change_pte_writable(): we broke COW and could map the page
> +	 * writable if we have an exclusive anonymous page ...
> +	 */
> +	if (!page || !PageAnon(page) || !PageAnonExclusive(page))
> +		return false;
> +
> +	/* ... and a write-fault isn't required for other reasons. */
> +	if (vma_soft_dirty_enabled(vma) && !pte_soft_dirty(pte))
> +		return false;
> +	return !userfaultfd_pte_wp(vma, pte);
>  }
...
> -static inline bool can_follow_write_pmd(pmd_t pmd, unsigned int flags)
> +/* FOLL_FORCE can write to even unwritable PMDs in COW mappings. */
> +static inline bool can_follow_write_pmd(pmd_t pmd, struct page *page,
> +					struct vm_area_struct *vma,
> +					unsigned int flags)
>  {
> -	return pmd_write(pmd) ||
> -	       ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pmd_dirty(pmd));
> +	/* If the pmd is writable, we can write to the page. */
> +	if (pmd_write(pmd))
> +		return true;
> +
> +	/* Maybe FOLL_FORCE is set to override it? */
> +	if (!(flags & FOLL_FORCE))
> +		return false;
> +
> +	/* But FOLL_FORCE has no effect on shared mappings */
> +	if (vma->vm_flags & (VM_MAYSHARE | VM_SHARED))
> +		return false;
> +
> +	/* ... or read-only private ones */
> +	if (!(vma->vm_flags & VM_MAYWRITE))
> +		return false;
> +
> +	/* ... or already writable ones that just need to take a write fault */
> +	if (vma->vm_flags & VM_WRITE)
> +		return false;
> +
> +	/*
> +	 * See can_change_pte_writable(): we broke COW and could map the page
> +	 * writable if we have an exclusive anonymous page ...
> +	 */
> +	if (!page || !PageAnon(page) || !PageAnonExclusive(page))
> +		return false;
> +
> +	/* ... and a write-fault isn't required for other reasons. */
> +	if (vma_soft_dirty_enabled(vma) && !pmd_soft_dirty(pmd))
> +		return false;
> +	return !userfaultfd_huge_pmd_wp(vma, pmd);
>  }

Perhaps only the initial call (common success path?) should
be inlined?
With the flags and vma tests being moved to an inline helper.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)