Re: [PATCH] RISC-V: Don't allow write+exec only page mapping request in mmap

[Palmer Dabbelt]

On Tue, 16 Jun 2020 07:03:06 PDT (-0700), yash.shah@xxxxxxxxxx wrote:
> As per the table 4.2 of the RISC-V instruction set manual[0], the PTE
> permission bit combination of "write+exec only" is invalid and reserved
> for future use. Hence, don't allow such mapping request in mmap call.
>
> An issue is been reported by David Abdurachmanov, that while running
> stress-ng with "sysbadaddr" argument, RCU stalls are observed on RISC-V
> specific kernel.
>
> This issue arises when the stress-sysbadaddr request for pages with
> "write+exec only" permission bits and then passes the address obtain
> from this mmap call to various system call. For the riscv kernel, the
> mmap call should fail for this particular combination of permission bits
> since it's not valid.
>
> [0]: https://www2.eecs.berkeley.edu/Pubs/TechRpts/2016/EECS-2016-161.pdf

That's super old.  I can't figure out how to get a stable link to a new
privilege spec (the riscv.org website has some crazy wordpress paths that I
don't trust, and github doesn't appear to have the PDF for the ratified 1.11
tag).  I'm just going to put the ratified PDF on dabbelt.com, as at least I
have control over that.  LMK if anyone knows where to find the ratified user
PDF of the manual, as that'd be nice to have as well...

It's now table 4.4 in the PDF I get from riscv.org, see

https://github.com/palmer-dabbelt/website/commit/4c2676320c9b580f592bd0a1074bb3c6507d97a5

or

http://dabbelt.com/~palmer/keep/riscv-isa-manual/riscv-privileged-20190608-1.pdf

> Signed-off-by: Yash Shah <yash.shah@xxxxxxxxxx>
> Reported-by: David Abdurachmanov <david.abdurachmanov@xxxxxxxxx>
> ---
>  arch/riscv/kernel/sys_riscv.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
> index f3619f5..12f8a7f 100644
> --- a/arch/riscv/kernel/sys_riscv.c
> +++ b/arch/riscv/kernel/sys_riscv.c
> @@ -8,6 +8,7 @@
>  #include <linux/syscalls.h>
>  #include <asm/unistd.h>
>  #include <asm/cacheflush.h>
> +#include <asm-generic/mman-common.h>
>
>  static long riscv_sys_mmap(unsigned long addr, unsigned long len,
>  			   unsigned long prot, unsigned long flags,
> @@ -16,6 +17,11 @@ static long riscv_sys_mmap(unsigned long addr, unsigned long len,
>  {
>  	if (unlikely(offset & (~PAGE_MASK >> page_shift_offset)))
>  		return -EINVAL;
> +
> +	if ((prot & PROT_WRITE) && (prot & PROT_EXEC))
> +		if (unlikely(!(prot & PROT_READ)))
> +			return -EINVAL;
> +
>  	return ksys_mmap_pgoff(addr, len, prot, flags, fd,
>  			       offset >> (PAGE_SHIFT - page_shift_offset));
>  }

This is on fixes, with my cleanups.  I'd really prefer to avoid linking to
dabbelt.com, so LMK if you have a better way to handle it.  I'm planning on
sumbitting an rc2 PR tomorrow, though...

Thanks!