Re: [PATCH 1/2] RISC-V: mm: Restrict address space for sv39,sv48,sv57

[Jessica Clarke <jrtc27@jrtc27.com>]

On 27 Jun 2023, at 23:21, Charlie Jenkins <charlie@rivosinc.com> wrote:
> 
> Make sv39 the default address space for mmap as some applications
> currently depend on this assumption.

They are just plain wrong too. Sv48 was in even Priv v1.10 (the first
spec where satp was named as such and contained the mode, rather than
requiring M-mode’s help in configuring virtual memory), predating the
ratified v1.11 spec. A 39-bit address space is pathetic and has
implications for ASLR.

I strongly suggest applications be forced to support at least Sv48,
which is totally reasonable given the address space sizes used by other
architectures. Sv57 is more disruptive to some runtimes, though ideally
even that would be free for the kernel to use rather than committing to
not using it for the default uABI.

Jess

> The RISC-V specification enforces
> that bits outside of the virtual address range are not used, so
> restricting the size of the default address space as such should be
> temporary. A hint address passed to mmap will cause the largest address
> space that fits entirely into the hint to be used. If the hint is less
> than or equal to 1<<38, a 39-bit address will be used. After an address
> space is completely full, the next smallest address space will be used.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
> arch/riscv/include/asm/elf.h       |  2 +-
> arch/riscv/include/asm/pgtable.h   | 13 +++++++++-
> arch/riscv/include/asm/processor.h | 41 +++++++++++++++++++++++++-----
> 3 files changed, 47 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
> index 30e7d2455960..1b57f13a1afd 100644
> --- a/arch/riscv/include/asm/elf.h
> +++ b/arch/riscv/include/asm/elf.h
> @@ -49,7 +49,7 @@ extern bool compat_elf_check_arch(Elf32_Ehdr *hdr);
>  * the loader.  We need to make sure that it is out of the way of the program
>  * that it will "exec", and that there is sufficient room for the brk.
>  */
> -#define ELF_ET_DYN_BASE ((TASK_SIZE / 3) * 2)
> +#define ELF_ET_DYN_BASE ((DEFAULT_MAP_WINDOW / 3) * 2)
> 
> #ifdef CONFIG_64BIT
> #ifdef CONFIG_COMPAT
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 75970ee2bda2..e83912e97870 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -57,18 +57,29 @@
> #define MODULES_END (PFN_ALIGN((unsigned long)&_start))
> #endif
> 
> +
> /*
>  * Roughly size the vmemmap space to be large enough to fit enough
>  * struct pages to map half the virtual address space. Then
>  * position vmemmap directly below the VMALLOC region.
>  */
> #ifdef CONFIG_64BIT
> +#define VA_BITS_SV39 39
> +#define VA_BITS_SV48 48
> +#define VA_BITS_SV57 57
> +
> +#define VA_USER_SV39 (UL(1) << (VA_BITS_SV39 - 1))
> +#define VA_USER_SV48 (UL(1) << (VA_BITS_SV48 - 1))
> +#define VA_USER_SV57 (UL(1) << (VA_BITS_SV57 - 1))
> +
> #define VA_BITS (pgtable_l5_enabled ? \
> - 57 : (pgtable_l4_enabled ? 48 : 39))
> + VA_BITS_SV57 : (pgtable_l4_enabled ? VA_BITS_SV48 : VA_BITS_SV39))
> #else
> #define VA_BITS 32
> #endif
> 
> +#define DEFAULT_VA_BITS ((VA_BITS >= VA_BITS_SV39) ? VA_BITS_SV39 : VA_BITS)
> +
> #define VMEMMAP_SHIFT \
> (VA_BITS - PAGE_SHIFT - 1 + STRUCT_PAGE_MAX_SHIFT)
> #define VMEMMAP_SIZE BIT(VMEMMAP_SHIFT)
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 6fb8bbec8459..019dcd4ecae4 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -12,20 +12,47 @@
> 
> #include <asm/ptrace.h>
> 
> -/*
> - * This decides where the kernel will search for a free chunk of vm
> - * space during mmap's.
> - */
> -#define TASK_UNMAPPED_BASE PAGE_ALIGN(TASK_SIZE / 3)
> -
> -#define STACK_TOP TASK_SIZE
> #ifdef CONFIG_64BIT
> +#define DEFAULT_MAP_WINDOW (UL(1) << (DEFAULT_VA_BITS - 1))
> #define STACK_TOP_MAX TASK_SIZE_64
> +
> +#define arch_get_mmap_end(addr, len, flags) \
> + ((addr) == 0 || (addr) >= VA_USER_SV57 ? STACK_TOP_MAX :   \
> + (((addr) >= VA_USER_SV48) && (VA_BITS >= VA_BITS_SV48)) ? \
> + VA_USER_SV48 : \
> + VA_USER_SV39)
> +
> +#define arch_get_mmap_base(addr, base) \
> + (((addr >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) ?   \
> + base + STACK_TOP_MAX - DEFAULT_MAP_WINDOW : \
> + (((addr) >= VA_USER_SV48) && (VA_BITS >= VA_BITS_SV48)) ? \
> + base + VA_USER_SV48 - DEFAULT_MAP_WINDOW : \
> + base)
> +
> #else
> +#define DEFAULT_MAP_WINDOW TASK_SIZE
> #define STACK_TOP_MAX TASK_SIZE
> +
> +#define arch_get_mmap_end(addr, len, flags) \
> + ((addr) > DEFAULT_MAP_WINDOW ? STACK_TOP_MAX : DEFAULT_MAP_WINDOW)
> +
> +#define arch_get_mmap_base(addr, base) \
> + ((addr > DEFAULT_MAP_WINDOW) ? \
> + base + STACK_TOP_MAX - DEFAULT_MAP_WINDOW : \
> + base)
> +
> #endif
> #define STACK_ALIGN 16
> 
> +
> +#define STACK_TOP DEFAULT_MAP_WINDOW
> +
> +/*
> + * This decides where the kernel will search for a free chunk of vm
> + * space during mmap's.
> + */
> +#define TASK_UNMAPPED_BASE PAGE_ALIGN(DEFAULT_MAP_WINDOW / 3)
> +
> #ifndef __ASSEMBLY__
> 
> struct task_struct;
> -- 
> 2.34.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv