2025-04-23T17:45:53-07:00, Deepak Gupta <de...@rivosinc.com>: > On Thu, Apr 10, 2025 at 12:03:44PM +0200, Radim Krčmář wrote: >>2025-03-14T14:39:25-07:00, Deepak Gupta <de...@rivosinc.com>: >>> 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. > > You're right. Above hunk (arch/riscv/include/asm/mman.h) alone should be > enough. > I did this change in `sys_riscv.c` out of caution. If it feels like > un-necessary, > I'll remove it. No hard feelings either way.
I think it makes the code harder to reason about. Here it is not clear why this caller of ksys_mmap_pgoff() has to do this, while others don't.
