Re: riscv64: adjust VM_MIN_ADDRESS

2022-03-21 Thread Mark Kettenis
> Date: Mon, 21 Mar 2022 19:56:14 +
> From: Miod Vallat 
> 
> A long, long time ago (I think it was late 2003), OpenBSD/i386 was
> vulnerable to a trusted-yet-NULL pointer dereference in the agp code.
> The attack involved using mmap(2) with MAP_FIXED and a hint of zero.
> 
> Shortly afterwards, in addition to fixing the overtrusting code, it was
> decided never to allow mmap(2) to allow address zero to get mapped, by
> never making VM_MIN_ADDRESS equal to zero (I actually argued for this
> change to only be applied to platforms with shared kernel/userland
> address spaces, but the party's line prevailed), which is why
> VM_MIN_ADDRESS is nowadays PAGE_SIZE instead of zero.
> 
> Except on riscv64.
> 
> The following diff adjusts VM_MIN_ADDRESS to follow the party's line.
> 
> Completely untested due to lack of hardware.

Bleah, I thought we fixed that already.

kernel boots; ok kettenis@

> Index: vmparam.h
> ===
> RCS file: /OpenBSD/src/sys/arch/riscv64/include/vmparam.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 vmparam.h
> --- vmparam.h 2 Jul 2021 10:42:22 -   1.5
> +++ vmparam.h 21 Mar 2022 19:49:00 -
> @@ -111,8 +111,7 @@
>   * VM_MIN_USER_ADDRESS and VM_MAX_USER_ADDRESS define the start and end of 
> the
>   * user address space.
>   */
> -// XXX OpenBSD/arm64 starts VM_MIN_ADDRESS from PAGE_SIZE. Why?
> -#define  VM_MIN_ADDRESS  (0xUL)
> +#define  VM_MIN_ADDRESS  ((vaddr_t)PAGE_SIZE)
>  #define  VM_MAX_ADDRESS  (0xUL)
>  
>  #define  VM_MIN_KERNEL_ADDRESS   (0xffc0UL)
> 
> 



Re: riscv64: adjust VM_MIN_ADDRESS

2022-03-21 Thread Theo de Raadt
Miod Vallat  wrote:

Absolutely this should be fixed.

> Shortly afterwards, in addition to fixing the overtrusting code, it was
> decided never to allow mmap(2) to allow address zero to get mapped, by
> never making VM_MIN_ADDRESS equal to zero (I actually argued for this
> change to only be applied to platforms with shared kernel/userland
> address spaces, but the party's line prevailed), which is why
> VM_MIN_ADDRESS is nowadays PAGE_SIZE instead of zero.

I still stand by the position that accidental *NULL dereference should
not accidentally land on top of an a mapped 0 page.  Not in userland and
not in the kernel either.

I continue to be worried by attackers who have incomplete control, but
are able to trick a mmap call to map at 0, and then in the following
incomplete attack hide NULL or NULL+n dereferences, before later steps
which elevate to control.

On the other hand, noone needs to map memory there.

So blocking page 0 still makes sense.



riscv64: adjust VM_MIN_ADDRESS

2022-03-21 Thread Miod Vallat
A long, long time ago (I think it was late 2003), OpenBSD/i386 was
vulnerable to a trusted-yet-NULL pointer dereference in the agp code.
The attack involved using mmap(2) with MAP_FIXED and a hint of zero.

Shortly afterwards, in addition to fixing the overtrusting code, it was
decided never to allow mmap(2) to allow address zero to get mapped, by
never making VM_MIN_ADDRESS equal to zero (I actually argued for this
change to only be applied to platforms with shared kernel/userland
address spaces, but the party's line prevailed), which is why
VM_MIN_ADDRESS is nowadays PAGE_SIZE instead of zero.

Except on riscv64.

The following diff adjusts VM_MIN_ADDRESS to follow the party's line.

Completely untested due to lack of hardware.

Index: vmparam.h
===
RCS file: /OpenBSD/src/sys/arch/riscv64/include/vmparam.h,v
retrieving revision 1.5
diff -u -p -r1.5 vmparam.h
--- vmparam.h   2 Jul 2021 10:42:22 -   1.5
+++ vmparam.h   21 Mar 2022 19:49:00 -
@@ -111,8 +111,7 @@
  * VM_MIN_USER_ADDRESS and VM_MAX_USER_ADDRESS define the start and end of the
  * user address space.
  */
-// XXX OpenBSD/arm64 starts VM_MIN_ADDRESS from PAGE_SIZE. Why?
-#defineVM_MIN_ADDRESS  (0xUL)
+#defineVM_MIN_ADDRESS  ((vaddr_t)PAGE_SIZE)
 #defineVM_MAX_ADDRESS  (0xUL)
 
 #defineVM_MIN_KERNEL_ADDRESS   (0xffc0UL)