d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

riscv: fix VM_MAXUSER_ADDRESS checks in asm routines

Browse Source 
There are two issues with the checks against VM_MAXUSER_ADDRESS. First,
the comparison should consider the values as unsigned, otherwise
addresses with the high bit set will fail to branch. Second, the value
of VM_MAXUSER_ADDRESS is, by convention, one larger than the maximum
mappable user address and invalid itself. Thus, use the bgeu instruction
for these comparisons.

Add a regression test case for copyin(9).

PR:		257193
Reported by:	Robert Morris <rtm@lcs.mit.edu>
Reviewed by:	markj
Differential Revision:	https://reviews.freebsd.org/D31209
This commit is contained in:
Mitchell Horne 2021-10-07 18:12:30 -03:00
parent 4a9f2f8b07
commit 8babb5582e
3 changed files with 37 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
sys/riscv/riscv/copyinout.S
View File

@ -118,7 +118,7 @@ ENTRY(copyout)
beqz a2, copyout_end /* If len == 0 then skip loop */
add a3, a1, a2
li a4, VM_MAXUSER_ADDRESS
bgt a3, a4, copyio_fault_nopcb
bgeu a3, a4, copyio_fault_nopcb
copycommon
@ -136,7 +136,7 @@ ENTRY(copyin)
beqz a2, copyin_end /* If len == 0 then skip loop */
add a3, a0, a2
li a4, VM_MAXUSER_ADDRESS
bgt a3, a4, copyio_fault_nopcb
bgeu a3, a4, copyio_fault_nopcb
copycommon
@ -159,7 +159,7 @@ ENTRY(copyinstr)
ENTER_USER_ACCESS(a7)
li a7, VM_MAXUSER_ADDRESS
1: bgt a0, a7, copyio_fault
1: bgeu a0, a7, copyio_fault
lb a4, 0(a0) /* Load from uaddr */
addi a0, a0, 1
sb a4, 0(a1) /* Store in kaddr */

20
sys/riscv/riscv/support.S
View File

@ -56,7 +56,7 @@ END(fsu_fault)
*/
ENTRY(casueword32)
li a4, (VM_MAXUSER_ADDRESS-3)
bgt a0, a4, fsu_fault_nopcb
bgeu a0, a4, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a4) /* And set it */
ENTER_USER_ACCESS(a4)
@ -77,7 +77,7 @@ END(casueword32)
*/
ENTRY(casueword)
li a4, (VM_MAXUSER_ADDRESS-7)
bgt a0, a4, fsu_fault_nopcb
bgeu a0, a4, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a4) /* And set it */
ENTER_USER_ACCESS(a4)
@ -98,7 +98,7 @@ END(casueword)
*/
ENTRY(fubyte)
li a1, VM_MAXUSER_ADDRESS
bgt a0, a1, fsu_fault_nopcb
bgeu a0, a1, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a1) /* And set it */
ENTER_USER_ACCESS(a1)
@ -113,7 +113,7 @@ END(fubyte)
*/
ENTRY(fuword16)
li a1, (VM_MAXUSER_ADDRESS-1)
bgt a0, a1, fsu_fault_nopcb
bgeu a0, a1, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a1) /* And set it */
ENTER_USER_ACCESS(a1)
@ -128,7 +128,7 @@ END(fuword16)
*/
ENTRY(fueword32)
li a2, (VM_MAXUSER_ADDRESS-3)
bgt a0, a2, fsu_fault_nopcb
bgeu a0, a2, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a2) /* And set it */
ENTER_USER_ACCESS(a2)
@ -147,7 +147,7 @@ END(fueword32)
ENTRY(fueword)
EENTRY(fueword64)
li a2, (VM_MAXUSER_ADDRESS-7)
bgt a0, a2, fsu_fault_nopcb
bgeu a0, a2, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a2) /* And set it */
ENTER_USER_ACCESS(a2)
@ -165,7 +165,7 @@ END(fueword)
*/
ENTRY(subyte)
li a2, VM_MAXUSER_ADDRESS
bgt a0, a2, fsu_fault_nopcb
bgeu a0, a2, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a2) /* And set it */
ENTER_USER_ACCESS(a2)
@ -181,7 +181,7 @@ END(subyte)
*/
ENTRY(suword16)
li a2, (VM_MAXUSER_ADDRESS-1)
bgt a0, a2, fsu_fault_nopcb
bgeu a0, a2, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a2) /* And set it */
ENTER_USER_ACCESS(a2)
@ -197,7 +197,7 @@ END(suword16)
*/
ENTRY(suword32)
li a2, (VM_MAXUSER_ADDRESS-3)
bgt a0, a2, fsu_fault_nopcb
bgeu a0, a2, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a2) /* And set it */
ENTER_USER_ACCESS(a2)
@ -214,7 +214,7 @@ END(suword32)
ENTRY(suword)
EENTRY(suword64)
li a2, (VM_MAXUSER_ADDRESS-7)
bgt a0, a2, fsu_fault_nopcb
bgeu a0, a2, fsu_fault_nopcb
la a6, fsu_fault /* Load the fault handler */
SET_FAULT_HANDLER(a6, a2) /* And set it */
ENTER_USER_ACCESS(a2)

24
tests/sys/kern/kern_copyin.c
View File

@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$");
#include <sys/exec.h>
#include <sys/sysctl.h>
#include <errno.h>
#include <fcntl.h>
#include <limits.h>
#include <stdio.h>
#include <stdlib.h>
@ -54,6 +55,21 @@ copyin_checker(uintptr_t uaddr, size_t len)
return (ret == -1 ? errno : 0);
}
#if __SIZEOF_POINTER__ == 8
/*
* A slightly more direct path to calling copyin(), but without the ability
* to specify a length.
*/
static int
copyin_checker2(uintptr_t uaddr)
{
int ret;
ret = fcntl(scratch_file, F_GETLK, (const void *)uaddr);
return (ret == -1 ? errno : 0);
}
#endif
#ifdef __amd64__
static uintptr_t
get_maxuser_address(void)
@ -83,6 +99,10 @@ get_maxuser_address(void)
#endif
#define FMAX ULONG_MAX
#if __SIZEOF_POINTER__ == 8
/* PR 257193 */
#define ADDR_SIGNED 0x800000c000000000
#endif
ATF_TC_WITHOUT_HEAD(kern_copyin);
ATF_TC_BODY(kern_copyin, tc)
@ -122,6 +142,10 @@ ATF_TC_BODY(kern_copyin, tc)
ATF_CHECK(copyin_checker(FMAX - 10, 9) == EFAULT);
ATF_CHECK(copyin_checker(FMAX - 10, 10) == EFAULT);
ATF_CHECK(copyin_checker(FMAX - 10, 11) == EFAULT);
#if __SIZEOF_POINTER__ == 8
ATF_CHECK(copyin_checker(ADDR_SIGNED, 1) == EFAULT);
ATF_CHECK(copyin_checker2(ADDR_SIGNED) == EFAULT);
#endif
}
ATF_TP_ADD_TCS(tp)