Re: [PATCH v1] riscv: mm: Do not make execute-only PTEs readable in pte_wrprotect()

[Jisheng Zhang <jszhang@kernel.org>]

On Sat, Oct 17, 2026 at 12:22:28PM -0400, Nickolai Zeldovich wrote:
> On riscv, a user page mapped execute-only becomes readable when the
> kernel write-protects its PTE. An unprivileged process that writes to
> a MAP_SHARED file mapping, changes it to PROT_EXEC with mprotect() and
> then has the page written back (msync(), or any writeback of the file)
> can read the page afterwards, although every load from it faulted
> before the writeback; the same happens to an execute-only anonymous
> THP in the parent and the child of a fork(). The mapping stays
> readable until the next mprotect(). Nothing else breaks, but
> execute-only memory loses its point: code that relies on it not being
> readable from user space can be read. This affects every kernel since
> v7.0.
> 
> Since commit 540de7ade1e1 ("riscv/mm: update write protect to work on
> shadow stacks"), pte_wrprotect() sets _PAGE_READ unconditionally, so
> that a shadow stack PTE (XWR = 010) becomes XWR = 001 rather than the
> non-leaf encoding 000. That commit assumed that every other PTE it is
> applied to already has _PAGE_READ. Execute-only PTEs (XWR = 100,
> PAGE_EXEC, used for PROT_EXEC mappings) do not, and pte_wrprotect() is
> applied to PTEs that are not writable: the writeback path
> folio_mkclean() -> page_vma_mkclean_one() write-protects every present
> PTE that is dirty or writable, and mprotect() keeps _PAGE_DIRTY in the
> PTE (_PAGE_CHG_MASK), so a written page that was made execute-only is
> turned into XWR = 101. copy_huge_pmd() write protects an anonymous huge
> PMD whether or not it is writable, through pmdp_set_wrprotect() ->
> ptep_set_wrprotect() for the parent, which open-codes the same
> expression, and pmd_wrprotect() for the child.
> 
> Set _PAGE_READ only when the PTE is writable, which is the shadow stack
> case the bit was added for (a writable PTE without _PAGE_READ is only
> ever a shadow stack PTE), and use pte_wrprotect() in
> ptep_set_wrprotect() instead of repeating the expression. For a PTE
> that is not writable pte_wrprotect() is a no-op again, as it was before
> 540de7ade1e1.
> 
> Tested under QEMU (riscv64 virt, defconfig with an ext4 disk): after
> write, mprotect(PROT_EXEC) and msync(MS_SYNC) a load from the shared
> file page succeeds without this patch and faults with it; with
> CONFIG_TRANSPARENT_HUGEPAGE the execute-only anonymous THP is readable
> after fork() without this patch and not with it.
> 
> Fixes: 540de7ade1e1 ("riscv/mm: update write protect to work on shadow stacks")
> Assisted-by: LLM
> Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>

Reviewed-by: Jisheng Zhang <jszhang@kernel.org>

> ---
> 
> Notes:
>     The second hunk (ptep_set_wrprotect()) conflicts with "riscv: mm: Make
>     ptep_set_wrprotect() atomic again" (posted 2026-10-16), which already
>     computes the new PTE with pte_wrprotect(); if that patch is applied
>     first, only the pte_wrprotect() hunk of this one is needed, and the
>     fork/THP case is fixed through it.
>     
>     Reproducer (riscv64; the file case needs a block-backed filesystem such
>     as ext4, the THP case CONFIG_TRANSPARENT_HUGEPAGE; no privileges):
>     
>         /* An execute-only mapping must not become readable through writeback or fork. */
>         #include <fcntl.h>
>         #include <setjmp.h>
>         #include <signal.h>
>         #include <stdio.h>
>         #include <sys/mman.h>
>         #include <sys/wait.h>
>         #include <unistd.h>
>     
>         static sigjmp_buf env;
>     
>         static void segv(int sig)
>         {
>         	siglongjmp(env, 1);
>         }
>     
>         /* 1 if a user load from p succeeds */
>         static int readable(volatile char *p)
>         {
>         	if (sigsetjmp(env, 1))
>         		return 0;
>         	(void)*p;
>         	return 1;
>         }
>     
>         int main(int argc, char **argv)
>         {
>         	const char *path = argc > 1 ? argv[1] : "/mnt/xom";
>         	size_t thp = 2UL << 20;
>         	int fd, bad = 0;
>         	char *p, *a;
>     
>         	signal(SIGSEGV, segv);
>     
>         	/* Shared file mapping: write, make it execute-only, write back. */
>         	fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0600);
>         	if (fd < 0 || ftruncate(fd, 4096))
>         		return perror(path), 2;
>         	p = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>         	if (p == MAP_FAILED)
>         		return perror("mmap"), 2;
>         	p[0] = 1;
>         	mprotect(p, 4096, PROT_EXEC);
>         	printf("file: before msync readable=%d\n", readable(p));
>         	msync(p, 4096, MS_SYNC);
>         	printf("file: after msync  readable=%d\n", readable(p));
>         	bad |= readable(p);
>     
>         	/* Anonymous THP: write, make it execute-only, fork. */
>         	a = mmap(NULL, 2 * thp, PROT_READ | PROT_WRITE,
>         		 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>         	a = (char *)(((unsigned long)a + thp - 1) & ~(thp - 1));
>         	madvise(a, thp, MADV_HUGEPAGE);
>         	a[0] = 1;
>         	mprotect(a, thp, PROT_EXEC);
>         	printf("thp:  before fork  readable=%d\n", readable(a));
>         	if (fork() == 0)
>         		_exit(0);
>         	wait(NULL);
>         	printf("thp:  after fork   readable=%d\n", readable(a));
>         	bad |= readable(a);
>     
>         	printf("%s\n", bad ? "FAIL: execute-only page became readable" : "PASS");
>         	return bad;
>         }
>     
>         riscv64-linux-gnu-gcc -O2 -static -o xom xom_min.c
>         ./xom /mnt/ext4/xom
>     
>     Before (mainline 70dd4ee72eb2, defconfig + THP, QEMU virt):
>         file: before msync readable=0
>         file: after msync  readable=1
>         thp:  before fork  readable=0
>         thp:  after fork   readable=1
>         FAIL: execute-only page became readable
>     
>     After:
>         file: before msync readable=0
>         file: after msync  readable=0
>         thp:  before fork  readable=0
>         thp:  after fork   readable=0
>         PASS
> 
>  arch/riscv/include/asm/pgtable.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 40b1ed4f3ea8..872b8b6a944b 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -406,7 +406,11 @@ static inline int pte_special(pte_t pte)
>  
>  static inline pte_t pte_wrprotect(pte_t pte)
>  {
> -	return __pte((pte_val(pte) & ~(_PAGE_WRITE)) | (_PAGE_READ));
> +	/* Shadow stack is XWR = 010: keep a leaf by making it readable */
> +	if (pte_val(pte) & _PAGE_WRITE)
> +		pte = __pte(pte_val(pte) | _PAGE_READ);
> +
> +	return __pte(pte_val(pte) & ~(_PAGE_WRITE));
>  }
>  
>  #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
> @@ -692,8 +696,7 @@ static inline void ptep_set_wrprotect(struct mm_struct *mm,
>  	 * encoding 000b which is wrong encoding with V = 1. This should lead to page fault
>  	 * but we dont want this wrong configuration to be set in page tables.
>  	 */
> -	atomic_long_set((atomic_long_t *)ptep,
> -			((pte_val(read_pte) & ~(unsigned long)_PAGE_WRITE) | _PAGE_READ));
> +	atomic_long_set((atomic_long_t *)ptep, pte_val(pte_wrprotect(read_pte)));
>  }
>  
>  #define __HAVE_ARCH_PTEP_CLEAR_YOUNG_FLUSH
> 
> base-commit: 70dd4ee72eb2d5793c62e71c14e3aba78e5a4e56
> -- 
> 2.53.0
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv