Re: [tip: x86/urgent] x86/mm/pat: Acquire init_mm read lock on attribute change to avoid UAF

[Jiri Slaby <jirislaby@kernel.org>]

On 01. 09. 26, 0:27, tip-bot2 for Lorenzo Stoakes (ARM) wrote:
> The following commit has been merged into the x86/urgent branch of tip:
> 
> Commit-ID:     477cf5cd1f698053df7426b7b8d9339e85e00946
> Gitweb:        https://git.kernel.org/tip/477cf5cd1f698053df7426b7b8d9339e85e00946
> Author:        Lorenzo Stoakes (ARM) <ljs@kernel.org>
> AuthorDate:    Thu, 13 Aug 2026 12:01:25 +03:00
> Committer:     Dave Hansen <dave.hansen@linux.intel.com>
> CommitterDate: Mon, 31 Aug 2026 15:15:42 -07:00
> 
> x86/mm/pat: Acquire init_mm read lock on attribute change to avoid UAF
> 
> A previous commit protected against races between ptdump and CPA collapse,
> however one still exists between attribute changes and collapse as reported
> by Denis V. Lunev (linked).
> 
> When an attribute change arises, a lockless page table walker obtains a PTE
> entry, which is later written to via set_pte_atomic():
> 
> ...
> -> change_page_attr_set_clr()
> -> __change_page_attr_set_clr()
> -> __change_page_attr()
> 	-> _lookup_address_cpa()
> 	-> lookup_address_in_pgd_attr()
> 	-> [ lockless page table walker ]
> -> set_pte_atomic()
> 
> There is nothing preventing a concurrent CPA collapse which can free the
> PTE that was retrieved here, resulting in a use-after-free.
> 
> With the mmap write lock taken on init_mm over CPA collapse, resolve this
> race by acquiring an mmap read lock on init_mm over
> __change_page_attr_set_clr().
> 
> This locks across the whole operation over which the walk and the PTE entry
> write occurs, solving the race.
> 
> It is safe to do this here, as no spinlocks are held upon entry to
> __change_page_attr_set_clr().
> 
> However, the lock must not be held over an allocation, as allocation can
> trigger reclaim and shrinkers may call into CPA recursively, making
> deadlocks possible (init_mm -> ... -> fs_reclaim -> init_mm).
> 
> A page table is allocated when a huge page needs to be split:
> 
> -> change_page_attr_set_clr()
> -> __change_page_attr_set_clr()
> -> __change_page_attr()
> -> split_large_page()
> [ pagetable_alloc() ]
> -> __split_large_page()
> 
> Avoid deadlocks by dropping the mmap lock across pagetable_alloc() in
> split_large_page() and track whether this is needed by adding a new
> 'init_mm_read_locked' flag to struct cpa_data.
> 
> This is safe as __split_large_page() (called with locks re-established)
> revalidates that the page table entry is the same as it was prior to the
> locks being dropped and __change_page_attr() repeats the entire page table
> walk whenever a split occurs, so concurrent split and collapse are
> accounted for.
> 
> Concurrent ptdump is also safe as the lock is only dropped over page table
> allocation during which time the page table has not yet been modified.
> 
> The CPA_COLLAPSE flag is only set by set_memory_rox(), which exclusively
> operates upon vmalloc ranges, and on x86 only within the module mapping
> space.
> 
> This is important, because some callers directly invoke
> __change_page_attr_set_clr(), bypassing this lock. However, none of these
> operate within the module mapping space.
> 
> * cpa_process_alias() - a recursive helper called by
>    __change_page_attr_set_clr().
> * __set_memory_enc_pgtable() - operates on the direct mapping and (via
>    __vmbus_establish_gpadl()) the vmalloc mapping space.
> * __set_pages_[n]p() - called by set_direct_map_[invalid, default,
>    valid]_noflush(), __kernel_map_pages() - operates on the direct map.
> * kernel_[un]map_pages_in_pgd() - operates on EFI ranges.
> 
> This work is based upon Denis V. Lunev's excellent analysis of the bug with
> gratitude.
> 
> Fixes: 41d88484c71c ("x86/mm/pat: restore large ROX pages after fragmentation")
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Tested-by: Atish Patra <atishp@meta.com>
> Tested-by: Nikunj A Dadhania <nikunj@amd.com>
> Link: https://lore.kernel.org/all/20260626163213.2284080-1-den@openvz.org/
> Cc:stable@vger.kernel.org
> Link: https://patch.msgid.link/20260813-cpa-fixes-v2-2-39b4ff90f91d@kernel.org
> ---
>   arch/x86/mm/pat/set_memory.c | 28 +++++++++++++++++++++++++---
>   1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index c38faf3..cb5d6d6 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -22,6 +22,7 @@
>   #include <linux/cc_platform.h>
>   #include <linux/set_memory.h>
>   #include <linux/memregion.h>
> +#include <linux/cleanup.h>
>   
>   #include <asm/e820/api.h>
>   #include <asm/processor.h>
> @@ -49,7 +50,8 @@ struct cpa_data {
>   	unsigned int	flags;
>   	unsigned int	force_split		: 1,
>   			force_static_prot	: 1,
> -			force_flush_all		: 1;
> +			force_flush_all		: 1,
> +			init_mm_read_locked	: 1;
>   	struct page	**pages;
>   };
>   
> @@ -409,7 +411,7 @@ static void __cpa_flush_tlb(void *data)
>   
>   static int collapse_large_pages(unsigned long addr, struct list_head *pgtables);
>   
> -static void cpa_collapse_large_pages(struct cpa_data *cpa)
> +static void __cpa_collapse_large_pages(struct cpa_data *cpa)
>   {
>   	unsigned long start, addr, end;
>   	struct ptdesc *ptdesc, *tmp;
> @@ -443,6 +445,18 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
>   	}
>   }
>   
> +static void cpa_collapse_large_pages(struct cpa_data *cpa)
> +{
> +	/*
> +	 * Take the mmap write lock on init_mm to:
> +	 * - Avoid a use-after-free if raced by ptdump (which takes its own
> +	 *   write lock on init_mm).
> +	 * - Serialise concurrent CPA walkers.
> +	 */
> +	scoped_guard(mmap_write_lock, &init_mm)
> +		__cpa_collapse_large_pages(cpa);
> +}
> +

Ah, this belongs to the "Acquire init_mm *write* lock" counterpart. Not 
sure how you managed to split/combine these two 8-).

thanks,
-- 
js
suse labs