Re: [PATCH v12 06/28] riscv/mm : ensure PROT_WRITE leads to VM_READ | VM_WRITE

2025-03-14T14:39:25-07:00, Deepak Gupta debug@rivosinc.com:

diff --git a/arch/riscv/include/asm/mman.h b/arch/riscv/include/asm/mman.h +static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,

• 				   unsigned long pkey __always_unused)
  

+{

• unsigned long ret = 0;
• /*

• * If PROT_WRITE was specified, force it to VM_READ | VM_WRITE.
  

• * Only VM_WRITE means shadow stack.
  

• */
  

This function also changes PROT_WX to VM_RWX, which is effectively not changing anything, but I think it deserves an explicit intent. (At least in the commit message.)

• if (prot & PROT_WRITE)

• ret = (VM_READ | VM_WRITE);
  

• return ret;

+} diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c @@ -16,6 +17,15 @@ static long riscv_sys_mmap(unsigned long addr, unsigned long len,

• /*

• * If PROT_WRITE is specified then extend that to PROT_READ
  

• * protection_map[VM_WRITE] is now going to select shadow stack encodings.
  

• * So specifying PROT_WRITE actually should select protection_map [VM_WRITE | VM_READ]
  

• * If user wants to create shadow stack then they should use `map_shadow_stack` syscall.
  

• */
  

• if (unlikely((prot & PROT_WRITE) && !(prot & PROT_READ)))

• prot |= PROT_READ;
  

Why isn't the previous hunk be enough? (Or why don't we do just this?)

riscv_sys_mmap() eventually calls arch_calc_vm_prot_bits(), so I'd rather fix each code path just once.

Thanks.