This is an automated email from the ASF dual-hosted git repository. acassis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 96bd521662d78159d2f0e2f67cf94fb0abf91f11 Author: Marco Casaroli <[email protected]> AuthorDate: Mon Oct 19 21:34:40 2026 +0200 arch/risc-v: Check the range in up_addrenv_mprot(). up_addrenv_mprot() walked the page tables for each page of the range and changed the last level entry. It did not check that the address is a user address, and it left the first pages changed when a later page had no page table. PROT_WRITE without PROT_READ set W without R, which is a reserved encoding: each access to the page faults. Now the function first checks each page: a user address with a valid page entry. If one page fails the check, it changes nothing and returns -ENOMEM. With CONFIG_PAGING a page of .text, .data or the heap that is not mapped yet passes the check, because the page fault handler maps it at its first access. PROT_WRITE now also sets R. The ELF loader is the only caller today, and its .text range passes the check. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Marco Casaroli <[email protected]> --- arch/risc-v/src/common/riscv_addrenv_perms.c | 70 +++++++++++++++++++++------- 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/arch/risc-v/src/common/riscv_addrenv_perms.c b/arch/risc-v/src/common/riscv_addrenv_perms.c index a989a596b43..cea004db7c8 100644 --- a/arch/risc-v/src/common/riscv_addrenv_perms.c +++ b/arch/risc-v/src/common/riscv_addrenv_perms.c @@ -50,14 +50,44 @@ * Private Functions ****************************************************************************/ +static uintptr_t get_pgtable(uintptr_t l1vaddr, uintptr_t vaddr) +{ + uintptr_t lnvaddr; + uint32_t ptlevel; + + for (ptlevel = 1, lnvaddr = l1vaddr; + ptlevel < RV_MMU_PT_LEVELS && lnvaddr != 0; + ptlevel++) + { + lnvaddr = riscv_pgvaddr(mmu_pte_to_paddr( + mmu_ln_getentry(ptlevel, lnvaddr, vaddr))); + } + + return lnvaddr; +} + +/* With on-demand paging the page fault handler maps a page of .text, .data + * or the heap at its first access. + */ + +static bool paged_vaddr(uintptr_t vaddr) +{ +#ifdef CONFIG_PAGING + return (vaddr >= CONFIG_ARCH_TEXT_VBASE && vaddr < ARCH_TEXT_VEND) || + (vaddr >= CONFIG_ARCH_DATA_VBASE && vaddr < ARCH_DATA_VEND) || + (vaddr >= CONFIG_ARCH_HEAP_VBASE && vaddr < ARCH_HEAP_VEND); +#else + UNUSED(vaddr); + return false; +#endif +} + static int modify_region(uintptr_t vstart, uintptr_t vend, uintptr_t setmask) { uintptr_t l1vaddr; uintptr_t lnvaddr; uintptr_t entry; - uintptr_t paddr; uintptr_t vaddr; - uint32_t ptlevel; /* Must perform a reverse table walk */ @@ -70,29 +100,35 @@ static int modify_region(uintptr_t vstart, uintptr_t vend, uintptr_t setmask) return -EINVAL; } + /* Change nothing unless each page is mapped user memory */ + for (vaddr = vstart; vaddr < vend; vaddr += MM_PGSIZE) { - for (ptlevel = 1, lnvaddr = l1vaddr; - ptlevel < RV_MMU_PT_LEVELS; - ptlevel++) + lnvaddr = get_pgtable(l1vaddr, vaddr); + if (!riscv_uservaddr(vaddr) || lnvaddr == 0) { - paddr = mmu_pte_to_paddr(mmu_ln_getentry(ptlevel, lnvaddr, vaddr)); - lnvaddr = riscv_pgvaddr(paddr); - if (!lnvaddr) - { - return -EINVAL; - } + return -ENOMEM; } + if ((mmu_ln_getentry(RV_MMU_PT_LEVELS, lnvaddr, vaddr) & + PTE_VALID) == 0 && !paged_vaddr(vaddr)) + { + return -ENOMEM; + } + } + + for (vaddr = vstart; vaddr < vend; vaddr += MM_PGSIZE) + { /* Get entry and modify the flags */ - entry = mmu_ln_getentry(ptlevel, lnvaddr, vaddr); - entry &= ~CLR_MASK; - entry |= setmask; + lnvaddr = get_pgtable(l1vaddr, vaddr); + entry = mmu_ln_getentry(RV_MMU_PT_LEVELS, lnvaddr, vaddr); + entry &= ~CLR_MASK; + entry |= setmask; /* Restore the entry */ - mmu_ln_restore(ptlevel, lnvaddr, vaddr, entry); + mmu_ln_restore(RV_MMU_PT_LEVELS, lnvaddr, vaddr, entry); } /* When all is set and done, flush the data caches */ @@ -142,9 +178,11 @@ int up_addrenv_mprot(arch_addrenv_t *addrenv, uintptr_t addr, size_t len, setmask |= PTE_R; } + /* A writable page must be readable: W without R is reserved */ + if (prot & PROT_WRITE) { - setmask |= PTE_W; + setmask |= PTE_R | PTE_W; } if (prot & PROT_EXEC)
