Re: [PATCH v6 1/3] mm: make persistent huge zero folio read-only

[David Hildenbrand (Arm)]

On 7/30/26 11:06, Xueyuan Chen wrote:
> The persistent huge zero folio is shared globally and should stay zero
> after initialization. As Jann Horn pointed out[1], kernel bugs have ended
> up writing to pages that were meant to be read-only, including in
> security-sensitive cases. Making the persistent huge zero folio read-only
> in the direct map turns such writes into faults instead of silent zero-page
> corruption.
> 
> Add set_direct_map_ro_noflush() so mm code can make a direct-map range
> read-only. Use an address-based signature to match ongoing direct-map
> helper work[2], where existing page-based helpers may move the same way.
> The helper is direct-map specific and leaves TLB invalidation to its
> caller. Architectures without direct-map permission support keep existing
> behavior through the generic stub.
> 
> The folio is allocated and zeroed through the writable direct map before
> thp_shrinker_init() changes its permissions. thp_shrinker_init() is called
> from hugepage_init(), which is registered as a subsys_initcall and runs
> after SMP initialization. Stale writable kernel TLB entries may therefore
> exist. Flush the direct-map range immediately after the page-table update
> so they cannot bypass the read-only mapping.
> 
> GFP_TRANSHUGE includes __GFP_HIGHMEM. On 32-bit systems, the persistent
> folio may therefore reside in high memory, where folio_address() returns
> NULL. Such a folio has no permanent direct-map mapping to protect, so skip
> the permission change and TLB flush.
> 
> Treat the direct-map permission change as best-effort. Architectures that
> do not implement the helper keep the existing behavior via the generic
> stub.
> 
> Inspired by Jann Horn's read-only zero page work[1] and follow-up
> discussion[3] with Yang Shi.
> 
> Link: https://lore.kernel.org/r/20260508-ro-zeropage-v1-1-9808abc20b49@xxxxxxxxxx [1]
> Link: https://lore.kernel.org/r/0e5b23a6-4895-454a-9dfa-6dc21adc2991@xxxxxxxxxx [2]
> Link: https://lore.kernel.org/r/CAHbLzkrXXe7r3n3jXgDKtwZhRqj=jDx9E6dLOULohnhBguvi9A@xxxxxxxxxxxxxx [3]
> Link: https://lore.kernel.org/r/20260727113530.5cd347581a66b3279490a604@xxxxxxxxxxxxxxxxxxxx
> Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
> Suggested-by: Usama Arif <usama.arif@xxxxxxxxx>
> Co-developed-by: Lance Yang <lance.yang@xxxxxxxxx>
> Signed-off-by: Lance Yang <lance.yang@xxxxxxxxx>
> Signed-off-by: Xueyuan Chen <xueyuan.chen21@xxxxxxxxx>
> ---
>  include/linux/set_memory.h | 29 +++++++++++++++++++++++++++++
>  mm/huge_memory.c           | 20 +++++++++++++++++++-
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/set_memory.h b/include/linux/set_memory.h
> index 3030d9245f5a..e83ced6a3827 100644
> --- a/include/linux/set_memory.h
> +++ b/include/linux/set_memory.h
> @@ -40,6 +40,24 @@ static inline int set_direct_map_valid_noflush(struct page *page,
>  	return 0;
>  }
>  
> +/**
> + * set_direct_map_ro_noflush - make a direct-map range read-only
> + * @addr: start address in the direct map
> + * @nr_pages: number of pages starting at @addr
> + *
> + * Make the direct-map range starting at @addr read-only without invalidating
> + * TLBs. Callers must either ensure that no stale writable translations can
> + * be used, or treat the permission change as a best-effort hardening step.
> + *
> + * Return: 0 on success or when direct-map permission changes are unsupported,
> + * or a negative errno on failure.
> + */
> +static inline int set_direct_map_ro_noflush(const void *addr,
> +					    unsigned long nr_pages)

Two tabs here in MM land.

> +{
> +	return 0;
> +}

Nobody checks the return value, so how helpful is it to return it? Do we expect
other users to check for the return code?

I guess we just try to keep consistency with the other functions?

[...]


> @@ -932,6 +934,8 @@ static int __init thp_shrinker_init(void)
>  	shrinker_register(deferred_split_shrinker);
>  
>  	if (IS_ENABLED(CONFIG_PERSISTENT_HUGE_ZERO_FOLIO)) {
> +		unsigned long addr;
> +
>  		/*
>  		 * Bump the reference of the huge_zero_folio and do not
>  		 * initialize the shrinker.
> @@ -940,8 +944,22 @@ static int __init thp_shrinker_init(void)
>  		 * that get_huge_zero_folio() will most likely not fail as
>  		 * thp_shrinker_init() is invoked early on during boot.
>  		 */
> -		if (!get_huge_zero_folio())
> +		if (!get_huge_zero_folio()) {
>  			pr_warn("Allocating persistent huge zero folio failed\n");
> +			return 0;
> +		}
> +
> +		/* Highmem folios have no permanent direct-map mapping to protect. */
> +		if (folio_test_highmem(huge_zero_folio))
> +			return 0;
> +
> +		addr = (unsigned long)folio_address(huge_zero_folio);
> +		/*
> +		 * The folio was zeroed through the writable direct map. Flush
> +		 * after the page-table update to invalidate stale translations.
> +		 */
> +		set_direct_map_ro_noflush((void *)addr, HPAGE_PMD_NR);
> +		flush_tlb_kernel_range(addr, addr + HPAGE_PMD_SIZE);
>  		return 0;
>  	}
>  

The code changed upstream in the meantime. There is now the huge_zero_init() we
can hook into instead.

-- 
Cheers,

David